Re: [commons-compress] branch master updated: COMPRESS-490 throw IOException for certain malformed archives

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Re: [commons-compress] branch master updated: COMPRESS-490 throw IOException for certain malformed archives

garydgregory
IMO exceptions messages should start with a capital letter, like a
sentence.

Gary

On Thu, Aug 8, 2019, 12:29 <[hidden email]> 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 <[hidden email]>
> 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:
>
>