[BUG] NullPointerException in serializeToResValuesXml

[IgorEisberg]

Reviving issue #2709 as I stumbled upon it with an updated app I was trying to decompile.
The sample APK provided at the bottom of the issue is sufficient to demonstrate the issue.
Using aapt2 dump resources you can see a problematic color resource with an unexpected @null value:

...
    resource 0x7f0601f9 color/green_f2fdf1
      () #fff2fdf1
    resource 0x7f0601fa color/green_highlighter_feedback
      () #e9cbf2ce
    resource 0x7f0601fb color/green_light_gradient
      () #ff52cb8c
    resource 0x7f0601fc color/green_oc831f
      () @null
    resource 0x7f0601fd color/grey
      () #ffd0cecd
    resource 0x7f0601fe color/grey_121518
      () #ff121518
    resource 0x7f0601ff color/grey_535353
      () #ff9d9d9d
...

From debugging how Apktool categorizes the value type, it's detected as a string type rather than a reference. I have no idea how this happens to begin with, but we should at least allow this to be decompiled, even as an <item> with a @null reference (aapt2 allows to recompile that).

[iBotPeaches]

Thanks - I'll spend a bit on investigating why the type is acting odd before I just take a shortcut and patch an NPE.

[iBotPeaches]

So I peeked a few areas that I believe I need to investigate further.

Apktool/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/value/ResValueFactory.java

Lines 35 to 40 in 63b1976

	if (value == TypedValue.DATA_NULL_UNDEFINED) { // Special case $empty as explicitly defined empty value
	return new ResStringValue(null, value);
	} else if (value == TypedValue.DATA_NULL_EMPTY) {
	return new ResEmptyValue(value, rawValue, type);
	}
	return new ResReferenceValue(mPackage, 0, null);

• Controls the creation of value class and explains the oddity of string/empty/reference

Apktool/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/value/ResScalarValue.java

Line 71 in 63b1976

boolean item = !"reference".equals(mType) && !type.equals(mType);

• Really old line that dates back to 2010 - unsure its purpose, but clearly the toggling of vs the type is a buggy inference

Apktool/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/value/ResScalarValue.java

Lines 75 to 94 in 63b1976

	// check for resource reference
	if (!type.equalsIgnoreCase("color")) {
	if (body.contains("@")) {
	if (!res.getFilePath().contains("string")) {
	item = true;
	}
	}
	}
	// Dummy attributes should be <item> with type attribute
	if (res.getResSpec().isDummyResSpec()) {
	item = true;
	}
	// Android does not allow values (false) for ids.xml anymore
	// https://issuetracker.google.com/issues/80475496
	// But it decodes as a ResBoolean, which makes no sense. So force it to empty
	if (type.equalsIgnoreCase("id") && !body.isEmpty()) {
	body = "";
	}

• Tons of workarounds on swapping item on/off based on critiera

[iBotPeaches]

Taking a rework of the first code snippet I posted. PR is up.