Use xmlpull-style parser and serialiser

[MuntashirAkon]

This is just what I've felt while using the library and only my expert opinion. I can also help you implementing this if you choose to do this.

The use of xmlpull-style parser and serialiser is quite common in Android ecosystem. The framework itself uses this library for parsing both normal and binary XML (newly introduced in Android 12) files. So, it allows greater control such as easy to convert between one XML format to another by simultaneous use of parser and serialiser. This also make it easier to separate reading and writing the XML contents. So, I propose, possibly, creating a wrapper around the XMLDocument class.

[REAndroid]

_ .. I can also help you implementing this if you choose to do this._

I really appreciate and you are welcome do this.

The framework itself uses this library for parsing both normal and binary XML (newly introduced in Android 12) files

I tried to implement exactly this, Here are my reasons not to use the standard XmlPullParser.

• XML and speed don't go together, android uses native C/C++ to execute traditional pull parser thus it doesn't care if you parse 10x on single document to get one job done. You can see easily the effect of parsing xml document on APKEditor compiling takes at least 8x time than decompiling. Keep in mind that we already loaded the entire resource table(arsc) on memory and we can't afford to do memory intensive operations.
• Class conflict: this lib is intended to work everywhere, android already has classes of org.xmlpull.* on framework and dex loader will ignore our class.

We can't separate XML and android but I can't see any good reason converting to/from readable XML, we have the power to do any editing live.

[MuntashirAkon]

When I proposed the use of xmlpull, I didn't say that I'd use it to parse raw XML files. I was only implying that the binary XML parser (i.e. ResXmlDocument) should also be wrapped around an XmlPullParser so that we can have more control over it. It is only a simple wrapper and will not add any additional memory overheads. These standardised methods would be beneficial when you're trying to convert binary XML to a readable XML or only trying to make simple changes to it. If you've looked at the implementation of XmlResourceParser of the AOSP, you'd find that they provide a dumbed-down implementation of XmlPullParser. Developers all have their unique requirements. So, providing a binary XML to raw XML converter should not be part of the library. Instead, it should be the job of the developers to do that, and XmlPullParser (along with other tools such as XmlSerializer and other built-in and third-party transformers) greatly simplifies that. It will also save you from taking the pain of developing a binary XML to raw XML converter fulfilling all the requests from the developers (I've already made a few, for instance). I hope you've now understood my point. (I've, in fact, created a parser myself which works great but I'm a bit confused with namespaces which I've left unimplemented.)

For raw XML files, I don't see any reason to package the MXParser with this library in Android (since Android already offers an efficient FastPullParser alternative). So, you can solve this issue by creating two separate modules alongside the core module: One for Android without the xmlpull dependency (and possibly, other optimizations) and another for other platforms with the dependency, and then release them separately. There are a few libraries that does this already.

[REAndroid]

Sorry i misunderstood you, you are right it make sense for bin XML to have standard serializer. A developer have to go thru all lines of ResXmlDocument code to understand what it does. It is easy to implement XmlPullParser on ResXmlDocument.

in fact, created a parser myself which works great but I'm a bit confused with namespaces which I've left unimplemented

if it is on your public repo please share me the link

Now everything on arsc side becoming stable and now i have a chance to fully concentrate on XML

[MuntashirAkon]

if it is on your public repo please share me the link

I haven't yet pushed it.

A developer have to go thru all lines of ResXmlDocument code to understand what it does. It is easy to implement XmlPullParser on ResXmlDocument.

Precisely. Working with ResXmlDocument is not very straight-forward and requires some documentation in order to make sense of the API. I would always closely follow Android because the developers are expected to already familiar with Android ecosystem.

[REAndroid]

Check ResXmlPullParser

[MuntashirAkon]

Check ResXmlPullParser

Almost perfect! This is my initial review:

1. It returns null on missing references. It should return reference numbers (@0xxxxxxxx or ?0xxxxxxx) so that they might be parsed later on.
2. In addition to above, it does not offer an option to provide EntryStore (which is similar to ResourceTable in the AOSP)
3. It does not handle namespace attributes i.e. xmlns and xmlns:prefix attributes (which should be present by default). Optionally, the xmlpull implementation in Android should support the following features:
   i. FEATURE_PROCESS_NAMESPACES - Whether to process namespaces (this is actually sort of prefix-handling if you've read the description)
   ii. FEATURE_REPORT_NAMESPACE_ATTRIBUTES - Whether to display namespace attributes when FEATURE_PROCESS_NAMESPACES is set to true.

[REAndroid]

1. It returns `null` on missing references. It should return reference numbers (`@0xxxxxxxx` or `?0xxxxxxx`) so that they might be parsed later on.

On which method ?

2. In addition to above, it does not offer an option to provide `EntryStore` (which is similar to `ResourceTable` in the AOSP)

For what purpose ?

3. It does not handle namespace attributes i.e. `xmlns` and `xmlns:prefix` attributes (which should be present by default). Optionally, the xmlpull implementation in Android should support the following features:

Will do on next commit

[MuntashirAkon]

On which method ?

The default XmlPullParser methods i.e. getAttributeValue(index) and getAttributeValue(namespace, name)

For what purpose ?

For resolving references. With this implementation, a nasty workaround is required to do this. Example:

    public static String getAttributeName(@NonNull ResXmlPullParser parser, @NonNull EntryStore entryStore, int index) {
        int resourceId = parser.getAttributeNameResource(index);
        String name;
        if (resourceId == 0) {
            name = parser.getAttributeName(index);
        } else {
            EntryGroup group = entryStore.getEntryGroup(resourceId);
            if (group == null) {
                name = String.format("@0x%08x", resourceId);
            } else {
                name = group.getSpecName();
            }
        }
        return name;
    }

    public static String getAttributeValue(@NonNull ResXmlPullParser parser, @NonNull EntryStore entryStore, int currentPackageId, int index) {
        int resourceId = parser.getAttributeNameResource(index);
        // Use Reflection to fetch ResXmlAttribute to resolve references
        ResXmlAttribute attr = null;
        try {
            Method getResXmlAttributeAt = ResXmlPullParser.class.getDeclaredMethod("geResXmlAttributeAt", int.class);
            getResXmlAttributeAt.setAccessible(true);
            attr = (ResXmlAttribute) getResXmlAttributeAt.invoke(parser, index);
        } catch (Throwable th) {
            th.printStackTrace();
        }
        if (attr == null) {
            return parser.getAttributeValue(index);
        }
        ValueType valueType = attr.getValueType();
        int raw = attr.getData();
        String value;
        if (valueType == ValueType.STRING) {
            value = ValueDecoder.escapeSpecialCharacter(attr.getValueAsString());
        } else {
            value = ValueDecoder.decode(entryStore,
                    currentPackageId,
                    resourceId,
                    valueType,
                    raw);
        }
        return value;
    }

[REAndroid]

getAttributeName If you can provide the values I can implement ResXmlPullParser#setDecoder(EntryStore entryStore, int currentPackageId)

getAttributeValue I don't see any purpose of this method except for logging, if you gonna to process this value latter you have to do some encoding again. e.g. dimension value 24.0dp. Anyways if dev can provide EntryStore and currentPackageId we can implement.

If you loaded your bin xml from ApkModule, we can retrieve all decoding materials ResXmlDocument.getApkFile().getTableBlock()

    public boolean getAttributeBooleanValue(int index, boolean defaultValue);
    public int getAttributeResourceValue(int index, int defaultValue);
    public int getAttributeIntValue(int index, int defaultValue); // this one needs fix
    public int getAttributeUnsignedIntValue(int index, int defaultValue);
    public float getAttributeFloatValue(int index, float defaultValue);

I didn't test this on android os, I appreciate if you share me your test result

[MuntashirAkon]

getAttributeName If you can provide the values I can implement ResXmlPullParser#setDecoder(EntryStore entryStore, int currentPackageId)

Sorry, I don't know what values are you looking for. But this is precisely what is needed, I think.

getAttributeValue I don't see any purpose of this method except for logging, if you gonna to process this value latter you have to do some encoding again. e.g. dimension value 24.0dp. Anyways if dev can provide EntryStore and currentPackageId we can implement.

I understand that. But it has use-case when a dev is working with XmlPullParser interface rather than XmlResourceParser. This is why I proposed that it should return what we are already used to (e.g. for references, it is @0xxxxxxxxx, ?0xxxxxxxxx and so on). or deferencing the values using EntryStore. null isn't exactly a helpful output.

I didn't test this on android os, I appreciate if you share me your test result

I'm exclusively testing the library on Android. So, it's working as expected.

[MuntashirAkon]

I have done a bit more analysis on this, and I think you should make geResXmlAttributeAt protected (maybe final) so that it's possible to subclass getAttributeValue methods to output whatever we want (we require lower level access to ResXmlAttribute for handling special cases). This, I think, is better than handling EntryStore in the ResXmlPullParser. Nonetheless, getAttributeValue should never null if it has a value (of any kind). What do you think?

[REAndroid]

it has use-case when a dev is working with XmlPullParser interface rather than XmlResourceParser

Convincing

and I think you should make geResXmlAttributeAt protected (maybe final) so that it's possible to subclass getAttributeValue methods to output whatever we want

I will do on next push, but i don't like protected , final and Immutable things unless it is very necessary for functionality or to avoid confusion resulting from too many public methods, some dev might need extend/override.

Since you are testing on android os, can you test class loader is using class XmlPullParser from our's or frameworks ? You can test simply by add the following method on XmlPullParser

public interface XmlPullParser{
    public static void checkThisClassUsed(){
        throw new RuntimeException("This class is used");
    }
   

Then call method XmlPullParser.checkThisClassUsed() somewhere (e.g application.onCreate) , you will get our exception or MethodNotFound