IMO exceptions messages should start with a capital letter, like a sentence.
Gary On Thu, Aug 8, 2019, 12:29 <bode...@apache.org> wrote: > This is an automated email from the ASF dual-hosted git repository. > > bodewig pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/commons-compress.git > > > The following commit(s) were added to refs/heads/master by this push: > new 5836405 COMPRESS-490 throw IOException for certain malformed > archives > 5836405 is described below > > commit 58364058bd2903ebd1568f6df22161e142a0dc86 > Author: Stefan Bodewig <bode...@apache.org> > AuthorDate: Thu Aug 8 18:28:41 2019 +0200 > > COMPRESS-490 throw IOException for certain malformed archives > --- > src/changes/changes.xml | 4 +++ > .../lz4/BlockLZ4CompressorInputStream.java | 12 ++++++++- > .../AbstractLZ77CompressorInputStream.java | 28 > ++++++++++++++++++- > .../snappy/SnappyCompressorInputStream.java | 31 > +++++++++++++++++++--- > 4 files changed, 70 insertions(+), 5 deletions(-) > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index cd51c61..b41a411 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -92,6 +92,10 @@ The <action> type attribute can be > add,update,fix,remove. > gathered output in the same order they have been added. > Github Pull Requests #78 and #79. > </action> > + <action type="fix" date="2019-08-08" issue="COMPRESS-490"> > + Throw IOException rather than RuntimeExceptions for certain > + malformed LZ4 or Snappy inputs. > + </action> > </release> > <release version="1.18" date="2018-08-16" > description="Release 1.18"> > diff --git > a/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorInputStream.java > b/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorInputStream.java > index a52dc60..493ec4e 100644 > --- > a/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorInputStream.java > +++ > b/src/main/java/org/apache/commons/compress/compressors/lz4/BlockLZ4CompressorInputStream.java > @@ -100,6 +100,9 @@ public class BlockLZ4CompressorInputStream extends > AbstractLZ77CompressorInputSt > if (literalSizePart == BACK_REFERENCE_SIZE_MASK) { > literalSizePart += readSizeBytes(); > } > + if (literalSizePart < 0) { > + throw new IOException("illegal block with a negative literal > size found"); > + } > startLiteral(literalSizePart); > state = State.IN_LITERAL; > } > @@ -136,7 +139,14 @@ public class BlockLZ4CompressorInputStream extends > AbstractLZ77CompressorInputSt > backReferenceSize += readSizeBytes(); > } > // minimal match length 4 is encoded as 0 > - startBackReference(backReferenceOffset, backReferenceSize + 4); > + if (backReferenceSize < 0) { > + throw new IOException("illegal block with a negative match > length found"); > + } > + try { > + startBackReference(backReferenceOffset, backReferenceSize + > 4); > + } catch (IllegalArgumentException ex) { > + throw new IOException("illegal block with bad offset found", > ex); > + } > state = State.IN_BACK_REFERENCE; > return true; > } > diff --git > a/src/main/java/org/apache/commons/compress/compressors/lz77support/AbstractLZ77CompressorInputStream.java > b/src/main/java/org/apache/commons/compress/compressors/lz77support/AbstractLZ77CompressorInputStream.java > index 31196ef..ed0cb3c 100644 > --- > a/src/main/java/org/apache/commons/compress/compressors/lz77support/AbstractLZ77CompressorInputStream.java > +++ > b/src/main/java/org/apache/commons/compress/compressors/lz77support/AbstractLZ77CompressorInputStream.java > @@ -130,9 +130,13 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > * Size of the window kept for back-references, must be > bigger than the biggest offset expected. > * > * @throws IOException if reading fails > + * @throws IllegalArgumentException if windowSize is not bigger than 0 > */ > public AbstractLZ77CompressorInputStream(final InputStream is, int > windowSize) throws IOException { > this.in = new CountingInputStream(is); > + if (windowSize <= 0) { > + throw new IllegalArgumentException("windowSize must be bigger > than 0"); > + } > this.windowSize = windowSize; > buf = new byte[3 * windowSize]; > writeIndex = readIndex = 0; > @@ -201,8 +205,12 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > * Used by subclasses to signal the next block contains the given > * amount of literal data. > * @param length the length of the block > + * @throws IllegalArgumentException if length is negative > */ > protected final void startLiteral(long length) { > + if (length < 0) { > + throw new IllegalArgumentException("length must not be > negative"); > + } > bytesRemaining = length; > } > > @@ -224,6 +232,10 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > * @throws IOException if the underlying stream throws or signals > * an EOF before the amount of data promised for the block have > * been read > + * @throws NullPointerException if <code>b</code> is null > + * @throws IndexOutOfBoundsException if <code>off</code> is > + * negative, <code>len</code> is negative, or <code>len</code> is > + * greater than <code>b.length - off</code> > */ > protected final int readLiteral(final byte[] b, final int off, final > int len) throws IOException { > final int avail = available(); > @@ -234,7 +246,7 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > } > > private void tryToReadLiteral(int bytesToRead) throws IOException { > - // min of "what is still inside the literal", "what does the user > want" and "how muc can fit into the buffer" > + // min of "what is still inside the literal", "what does the user > want" and "how much can fit into the buffer" > final int reallyTryToRead = Math.min((int) Math.min(bytesToRead, > bytesRemaining), > buf.length - writeIndex); > final int bytesRead = reallyTryToRead > 0 > @@ -271,8 +283,18 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > * Used by subclasses to signal the next block contains a > back-reference with the given coordinates. > * @param offset the offset of the back-reference > * @param length the length of the back-reference > + * @throws IllegalArgumentException if offset not bigger than 0 or > + * bigger than the number of bytes available for back-references > + * or if length is negative > */ > protected final void startBackReference(int offset, long length) { > + if (offset <= 0 || offset > writeIndex) { > + throw new IllegalArgumentException("offset must be bigger > than 0 but not bigger than the number" > + + " of bytes available for back-references"); > + } > + if (length < 0) { > + throw new IllegalArgumentException("length must not be > negative"); > + } > backReferenceOffset = offset; > bytesRemaining = length; > } > @@ -284,6 +306,10 @@ public abstract class > AbstractLZ77CompressorInputStream extends CompressorInputS > * @param len maximum amount of data to read > * @return number of bytes read, may be 0. Will never return -1 as > * EOF-detection is the responsibility of the subclass > + * @throws NullPointerException if <code>b</code> is null > + * @throws IndexOutOfBoundsException if <code>off</code> is > + * negative, <code>len</code> is negative, or <code>len</code> is > + * greater than <code>b.length - off</code> > */ > protected final int readBackReference(final byte[] b, final int off, > final int len) { > final int avail = available(); > diff --git > a/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorInputStream.java > b/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorInputStream.java > index 4650cb8..4657ac8 100644 > --- > a/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorInputStream.java > +++ > b/src/main/java/org/apache/commons/compress/compressors/snappy/SnappyCompressorInputStream.java > @@ -78,6 +78,7 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > * The block size used in compression > * > * @throws IOException if reading fails > + * @throws IllegalArgumentException if blockSize is not bigger than 0 > */ > public SnappyCompressorInputStream(final InputStream is, final int > blockSize) > throws IOException { > @@ -135,6 +136,9 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > case 0x00: > > length = readLiteralLength(b); > + if (length < 0) { > + throw new IOException("illegal block with a negative > literal size found"); > + } > uncompressedBytesRemaining -= length; > startLiteral(length); > state = State.IN_LITERAL; > @@ -152,6 +156,9 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > */ > > length = 4 + ((b >> 2) & 0x07); > + if (length < 0) { > + throw new IOException("illegal block with a negative > match length found"); > + } > uncompressedBytesRemaining -= length; > offset = (b & 0xE0) << 3; > b = readOneByte(); > @@ -160,7 +167,11 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > } > offset |= b; > > - startBackReference(offset, length); > + try { > + startBackReference(offset, length); > + } catch (IllegalArgumentException ex) { > + throw new IOException("illegal block with bad offset > found", ex); > + } > state = State.IN_BACK_REFERENCE; > break; > > @@ -175,11 +186,18 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > */ > > length = (b >> 2) + 1; > + if (length < 0) { > + throw new IOException("illegal block with a negative > match length found"); > + } > uncompressedBytesRemaining -= length; > > offset = (int) ByteUtils.fromLittleEndian(supplier, 2); > > - startBackReference(offset, length); > + try { > + startBackReference(offset, length); > + } catch (IllegalArgumentException ex) { > + throw new IOException("illegal block with bad offset > found", ex); > + } > state = State.IN_BACK_REFERENCE; > break; > > @@ -193,11 +211,18 @@ public class SnappyCompressorInputStream extends > AbstractLZ77CompressorInputStre > */ > > length = (b >> 2) + 1; > + if (length < 0) { > + throw new IOException("illegal block with a negative > match length found"); > + } > uncompressedBytesRemaining -= length; > > offset = (int) ByteUtils.fromLittleEndian(supplier, 4) & > 0x7fffffff; > > - startBackReference(offset, length); > + try { > + startBackReference(offset, length); > + } catch (IllegalArgumentException ex) { > + throw new IOException("illegal block with bad offset > found", ex); > + } > state = State.IN_BACK_REFERENCE; > break; > default: > >
