Repository: commons-compress
Updated Branches:
  refs/heads/master 32c30f6f0 -> 52dd5908e


COMPRESS-363 properly handle overflow inside BitInputStream


Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/52dd5908
Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/52dd5908
Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/52dd5908

Branch: refs/heads/master
Commit: 52dd5908e374973d69c51856b74d4d93d591c90a
Parents: 32c30f6
Author: Stefan Bodewig <bode...@apache.org>
Authored: Fri Jul 1 21:37:24 2016 +0200
Committer: Stefan Bodewig <bode...@apache.org>
Committed: Fri Jul 1 21:37:24 2016 +0200

----------------------------------------------------------------------
 src/changes/changes.xml                         |  5 ++
 .../commons/compress/utils/BitInputStream.java  | 40 ++++++++++++---
 .../compress/utils/BitInputStreamTest.java      | 51 ++++++++++++++++++++
 3 files changed, 90 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-compress/blob/52dd5908/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a6d5e47..62b0f8a 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -47,6 +47,11 @@ The <action> type attribute can be add,update,fix,remove.
       <action issue="COMPRESS-360" type="update" date="2016-06-25" 
dev="ggregory">
         Update Java requirement from 6 to 7.
       </action>
+      <action issue="COMPRESS-363" type="fix" date="2016-07-01">
+        BitInputStream could return bad results when overflowing
+        internally - if two consecutive reads tried to read more than
+        64 bits.
+      </action>
     </release>
     <release version="1.12" date="2016-06-21"
              description="Release 1.12 - API compatible to 1.11 but requires 
Java 6 at runtime.

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/52dd5908/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
----------------------------------------------------------------------
diff --git 
a/src/main/java/org/apache/commons/compress/utils/BitInputStream.java 
b/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
index b34590e..35773b7 100644
--- a/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
+++ b/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
@@ -82,7 +82,7 @@ public class BitInputStream implements Closeable {
         if (count < 0 || count > MAXIMUM_CACHE_SIZE) {
             throw new IllegalArgumentException("count must not be negative or 
greater than " + MAXIMUM_CACHE_SIZE);
         }
-        while (bitsCachedSize < count) {
+        while (bitsCachedSize < count && bitsCachedSize < 57) {
             final long nextByte = in.read();
             if (nextByte < 0) {
                 return nextByte;
@@ -95,15 +95,43 @@ public class BitInputStream implements Closeable {
             }
             bitsCachedSize += 8;
         }
+        int overflowBits = 0;
+        long overflow = 0l;
+        if (bitsCachedSize < count) {
+            // bitsCachedSize >= 57 and left-shifting it 8 bits would cause an 
overflow
+            int bitsToAddCount = count - bitsCachedSize;
+            overflowBits = 8 - bitsToAddCount;
+            final long nextByte = in.read();
+            if (nextByte < 0) {
+                return nextByte;
+            }
+            if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
+                long bitsToAdd = nextByte & MASKS[bitsToAddCount];
+                bitsCached |= (bitsToAdd << bitsCachedSize);
+                overflow = (nextByte >>> bitsToAddCount) & MASKS[overflowBits];
+            } else {
+                bitsCached <<= bitsToAddCount;
+                long bitsToAdd = (nextByte >>> (overflowBits)) & 
MASKS[bitsToAddCount];
+                bitsCached |= bitsToAdd;
+                overflow = nextByte & MASKS[overflowBits];
+            }
+            bitsCachedSize = count;
+        }
         
         final long bitsOut;
-        if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
-            bitsOut = (bitsCached & MASKS[count]);
-            bitsCached >>>= count;
+        if (overflowBits == 0) {
+            if (byteOrder == ByteOrder.LITTLE_ENDIAN) {
+                bitsOut = (bitsCached & MASKS[count]);
+                bitsCached >>>= count;
+            } else {
+                bitsOut = (bitsCached >> (bitsCachedSize - count)) & 
MASKS[count];
+            }
+            bitsCachedSize -= count;
         } else {
-            bitsOut = (bitsCached >> (bitsCachedSize - count)) & MASKS[count];
+            bitsOut = bitsCached & MASKS[count];
+            bitsCached = overflow;
+            bitsCachedSize = overflowBits;
         }
-        bitsCachedSize -= count;
         return bitsOut;
     }
 }

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/52dd5908/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
----------------------------------------------------------------------
diff --git 
a/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java 
b/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
index 6f90af3..11ff956 100644
--- a/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
@@ -118,6 +118,57 @@ public class BitInputStreamTest {
         bis.close();
     }
 
+    /**
+     * @see "https://issues.apache.org/jira/browse/COMPRESS-363";
+     */
+    @Test
+    public void littleEndianWithOverflow() throws Exception {
+        ByteArrayInputStream in = new ByteArrayInputStream(new byte[] {
+                87, // 01010111
+                45, // 00101101
+                66, // 01000010
+                15, // 00001111
+                90, // 01011010
+                29, // 00011101
+                88, // 01011000
+                61, // 00111101
+                33, // 00100001
+                74  // 01001010
+            });
+        BitInputStream bin = new BitInputStream(in, ByteOrder.LITTLE_ENDIAN);
+        assertEquals(23, // 10111
+                     bin.readBits(5));
+        assertEquals(714595605644185962l, // 
0001-00111101-01011000-00011101-01011010-00001111-01000010-00101101-010
+                     bin.readBits(63));
+        assertEquals(1186, // 01001010-0010
+                     bin.readBits(12));
+        assertEquals(-1 , bin.readBits(1));
+    }
+
+    @Test
+    public void bigEndianWithOverflow() throws Exception {
+        ByteArrayInputStream in = new ByteArrayInputStream(new byte[] {
+                87, // 01010111
+                45, // 00101101
+                66, // 01000010
+                15, // 00001111
+                90, // 01011010
+                29, // 00011101
+                88, // 01011000
+                61, // 00111101
+                33, // 00100001
+                74  // 01001010
+            });
+        BitInputStream bin = new BitInputStream(in, ByteOrder.BIG_ENDIAN);
+        assertEquals(10, // 01010
+                     bin.readBits(5));
+        assertEquals(8274274654740644818l, 
//111-00101101-01000010-00001111-01011010-00011101-01011000-00111101-0010
+                     bin.readBits(63));
+        assertEquals(330, // 0001-01001010
+                     bin.readBits(12));
+        assertEquals(-1 , bin.readBits(1));
+    }
+
     private ByteArrayInputStream getStream() {
         return new ByteArrayInputStream(new byte[] {
                 (byte) 0xF8,  // 11111000

Reply via email to