Whitespace collapsing, Backslash escaping, Quoted escaping not implemented

Gbr22 commented 11 months ago

Describe the bug Whitespace collapsing, Backslash escaping, Quoted escaping not implemented in APKEditor See this link for how it should behave: https://developer.android.com/guide/topics/resources/string-resource#escaping_quotes

Build examples:

Input string given to APKEditor to build	Expected text displayed in app	Current text displayed in app
`Whitespace collapsed`	Whitespace collapsed	Whitespace collapsed
`Unicode: \u00A9`	Unicode: ©	Unicode: \u00A9
`"That's cool!"`	That's cool!	"That's cool!"
`That's cool!`	`Compile Error`	That's cool!
`Backslash (\\) (\)`	Backslash (\) ()	Backslash (\\) (\)

Decompile example: (❌ incorrect, ✅ correct)

String in android studio	Expected string extracted from apk (mutiple correct examples)	String extracted from apk by APKEditor	String Extracted from apk by apktool
`"Tab ulated"`	`Tab\u0009ulated` `Tab\tulated` `"Tab ulated"`	❌ `Tab ulated`	✅ `Tab\u0009ulated`
`\'single\'`	`\'single\'` `"'single'"` `"\'single\'"`	❌ `'single'`	✅ `"'single'"`
`"\"double\""`	`"\"double\""` `\"double\"`	❌ `"double"`	✅ `\"double\"`
`(\\) (\)`	`(\\) ()`	❌ `(\) ()`	✅ `(\\) ()`
`\u00A9`	`\u00A9` `©` `©`	✅ `©`	✅ `©`

To Reproduce Steps to reproduce the behavior:

Used version '1.2.6'
Operating system 'Linux 5.15.90.1-microsoft-standard-WSL2 x86_64 GNU/Linux'
Commands java -jar APKEditor-1.2.6.jar d -dex -i app-debug.apk and java -jar APKEditor-1.2.6.jar b -i app-debug_decompile_xml -o packed.apk

See the original strings used to build the apk at strings (original) See the expected output in image strings (original) built with android studio I also attached the output produced by apktool for reference strings (apktool decompilation output). The decompiled strings created by apktool are different than strings (original) but that is not a problem, as both XML files produce the same apk as seen on images strings (original) built with android studio and strings (apktool decompilation output) built with android studio

Problem 1: The output produced by unpacking with APKEditor is not compatible with the format that other tools such as android studio expect.

To reproduce unpack the attached apk. The unpacked strings.xml is not compatible with the format that Android Studio accepts. When trying to build my example apk I get an error due to the unescaped single quotes '. After escaping the single quotes with a backslash \ and building the app I get the output seen in the attached image strings (APKEditor decompilation output manually edited to escape single quotes) built with android studio.

Problem 2: The APKEditor packer doesn't recognise backslash `\` escape codes and double quoted escapes as its input.

To reproduce unpack the attached apk. Edit the strings.xml to the attached strings (original) then pack/build with APKEditor. The built apk will produce the output shown in the image strings (original) packed with APKEditor with is not correct.

Used apk file app-debug.apk.zip

Attached XML

strings (original)

```xml Android escape and quotation test \n"Tab\tulated 1" \n"Tab ulated 2" \nTab\tulated 3 \nTab ulated 4 \nBackslash (\\) (\) \nQuestion\? \n"\"Double\"" \"quotes\" \n\'Single\' "'quote'" "\'test\'" \nmail\@example.com \nUnicode: \u00A9 \n"Whitespace Preserved" \nWhitespace collapsed \n"Newlines preserved" \nNewlines\n\nPreserved \nNewlines collapsed ```

strings (APKEditor decompilation output)

```xml Android escape and quotation test Tab ulated 1 Tab ulated 2 Tab ulated 3 Tab ulated 4 Backslash (\) () Question? "Double" "quotes" 'Single' 'quote' 'test' mail@example.com Unicode: © Whitespace Preserved Whitespace collapsed Newlines preserved Newlines Preserved Newlines collapsed ```

strings (APKEditor decompilation output manually edited to escape single quotes)

```xml Android escape and quotation test Tab ulated 1 Tab ulated 2 Tab ulated 3 Tab ulated 4 Backslash (\) () Question? "Double" "quotes" \'Single\' \'quote\' \'test\' mail@example.com Unicode: © Whitespace Preserved Whitespace collapsed Newlines preserved Newlines Preserved Newlines collapsed ```

strings (apktool decompilation output)

```xml "Android escape and quotation test Tab\u0009ulated 1 Tab\u0009ulated 2 Tab\u0009ulated 3 Tab ulated 4 Backslash (\\) () Question? \"Double\" \"quotes\" 'Single' 'quote' 'test' mail@example.com Unicode: © Whitespace Preserved Whitespace collapsed Newlines preserved Newlines Preserved Newlines collapsed " ```

Attached images

strings (original) built with android studio

![image](https://github.com/REAndroid/APKEditor/assets/25722827/86d4f006-e259-4e44-b28b-d8b5defa63cc)

strings (original) built with APKEditor

![image](https://github.com/REAndroid/APKEditor/assets/25722827/ead7774b-6350-4db7-8f9e-9418179714e6)

strings (apktool decompilation output) built with android studio

![image](https://github.com/REAndroid/APKEditor/assets/25722827/05d160b2-5b4d-4ba3-a041-7b8c86c25e5f)

strings (APKEditor decompilation output manually edited to escape single quotes) built with android studio

![image](https://github.com/REAndroid/APKEditor/assets/25722827/15e4e0d0-44b7-42ce-874d-5a98c720ff02)

REAndroid commented 11 months ago

Very impressive! I will take a look at it and get back to you. Thank you!

REAndroid commented 11 months ago

Thank you again for such professional issue reporting!

Let's start with

Input string given to APKEditor to build Expected text displayed in app Current text displayed in app

That's cool! Compile Error That's cool!

Input string given to APKEditor to build	Expected text displayed in app	Current text displayed in app
`That's cool!`	`Compile Error`	That's cool!

APKEditor As the name suggested it is intended primarily for editing, modifying & analyzing apk files but not limited to. We are in favor of the rules of android OS parser and not aapt/aapt2 . Anything logical you placed on APKEditor will be parsed as-it-is/literally for example you are free to place on layout attribute android:layout_width="Some text here" then APKEditor will compile without errors but its your responsibility to know this value may cause runtime errors on layout renderer. Surprisingly it's valid to have weird type names <hello name="app_name">My app name</hello> . Even it's valid to place style bags under attr
```
<attr name="MyAppTheme" parent="@android:style/Theme.NoTitleBar">
<item name="android:windowBackground">@android:color/transparent</item>
<item name="android:windowNoTitle">true</item>
</attr>
You can consume this somewhere as android:theme="@attr/MyAppTheme"
```

We already implemented escaping crucial characters like @?#, consider you have literal string entry <string name="abcd">\@meet/me</string> for such cases it's mandatory to escape @ otherwise our parser will interpret it as reference and looks for resource under type meet entry name me then will end up throwing error Local entry not found

I agree with you to follow the standard of what majority of developers are used to. I will try to implement all what you suggested and see if it doesn't affect the points i mentioned.

Since you already made this test code, could you please include styled (spannable) strings ? I really confused with xml & html things and I need help. For example

<string name="abc"><font color="#FF0000" size="large">Hello World</font></string>
<string name="abc"><font color=#FF0000;size=large>Hello World</font></string>

Gbr22 commented 11 months ago

Hi! I've updated my test application to include spannable strings.

Here is the new apk: app-debug.apk.zip

And can also find the source code here: https://github.com/Gbr22/APKEditorTest

I don't have an issue with APKEditor/ARSCLib offering a superset of the android strings XML format. (By superset I mean having a more "relaxed" parser that doesn't throw an error for unescaped single quotes ' for example).

But when generating XML, I think it should be done in a way that doesn't cause other tools to throw an error. For example when ARSCLib generates a file that contains the string That's cool!, it should escape it (to either That\'s cool! or "That's cool!") with the assuption that when the file is used in other programs (such as Android Studio), it might throw an exception for the unescaped That's cool!.

As for the xml & html things Chrome parses it like so:

<font color=#FF0000;size=large>Hello World</font> becomes <font color="#FF0000;size=large">Hello World</font>`

<font color=#FF0000; size=large>Hello World</font> becomes <font color="#FF0000;" size="large">Hello World</font>

<font color="#FF0000;"size="large">Hello World</font> becomes <font color="#FF0000;" size="large">Hello World</font>

Android Studio errors if you try to have an attribute without double quotes.

If the goal is to have the parser be as relaxed as possible I would have it parse similar to how chrome parses it. So a tag is allowed to have an attribute without the double quotes, but in that case a space ends the attribute.

REAndroid commented 11 months ago

I agree with your points, this also benefit us to some degree but the current literal-string way has also huge benefit. I am thinking of keeping both methods by introducing global switch -aapt-mode for selecting between escaped strings & literal-strings

Gbr22 commented 11 months ago

I guess the switch may be a solution that makes both crowds happy.

REAndroid / APKEditor