-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Allow space characters in SimpleName. #712
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
Conversation
DEX format allows Unicode space characters in SimpleName since version 040. Allowed space characters include everything in Unicode category 'Zs': 0x20, 0xa0, 0x1680, 0x2000..0x200a, 0x202f, 0x205f, 0x3000 Smali now supports symtax `method with spaces` (spaces are allowed in backtick-quoted names).
|
Just to clarify, the change in DEX format is not in the android open-source tree yet, but it will land soon. I thought it would be nice if smali allowed to construct such names as well. Backtick syntax is borrowed from kotlin (see e.g. this issue for motivation). |
|
Thanks for the PR! I haven't taken a detailed look at the code yet, but 1 general comment is that the new syntax should only be enabled if smali is assembling a version 40 file. This is normally done by passing API level to smali via the -a option, so, assuming this is going into next year's release (R? 11?), I think that will likely end up being api.. 30? That means, of course, we'll need to plumb the api level into the lexer, and dynamically enable that rule based on that |
|
@JesusFreke Right, this makes sense. I will amend the patch and resubmit. We can't be sure about the API version now, but 30 seems like a fair guess to me. |
|
the new syntax should only be enabled if smali is assembling a version 40
if you allow me the comment, i think that's not good.
i haven't looked at the code, but if new syntax and/or quoting is
introduced into smali, it should be valid for all dex versions. when smaling
for dex < 040, spaces should be rejected. but syntax should be the same for all versions and quoting should be accepted.
|
|
Yeah, that's a fair point. The quoting itself can be applicable to all versions, it's just the spaces in SimpleName that needs to be rejected on < 40 |
|
then maybe spaces whould be rejected during dex writing, which has access to dex version, and smali reading should be dex version agnostic. |
|
Updated the patch to check the API level. I added a new field to |
|
Change landed in ART: https://android.googlesource.com/platform/art/+/ab5f4c17a86b8c808dba862db566f0ffa1146367 |
|
@JesusFreke any followup on this? Is the updated patch better? |
|
Sorry for the delay, I was able to take a look at this today. I noticed that there are some issues around using quoted names in a class descriptor, e.g. I've started some work to expand on your initial implementation to better handle this case, and will likely get it finished and committed (including your 2 commits here) early next week. Thanks! |
|
@JesusFreke great, thank you! |
|
This has been merged thanks again! |
DEX format allows Unicode space characters in SimpleName since version 040.
Allowed space characters include everything in Unicode category 'Zs':
0x20, 0xa0, 0x1680, 0x2000..0x200a, 0x202f, 0x205f, 0x3000
Smali now supports symtax
method with spaces(spaces are allowed inbacktick-quoted names).