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