Skip to content

Conversation

IgorEisberg
Copy link
Contributor

This trick gets rid of unneeded flags in flag attrs that even contextually don't make sense.
Most commonly: android:layout_gravity="bottom|center|left"

000000000000000000000011 (left)
000000000000000000010001 (center)
000000000000000001010000 (bottom)

left|bottom already covers the bits set by center, and contextually something can't be left, bottom and center all at once.
There are very few flag attrs like that, where certain flags overlap, but this fixes this annoyance when editing res XMLs.

This trick gets rid of unneeded flags in flag attrs that even
contextually don't make sense.
Most commonly: android:layout_gravity="bottom|center|left"

000000000000000000000011 (left)
000000000000000000010001 (center)
000000000000000001010000 (bottom)

'left|bottom' already covers the bits set by 'center',
and contextually something can't be left, bottom and center
all at once. There are very few flag attrs like that, where
certain flags overlap, but this fixes this annoyance when
editing res XMLs.
@iBotPeaches
Copy link
Owner

Could you provide some background on why this happens? Is Apktool just disassembly what the source application was and this is an improvement? Since generally for static analysis Apktool should match exactly what source is.

What are the changes for network-security-config for?

@IgorEisberg
Copy link
Contributor Author

Could you provide some background on why this happens? Is Apktool just disassembly what the source application was and this is an improvement? Since generally for static analysis Apktool should match exactly what source is.

For some flag attrs, apktool can't perfectly recreate the flag combination that was originally used. The binary XML stores the value as an integer (a bit field), and for most flag attrs it's easy to extract the flags with a simple AND operation. However for some flag attrs defined by Android itself there are flags with overlapping bits: gravity, layout_gravity, and a few others. Your previous method isSubpartOf checked if a flag is a subset of a previously added flag, but not if it contributes any new bits to the combination, resulting in flags that were not originally there to be included. Like in my example above, both bottom|center|left and bottom|left result in the same stored integer.

What are the changes for network-security-config for?

Just so the test is very specific. Using // in XPath will allow false positives, it should be used sparsely, not when a concrete match is searched for.

@iBotPeaches
Copy link
Owner

Thanks - must have predated my work on the tool, since didn't even realize that. I think I could sit down with your example and expand a simple unit test to confirm this works and doesn't regress.

@IgorEisberg
Copy link
Contributor Author

For reference, the output from a demo run I did for gravity attr, value = 0b100000000000000001010101:

All Flags (Sorted):
000000000000000001110111 (fill)
000000000000000000000111 (fill_horizontal)
000000000000000001110000 (fill_vertical)
100000000000000000000011 (start)
100000000000000000000101 (end)
000000000000000000000011 (left)
000000000000000000000101 (right)
000000000000000000010001 (center)
000000000000000000110000 (top)
000000000000000001010000 (bottom)
000000000000000000000001 (center_horizontal)
000000000000000000001000 (clip_horizontal)
000000000000000000010000 (center_vertical)
000000000000000010000000 (clip_vertical)

Selected Flags:
100000000000000000000101 (end)
000000000000000000010001 (center)
000000000000000001010000 (bottom)

Filtered Flags:
100000000000000000000101 (end)
000000000000000001010000 (bottom)

Decoded:
100000000000000001010101 (end|bottom)

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just want to get a unit test in for this shortening logic before merge.

@iBotPeaches
Copy link
Owner

Don't have time for a test myself. Just merging as I trust your judgement.

@iBotPeaches iBotPeaches merged commit 2086d80 into iBotPeaches:master Apr 28, 2025
25 checks passed
iBotPeaches added a commit to iBotPeaches/apktool.org that referenced this pull request Apr 28, 2025
@IgorEisberg IgorEisberg deleted the flagsfix branch April 29, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants