You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2021/02/28 10:44:45 UTC
[commons-compress] branch master updated: COMPRESS-567 use
IOException rather than RuntimeExceptions
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 32509ee COMPRESS-567 use IOException rather than RuntimeExceptions
32509ee is described below
commit 32509ee94cef4d34ee6e1e82bd044331036ae273
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Sun Feb 28 11:41:58 2021 +0100
COMPRESS-567 use IOException rather than RuntimeExceptions
Most of the time the parameters passed to these methods are directly
read from archives or streams. Users of our classes expect
IOExceptions for corrupt archives. Throwing IOException from our
internal classes avoids having to perform the same type of checks
and throwing inside the calling code everywhere.
---
src/changes/changes.xml | 10 ++++++++++
.../org/apache/commons/compress/archivers/zip/ZipFile.java | 4 ++++
.../java/org/apache/commons/compress/utils/BitInputStream.java | 2 +-
.../commons/compress/utils/FixedLengthBlockOutputStream.java | 2 +-
.../compress/utils/MultiReadOnlySeekableByteChannel.java | 2 +-
.../commons/compress/utils/SeekableInMemoryByteChannel.java | 4 +++-
.../org/apache/commons/compress/utils/BitInputStreamTest.java | 4 ++--
.../compress/utils/MultiReadOnlySeekableByteChannelTest.java | 8 ++++----
.../compress/utils/SeekableInMemoryByteChannelTest.java | 8 ++++----
9 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 83cfc1a..e873eb5 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -306,6 +306,16 @@ The <action> type attribute can be add,update,fix,remove.
certain ZIP archives.
Github Pull Request #172.
</action>
+ <action issue="COMPRESS-567" type="fix" date="2021-02-28">
+ Made some of the stream classes used internally throw
+ IOExceptions on illegal arguments rather than
+ RuntimeExceptions to make it more likely that corrupt archives
+ cause expected checked exceptions rather than RuntimException
+ for various formats.
+
+ Fixes a specific case for ZIP but affects other formats as
+ well.
+ </action>
</release>
<release version="1.20" date="2020-02-08"
description="Release 1.20 (Java 7)">
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
index 8635f76..c8e366b 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java
@@ -1317,6 +1317,10 @@ public class ZipFile implements Closeable {
* underlying archive channel.
*/
private BoundedArchiveInputStream createBoundedInputStream(final long start, final long remaining) {
+ if (start < 0 || remaining < 0 || start + remaining < start) {
+ throw new IllegalArgumentException("Corrupted archive, stream boundaries"
+ + " are out of range");
+ }
return archive instanceof FileChannel ?
new BoundedFileChannelInputStream(start, remaining) :
new BoundedSeekableByteChannelInputStream(start, remaining, archive);
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 e683b50..b8001c4 100644
--- a/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
+++ b/src/main/java/org/apache/commons/compress/utils/BitInputStream.java
@@ -80,7 +80,7 @@ public class BitInputStream implements Closeable {
*/
public long readBits(final int count) throws IOException {
if (count < 0 || count > MAXIMUM_CACHE_SIZE) {
- throw new IllegalArgumentException("count must not be negative or greater than " + MAXIMUM_CACHE_SIZE);
+ throw new IOException("count must not be negative or greater than " + MAXIMUM_CACHE_SIZE);
}
if (ensureCache(count)) {
return -1;
diff --git a/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java b/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java
index 7320f90..1de269e 100644
--- a/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java
+++ b/src/main/java/org/apache/commons/compress/utils/FixedLengthBlockOutputStream.java
@@ -235,7 +235,7 @@ public class FixedLengthBlockOutputStream extends OutputStream implements Writab
throw new ClosedChannelException();
}
if (!buffer.hasArray()) {
- throw new IllegalArgumentException("Direct buffer somehow written to BufferAtATimeOutputChannel");
+ throw new IOException("Direct buffer somehow written to BufferAtATimeOutputChannel");
}
try {
diff --git a/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java b/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java
index f0c166c..b2233eb 100644
--- a/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java
+++ b/src/main/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannel.java
@@ -180,7 +180,7 @@ public class MultiReadOnlySeekableByteChannel implements SeekableByteChannel {
@Override
public synchronized SeekableByteChannel position(final long newPosition) throws IOException {
if (newPosition < 0) {
- throw new IllegalArgumentException("Negative position: " + newPosition);
+ throw new IOException("Negative position: " + newPosition);
}
if (!isOpen()) {
throw new ClosedChannelException();
diff --git a/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java b/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
index bee0751..749e50c 100644
--- a/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
+++ b/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
@@ -91,7 +91,7 @@ public class SeekableInMemoryByteChannel implements SeekableByteChannel {
public SeekableByteChannel position(final long newPosition) throws IOException {
ensureOpen();
if (newPosition < 0L || newPosition > Integer.MAX_VALUE) {
- throw new IllegalArgumentException("Position has to be in range 0.. " + Integer.MAX_VALUE);
+ throw new IOException("Position has to be in range 0.. " + Integer.MAX_VALUE);
}
position = (int) newPosition;
return this;
@@ -113,6 +113,8 @@ public class SeekableInMemoryByteChannel implements SeekableByteChannel {
*
* <p>This method violates the contract of {@link SeekableByteChannel#truncate} as it will not throw any exception when
* invoked on a closed channel.</p>
+ *
+ * @throws IllegalArgumentException if size is negative or bigger than the maximum of a Java integer
*/
@Override
public SeekableByteChannel truncate(final long newSize) {
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 ce996b3..b1916c5 100644
--- a/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/utils/BitInputStreamTest.java
@@ -28,14 +28,14 @@ import org.junit.Test;
public class BitInputStreamTest {
- @Test(expected = IllegalArgumentException.class)
+ @Test(expected = IOException.class)
public void shouldNotAllowReadingOfANegativeAmountOfBits() throws IOException {
try (final BitInputStream bis = new BitInputStream(getStream(), ByteOrder.LITTLE_ENDIAN)) {
bis.readBits(-1);
}
}
- @Test(expected = IllegalArgumentException.class)
+ @Test(expected = IOException.class)
public void shouldNotAllowReadingOfMoreThan63BitsAtATime() throws IOException {
try (final BitInputStream bis = new BitInputStream(getStream(), ByteOrder.LITTLE_ENDIAN)) {
bis.readBits(64);
diff --git a/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java b/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java
index 1bdfd3c..a12630e 100644
--- a/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java
+++ b/src/test/java/org/apache/commons/compress/utils/MultiReadOnlySeekableByteChannelTest.java
@@ -139,7 +139,7 @@ public class MultiReadOnlySeekableByteChannelTest {
@Test
public void cantPositionToANegativePosition() throws IOException {
final SeekableByteChannel s = MultiReadOnlySeekableByteChannel.forSeekableByteChannels(makeEmpty(), makeEmpty());
- thrown.expect(IllegalArgumentException.class);
+ thrown.expect(IOException.class);
s.position(-1);
}
@@ -374,11 +374,11 @@ public class MultiReadOnlySeekableByteChannelTest {
}
/*
- * <q>IllegalArgumentException - If the new position is negative</q>
+ * <q>IOException - If the new position is negative</q>
*/
@Test
- public void throwsIllegalArgumentExceptionWhenPositionIsSetToANegativeValue() throws Exception {
- thrown.expect(IllegalArgumentException.class);
+ public void throwsIOExceptionWhenPositionIsSetToANegativeValue() throws Exception {
+ thrown.expect(IOException.class);
try (SeekableByteChannel c = testChannel()) {
c.position(-1);
}
diff --git a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
index f5c79f9..7eee967 100644
--- a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
+++ b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
@@ -184,7 +184,7 @@ public class SeekableInMemoryByteChannelTest {
c.close();
}
- @Test(expected = IllegalArgumentException.class)
+ @Test(expected = IOException.class)
public void shouldThrowExceptionWhenSettingIncorrectPosition() throws IOException {
//given
final SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel();
@@ -297,10 +297,10 @@ public class SeekableInMemoryByteChannelTest {
}
/*
- * <q>IllegalArgumentException - If the new position is negative</q>
+ * <q>IOException - If the new position is negative</q>
*/
- @Test(expected = IllegalArgumentException.class)
- public void throwsIllegalArgumentExceptionWhenPositionIsSetToANegativeValue() throws Exception {
+ @Test(expected = IOException.class)
+ public void throwsIOExceptionWhenPositionIsSetToANegativeValue() throws Exception {
try (SeekableByteChannel c = new SeekableInMemoryByteChannel()) {
c.position(-1);
}