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

LEDataInputStream.java is non-free #1166

Closed
apoleon opened this Issue Feb 16, 2016 · 13 comments

Comments

Projects
None yet
5 participants
@apoleon

apoleon commented Feb 16, 2016

Hello,

LEDataInputStream.java has a non-free license.

http://mindprod.com/contact/nonmil.html

"All the software I write is marked, for non-military use only. This would include any use by the military, not just battle. It also includes work by contractors for the military such as interrogation or torture or even managing a payroll."

I'm currently in the process to package apktool for Debian but I can't package apktool as is due to this license. I was told that at least one free alternative exists for this class. Please replace LEDataInputStream.java with this one.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Feb 16, 2016

Owner

Non-free isn't the right word. There is a license on that code and Apktool follows it completely. If you find an alternative little endian input stream. Perhaps there is a completely public domain and/or one in an existing dependency we use, then you are free to make a pull request and I'll review it.

I don't have time to chase and find an alternative, and confirm that all changes match the previous wrapper. Especially when there is a large amount of bug reports to research/fix/patch.

Owner

iBotPeaches commented Feb 16, 2016

Non-free isn't the right word. There is a license on that code and Apktool follows it completely. If you find an alternative little endian input stream. Perhaps there is a completely public domain and/or one in an existing dependency we use, then you are free to make a pull request and I'll review it.

I don't have time to chase and find an alternative, and confirm that all changes match the previous wrapper. Especially when there is a large amount of bug reports to research/fix/patch.

@apoleon

This comment has been minimized.

Show comment
Hide comment
@apoleon

apoleon Feb 16, 2016

Am 16.02.2016 um 15:28 schrieb Connor Tumbleson:

Non-free isn't the right word. There is a license on that code and
Apktool follows it completely. If you find an alternative little endian
input stream. Perhaps there is a completely public domain and/or one in
an existing dependency we use, then you are free to make a pull request
and I'll review it.

I don't have time to chase and find an alternative, and confirm that all
changes match the previous wrapper. Especially when there is a large
amount of bug reports to research/fix/patch.

You might be following the license but by including this class in
Apktool everyone else must follow the license conditions of
LEDDataInputStream.java too as soon as apktool is distributed. That
means for any person or organization that is a contractor of the
military or the military itself that they are not allowed to use apktool
in any "military" context.

This is basically a discrimination against a certain field of endeavor
and that means according to organizations like

OSI:

https://opensource.org/osd

Debian:

https://www.debian.org/social_contract

The Free Software Foundation:

https://www.gnu.org/philosophy/free-sw.html

that LEDataInputStream.java and thus Apktool is not free software
because it needs this class for compilation.

If I can find the free alternative I will make a pull request.

apoleon commented Feb 16, 2016

Am 16.02.2016 um 15:28 schrieb Connor Tumbleson:

Non-free isn't the right word. There is a license on that code and
Apktool follows it completely. If you find an alternative little endian
input stream. Perhaps there is a completely public domain and/or one in
an existing dependency we use, then you are free to make a pull request
and I'll review it.

I don't have time to chase and find an alternative, and confirm that all
changes match the previous wrapper. Especially when there is a large
amount of bug reports to research/fix/patch.

You might be following the license but by including this class in
Apktool everyone else must follow the license conditions of
LEDDataInputStream.java too as soon as apktool is distributed. That
means for any person or organization that is a contractor of the
military or the military itself that they are not allowed to use apktool
in any "military" context.

This is basically a discrimination against a certain field of endeavor
and that means according to organizations like

OSI:

https://opensource.org/osd

Debian:

https://www.debian.org/social_contract

The Free Software Foundation:

https://www.gnu.org/philosophy/free-sw.html

that LEDataInputStream.java and thus Apktool is not free software
because it needs this class for compilation.

If I can find the free alternative I will make a pull request.

@BurgerZ

This comment has been minimized.

Show comment
Hide comment
@BurgerZ

BurgerZ Feb 16, 2016

Contributor

So, we have these options:

  1. Use ByteBuffer which allows order(ByteOrder.LITTLE_ENDIAN)
  2. Use Guava's LittleEndianDataInputStream

But, IMHO, this "non-military" license and complaining about wars, etc. has nothing to do with real world and real software licenses. Bulls**t.

Contributor

BurgerZ commented Feb 16, 2016

So, we have these options:

  1. Use ByteBuffer which allows order(ByteOrder.LITTLE_ENDIAN)
  2. Use Guava's LittleEndianDataInputStream

But, IMHO, this "non-military" license and complaining about wars, etc. has nothing to do with real world and real software licenses. Bulls**t.

@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Feb 18, 2016

@iBotPeaches free here means a Free Software license, i.e. one approved by the Free Software Foundation, Open Source Initiative, Debian-legal, etc. So having that one file with a license that explicitly bans military use makes it so that the whole apktool package cannot be included in Debian, Ubuntu, Fedora, RedHat, Linux Mint, etc. @apoleon has uploaded apktool to Debian without LEDataInputStream but that means is severely disabled. It looks like Guava's LittleEndianDataInputStream should be an easy replacement and is freely licensed.

eighthave commented Feb 18, 2016

@iBotPeaches free here means a Free Software license, i.e. one approved by the Free Software Foundation, Open Source Initiative, Debian-legal, etc. So having that one file with a license that explicitly bans military use makes it so that the whole apktool package cannot be included in Debian, Ubuntu, Fedora, RedHat, Linux Mint, etc. @apoleon has uploaded apktool to Debian without LEDataInputStream but that means is severely disabled. It looks like Guava's LittleEndianDataInputStream should be an easy replacement and is freely licensed.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Feb 18, 2016

Owner

We already have the Guava dependency for other reasons, so I guess it makes sense to use that one. I'm not sure why anyone would upload apktool without an absolutely needed file.

Apktool literally works on a Little Endian basis, without that file the application won't work. There should be no Debian apktool package until this issue is fixed.

Owner

iBotPeaches commented Feb 18, 2016

We already have the Guava dependency for other reasons, so I guess it makes sense to use that one. I'm not sure why anyone would upload apktool without an absolutely needed file.

Apktool literally works on a Little Endian basis, without that file the application won't work. There should be no Debian apktool package until this issue is fixed.

@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Feb 18, 2016

Yes! Let's fix it :-) It looks to me that this is all that is needed, along with removing LEDataInputStream:

diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
index dea33ff..865757d 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
@@ -23,7 +23,7 @@ import brut.androlib.res.data.value.*;
 import brut.util.Duo;
 import brut.androlib.res.data.ResTable;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.*;
 import java.math.BigInteger;
 import java.util.*;
@@ -61,7 +61,7 @@ public class ARSCDecoder {
             mCountIn = null;
             mFlagsOffsets = null;
         }
-        mIn = new ExtDataInput(new LEDataInputStream(arscStream));
+        mIn = new ExtDataInput(new LittleEndianDataInputStream(arscStream));
         mResTable = resTable;
         mKeepBroken = keepBroken;
     }
diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
index 1c768a9..87e65ab 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
@@ -20,7 +20,7 @@ import android.util.TypedValue;
 import brut.androlib.AndrolibException;
 import brut.androlib.res.xml.ResXmlEncoders;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
@@ -69,7 +69,7 @@ public class AXmlResourceParser implements XmlResourceParser {
     public void open(InputStream stream) {
         close();
         if (stream != null) {
-            m_reader = new ExtDataInput(new LEDataInputStream(stream));
+            m_reader = new ExtDataInput(new LittleEndianDataInputStream(stream));
         }
     }

eighthave commented Feb 18, 2016

Yes! Let's fix it :-) It looks to me that this is all that is needed, along with removing LEDataInputStream:

diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
index dea33ff..865757d 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
@@ -23,7 +23,7 @@ import brut.androlib.res.data.value.*;
 import brut.util.Duo;
 import brut.androlib.res.data.ResTable;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.*;
 import java.math.BigInteger;
 import java.util.*;
@@ -61,7 +61,7 @@ public class ARSCDecoder {
             mCountIn = null;
             mFlagsOffsets = null;
         }
-        mIn = new ExtDataInput(new LEDataInputStream(arscStream));
+        mIn = new ExtDataInput(new LittleEndianDataInputStream(arscStream));
         mResTable = resTable;
         mKeepBroken = keepBroken;
     }
diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
index 1c768a9..87e65ab 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
@@ -20,7 +20,7 @@ import android.util.TypedValue;
 import brut.androlib.AndrolibException;
 import brut.androlib.res.xml.ResXmlEncoders;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
@@ -69,7 +69,7 @@ public class AXmlResourceParser implements XmlResourceParser {
     public void open(InputStream stream) {
         close();
         if (stream != null) {
-            m_reader = new ExtDataInput(new LEDataInputStream(stream));
+            m_reader = new ExtDataInput(new LittleEndianDataInputStream(stream));
         }
     }
@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Feb 18, 2016

This is actually what is shipped in the current apktool Debian package:

diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
index bb8fbf5..5f854c2 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
@@ -23,7 +23,7 @@ import brut.androlib.res.data.value.*;
 import brut.util.Duo;
 import brut.androlib.res.data.ResTable;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.*;
 import java.math.BigInteger;
 import java.util.*;
@@ -61,7 +61,7 @@ public class ARSCDecoder {
             mCountIn = null;
             mFlagsOffsets = null;
         }
-        mIn = new ExtDataInput(new LEDataInputStream(arscStream));
+        mIn = new ExtDataInput((InputStream) new LittleEndianDataInputStream(arscStream));
         mResTable = resTable;
         mKeepBroken = keepBroken;
     }
diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
index 1c768a9..889dbc8 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
@@ -20,7 +20,7 @@ import android.util.TypedValue;
 import brut.androlib.AndrolibException;
 import brut.androlib.res.xml.ResXmlEncoders;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
@@ -69,7 +69,7 @@ public class AXmlResourceParser implements XmlResourceParser {
     public void open(InputStream stream) {
         close();
         if (stream != null) {
-            m_reader = new ExtDataInput(new LEDataInputStream(stream));
+            m_reader = new ExtDataInput((InputStream) new LittleEndianDataInputStream(stream));
         }
     }

eighthave commented Feb 18, 2016

This is actually what is shipped in the current apktool Debian package:

diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
index bb8fbf5..5f854c2 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
@@ -23,7 +23,7 @@ import brut.androlib.res.data.value.*;
 import brut.util.Duo;
 import brut.androlib.res.data.ResTable;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.*;
 import java.math.BigInteger;
 import java.util.*;
@@ -61,7 +61,7 @@ public class ARSCDecoder {
             mCountIn = null;
             mFlagsOffsets = null;
         }
-        mIn = new ExtDataInput(new LEDataInputStream(arscStream));
+        mIn = new ExtDataInput((InputStream) new LittleEndianDataInputStream(arscStream));
         mResTable = resTable;
         mKeepBroken = keepBroken;
     }
diff --git a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
index 1c768a9..889dbc8 100644
--- a/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
+++ b/brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
@@ -20,7 +20,7 @@ import android.util.TypedValue;
 import brut.androlib.AndrolibException;
 import brut.androlib.res.xml.ResXmlEncoders;
 import brut.util.ExtDataInput;
-import com.mindprod.ledatastream.LEDataInputStream;
+import com.google.common.io.LittleEndianDataInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
@@ -69,7 +69,7 @@ public class AXmlResourceParser implements XmlResourceParser {
     public void open(InputStream stream) {
         close();
         if (stream != null) {
-            m_reader = new ExtDataInput(new LEDataInputStream(stream));
+            m_reader = new ExtDataInput((InputStream) new LittleEndianDataInputStream(stream));
         }
     }
@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Feb 18, 2016

The key difference is:

  • LEDataInputStream: public final class LEDataInputStream implements DataInput
  • LittleEndianDataInputStream: LittleEndianDataInputStream extends FilterInputStream implements DataInput

eighthave commented Feb 18, 2016

The key difference is:

  • LEDataInputStream: public final class LEDataInputStream implements DataInput
  • LittleEndianDataInputStream: LittleEndianDataInputStream extends FilterInputStream implements DataInput
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Feb 18, 2016

Owner

I was not aware the current implementation was a 1 to 1 match for the Guava implementation. I need to do some testing on my own, but thanks for the diff report.

Owner

iBotPeaches commented Feb 18, 2016

I was not aware the current implementation was a 1 to 1 match for the Guava implementation. I need to do some testing on my own, but thanks for the diff report.

@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Feb 18, 2016

I tried changing the cast to (DataInput) but that fails with this stacktrace:

$ apktool  d LocationPrivacy-0.3.apk 
 17:33:03 up 2 days, 20:49,  0 users,  load average: 1.56, 1.78, 2.20
USER     TTY      FROM             LOGIN@   IDLE   JCPU   PCPU WHAT
I: Using Apktool 2.0.2-dirty on LocationPrivacy-0.3.apk
I: Loading resource table...
Exception in thread "main" brut.androlib.AndrolibException: Could not decode arsc file
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:52)
    at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:544)
    at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:63)
    at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:55)
    at brut.androlib.Androlib.getResTable(Androlib.java:65)
    at brut.androlib.ApkDecoder.setTargetSdkVersion(ApkDecoder.java:197)
    at brut.androlib.ApkDecoder.decode(ApkDecoder.java:96)
    at brut.apktool.Main.cmdDecode(Main.java:165)
    at brut.apktool.Main.main(Main.java:81)
Caused by: java.io.IOException: Expected: 0x00000008, got: 0x00000800
    at brut.util.ExtDataInput.skipCheckShort(ExtDataInput.java:56)
    at brut.androlib.res.decoder.ARSCDecoder.readValue(ARSCDecoder.java:254)
    at brut.androlib.res.decoder.ARSCDecoder.readEntry(ARSCDecoder.java:203)
    at brut.androlib.res.decoder.ARSCDecoder.readConfig(ARSCDecoder.java:191)
    at brut.androlib.res.decoder.ARSCDecoder.readType(ARSCDecoder.java:159)
    at brut.androlib.res.decoder.ARSCDecoder.readPackage(ARSCDecoder.java:116)
    at brut.androlib.res.decoder.ARSCDecoder.readTable(ARSCDecoder.java:78)
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:47)
    ... 8 more

eighthave commented Feb 18, 2016

I tried changing the cast to (DataInput) but that fails with this stacktrace:

$ apktool  d LocationPrivacy-0.3.apk 
 17:33:03 up 2 days, 20:49,  0 users,  load average: 1.56, 1.78, 2.20
USER     TTY      FROM             LOGIN@   IDLE   JCPU   PCPU WHAT
I: Using Apktool 2.0.2-dirty on LocationPrivacy-0.3.apk
I: Loading resource table...
Exception in thread "main" brut.androlib.AndrolibException: Could not decode arsc file
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:52)
    at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:544)
    at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:63)
    at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:55)
    at brut.androlib.Androlib.getResTable(Androlib.java:65)
    at brut.androlib.ApkDecoder.setTargetSdkVersion(ApkDecoder.java:197)
    at brut.androlib.ApkDecoder.decode(ApkDecoder.java:96)
    at brut.apktool.Main.cmdDecode(Main.java:165)
    at brut.apktool.Main.main(Main.java:81)
Caused by: java.io.IOException: Expected: 0x00000008, got: 0x00000800
    at brut.util.ExtDataInput.skipCheckShort(ExtDataInput.java:56)
    at brut.androlib.res.decoder.ARSCDecoder.readValue(ARSCDecoder.java:254)
    at brut.androlib.res.decoder.ARSCDecoder.readEntry(ARSCDecoder.java:203)
    at brut.androlib.res.decoder.ARSCDecoder.readConfig(ARSCDecoder.java:191)
    at brut.androlib.res.decoder.ARSCDecoder.readType(ARSCDecoder.java:159)
    at brut.androlib.res.decoder.ARSCDecoder.readPackage(ARSCDecoder.java:116)
    at brut.androlib.res.decoder.ARSCDecoder.readTable(ARSCDecoder.java:78)
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:47)
    ... 8 more
@eighthave

This comment has been minimized.

Show comment
Hide comment
@eighthave

eighthave Mar 21, 2016

hey @iBotPeaches any luck with this? The window is closing probably this week for getting this into the next Ubuntu release (16.04). This issue is literally the only blocker.

Turns out that the guava implementation is not a direct replacement, but it should not be hard to use the guava one.

eighthave commented Mar 21, 2016

hey @iBotPeaches any luck with this? The window is closing probably this week for getting this into the next Ubuntu release (16.04). This issue is literally the only blocker.

Turns out that the guava implementation is not a direct replacement, but it should not be hard to use the guava one.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 21, 2016

Owner

Probably not going to make it for that release window. I would delay it till 16.10, possibly even further. This is not a simple replace or I'm missing something easy.

The first unit test to run is the AndResGuard test, and we can examine the file it's about to read. The first set of bytes is

02 00 0C 00 74 04 00 00 01 00 00 00

Or broken down

Original Parsed Cleaned Decimal Field
02 00 00 02 0x2 2 type
0C 00 00 0C 0xC 12 headerSize
74 04 00 00 00 00 04 74 0x474 1140 size
01 00 00 00 00 00 00 01 0x1 1 packageCount

Our current implementation reads the first two bytes for a short and then shifts them into place to 0x0002 or 2. This is what we are expecting - TYPE_TABLE = 0x0002.

The next implementation, ignoring everything but the returned value from getShort() we get 0x200 or 512. This is not right, at least with my understanding of a little endian reader. The amount of rewriting required to match this implement is not worth the effort.

As I said though, I might be implement Guava incorrectly, thus causing this incorrect behavior. As I'm not in a hurry to change something that isn't broken, I'm just going to sit on this. Working pull requests that pass unit tests will be accepted.

Owner

iBotPeaches commented Mar 21, 2016

Probably not going to make it for that release window. I would delay it till 16.10, possibly even further. This is not a simple replace or I'm missing something easy.

The first unit test to run is the AndResGuard test, and we can examine the file it's about to read. The first set of bytes is

02 00 0C 00 74 04 00 00 01 00 00 00

Or broken down

Original Parsed Cleaned Decimal Field
02 00 00 02 0x2 2 type
0C 00 00 0C 0xC 12 headerSize
74 04 00 00 00 00 04 74 0x474 1140 size
01 00 00 00 00 00 00 01 0x1 1 packageCount

Our current implementation reads the first two bytes for a short and then shifts them into place to 0x0002 or 2. This is what we are expecting - TYPE_TABLE = 0x0002.

The next implementation, ignoring everything but the returned value from getShort() we get 0x200 or 512. This is not right, at least with my understanding of a little endian reader. The amount of rewriting required to match this implement is not worth the effort.

As I said though, I might be implement Guava incorrectly, thus causing this incorrect behavior. As I'm not in a hurry to change something that isn't broken, I'm just going to sit on this. Working pull requests that pass unit tests will be accepted.

chirayudesai added a commit to chirayudesai/Apktool that referenced this issue Mar 23, 2016

Replace little endian data input stream implementation
* Drop LEDataInputStream (which had a restrictive license)
  with LittleEndianDataInputStream, which is public domain.

A minor change has been made to the new class, removing
the interitance of InputStream.
This makes it's behaviour indentical to the previous implementation,
and unit tests pass.

Fixes #1166
Source: http://www.peterfranza.com/2008/09/26/little-endian-input-stream/
@chirayudesai

This comment has been minimized.

Show comment
Hide comment
@chirayudesai

chirayudesai Mar 23, 2016

Contributor

Hi, I came across a public domain implementation of the class when looking for alternatives other than guava, and with a small change (removing 'extends InputStream) it works. Unit tests pass.

Contributor

chirayudesai commented Mar 23, 2016

Hi, I came across a public domain implementation of the class when looking for alternatives other than guava, and with a small change (removing 'extends InputStream) it works. Unit tests pass.

@iBotPeaches iBotPeaches closed this in #1201 Mar 24, 2016

amorris13 pushed a commit to amorris13/Apktool that referenced this issue Aug 5, 2016

Anthony Morris
Use Guava's LittleEndianDataInputStream.
This replaces the custom LittleEndianDataInputStream with
guava's implementation. To do this, I had to fix ExtDataInput
to better handle the case where skipBytes doesn't skip all the
bytes (the tests failed without this, and succeed with it). This
appears to be the main difference between the two implementations.
Guava's implementation is preferred because it is already a
dependency and because its license is clearer (the previous
implementation had a vague "public domain" comment in the thread
which may not be legally sufficient).

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