Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitespace collapsing, Backslash escaping, Quoted escaping not implemented #51

Open
Gbr22 opened this issue Jul 16, 2023 · 5 comments
Open
Labels
good first issue Good for newcomers

Comments

@Gbr22
Copy link

Gbr22 commented Jul 16, 2023

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:

  1. Used version '1.2.6'
  2. Operating system 'Linux 5.15.90.1-microsoft-standard-WSL2 x86_64 GNU/Linux'
  3. 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)

    <string name="test_string">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
</string>

strings (APKEditor decompilation output)

  <string name="test_string">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 </string>
</resources>

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

  <string name="test_string">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 </string>
</resources>

strings (apktool decompilation output)

    <string name="test_string">"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 "</string>

Attached images

strings (original) built with android studio

image

strings (original) built with APKEditor

image

strings (apktool decompilation output) built with android studio

image

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

image

@REAndroid REAndroid added the good first issue Good for newcomers label Jul 16, 2023
@REAndroid
Copy link
Owner

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

@REAndroid REAndroid pinned this issue Jul 16, 2023
@REAndroid
Copy link
Owner

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!
  • 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
Copy link
Author

Gbr22 commented Jul 17, 2023

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
Copy link
Owner

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
Copy link
Author

Gbr22 commented Jul 24, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants