Files extraction problem #921

Closed
Furniel opened this Issue Apr 21, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@Furniel
Contributor

Furniel commented Apr 21, 2015

Apktool 2.0.0 don`t extracts files without extension. For example Google Play Services raw folder contains:
gtm_analytics
indexes.txt
maps_dav_k2.bin
oss_notice.txt
pinyins.txt
pulsar_key_file

Latest version of apktool don`t extracts gtm_analytics & pulsar_key_file but creates raws.xml in values folder:

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <item type="raw" name="gtm_analytics">res/raw/gtm_analytics</item>
    <item type="raw" name="pulsar_key_file">res/raw/pulsar_key_file</item>
</resources>

Apk can`t be compiled without manual adding of this files and removing raws.xml.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Apr 21, 2015

Owner

If by raw you mean res/raw. I haven't touched that block of code in a long long time, but then again I have no unit-test for a no extension file there.

I've also never heard of aapt creating a raws.xml file. Basically, I need to investigate this. Thanks for the tip!

Owner

iBotPeaches commented Apr 21, 2015

If by raw you mean res/raw. I haven't touched that block of code in a long long time, but then again I have no unit-test for a no extension file there.

I've also never heard of aapt creating a raws.xml file. Basically, I need to investigate this. Thanks for the tip!

@iBotPeaches iBotPeaches added this to the v2.0.1 milestone Apr 22, 2015

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Apr 22, 2015

Owner

This is regression from here: bbc6023

However, I'm not sure of a fix yet.

res/foo in some instances is a string literally named res/foo. In the case of many Japanese applications in order to break apktool. The fix above was looking for a . in the string, so it could cast it to a ResFileValue. This breaks files with no extensions, but there is no way to detect a bogus entry from real one at that point.

Checking if each string exists in the filesystem is slow as hell. Still investigating a proper fix.

Owner

iBotPeaches commented Apr 22, 2015

This is regression from here: bbc6023

However, I'm not sure of a fix yet.

res/foo in some instances is a string literally named res/foo. In the case of many Japanese applications in order to break apktool. The fix above was looking for a . in the string, so it could cast it to a ResFileValue. This breaks files with no extensions, but there is no way to detect a bogus entry from real one at that point.

Checking if each string exists in the filesystem is slow as hell. Still investigating a proper fix.

@Furniel

This comment has been minimized.

Show comment
Hide comment
@Furniel

Furniel Apr 22, 2015

Contributor

I`m not really sure that it will be true for all applications, but at this moment i found files without extension only in raw folder, so maybe we can fix this bug by this code

if ((value.startsWith("res/") && value.contains(".")) || value.startsWith("res/raw")) {

and to be sure that it`s not a string we can add something like this:

if ((value.startsWith("res/") && value.contains(".")) || value.startsWith("res/raw")) {
    File f = new File(value); //not sure that we can check if string exists in the filesystem in such way
    if(value.startsWith("res/raw") && !f.exists()) {
        return new ResStringValue(value);
    }
    else
    {
        return new ResFileValue(value);
    }
}
Contributor

Furniel commented Apr 22, 2015

I`m not really sure that it will be true for all applications, but at this moment i found files without extension only in raw folder, so maybe we can fix this bug by this code

if ((value.startsWith("res/") && value.contains(".")) || value.startsWith("res/raw")) {

and to be sure that it`s not a string we can add something like this:

if ((value.startsWith("res/") && value.contains(".")) || value.startsWith("res/raw")) {
    File f = new File(value); //not sure that we can check if string exists in the filesystem in such way
    if(value.startsWith("res/raw") && !f.exists()) {
        return new ResStringValue(value);
    }
    else
    {
        return new ResFileValue(value);
    }
}
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Apr 22, 2015

Owner

The first fix would work in theory, but then it would be a matter of time until apk authors would adapt their bogus strings to be res/raw/foo and then we are back to a broken apktool.

I think the best fix is for every ResFileValue we expect, we check the filesystem for it. It may slow down execution, but it prevents errors. The only problem is at this point in execution we don't have the variables needed to check the filesystem. I will work on a small refactor to adapt this.

Owner

iBotPeaches commented Apr 22, 2015

The first fix would work in theory, but then it would be a matter of time until apk authors would adapt their bogus strings to be res/raw/foo and then we are back to a broken apktool.

I think the best fix is for every ResFileValue we expect, we check the filesystem for it. It may slow down execution, but it prevents errors. The only problem is at this point in execution we don't have the variables needed to check the filesystem. I will work on a small refactor to adapt this.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches May 10, 2015

Owner

Okay, the IO time for checking the filesystem is not an option. I think I'm overthinking this.

I might just wait for the ClassCastException, if it exists then we can force a resStringValue instead of resFileValue.

Owner

iBotPeaches commented May 10, 2015

Okay, the IO time for checking the filesystem is not an option. I think I'm overthinking this.

I might just wait for the ClassCastException, if it exists then we can force a resStringValue instead of resFileValue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment