This is an automated email from the ASF dual-hosted git repository.
ggregory 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 8a9a5847c COMPRESS-632: Fixes for dump file parsing (#442)
8a9a5847c is described below
commit 8a9a5847c04ae39a1d45b365f8bb82022466067d
Author: Yakov Shafranovich <yako...@amazon.com>
AuthorDate: Tue Nov 28 07:49:07 2023 -0500
COMPRESS-632: Fixes for dump file parsing (#442)
* Added check for negative blocksize
* more dump tests
* removing unused imports
* fixing checkstyle violations
* moved null check to DumpArchiveUtil
* Trim whitespace
* Format and normalize tests declarations
* fix exception message to match unit test
---------
Co-authored-by: Gary Gregory <garydgreg...@users.noreply.github.com>
---
.../archivers/dump/DumpArchiveConstants.java | 4 +--
.../compress/archivers/dump/DumpArchiveUtil.java | 13 ++++-----
.../compress/archivers/dump/TapeInputStream.java | 3 +++
.../archivers/dump/DumpArchiveInputStreamTest.java | 16 ++++++++++++
.../archivers/dump/DumpArchiveUtilTest.java | 21 +++++++++++++++
...chiveUtilTest.java => TapeInputStreamTest.java} | 29 +++++++++------------
.../compress/dump/directory_null_bytes.dump | Bin 0 -> 78903 bytes
.../compress/dump/invalid_compression_type.dump | Bin 0 -> 11904 bytes
8 files changed, 62 insertions(+), 24 deletions(-)
diff --git
a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveConstants.java
b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveConstants.java
index 47d9d34cc..6c266d43a 100644
---
a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveConstants.java
+++
b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveConstants.java
@@ -26,7 +26,7 @@ public final class DumpArchiveConstants {
* The type of compression.
*/
public enum COMPRESSION_TYPE {
- ZLIB(0), BZLIB(1), LZO(2);
+ UNKNOWN(-1), ZLIB(0), BZLIB(1), LZO(2);
public static COMPRESSION_TYPE find(final int code) {
for (final COMPRESSION_TYPE t : values()) {
@@ -35,7 +35,7 @@ public final class DumpArchiveConstants {
}
}
- return null;
+ return UNKNOWN;
}
final int code;
diff --git
a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtil.java
b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtil.java
index 223039dff..ec26377f8 100644
---
a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtil.java
+++
b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtil.java
@@ -37,11 +37,9 @@ final class DumpArchiveUtil {
*/
public static int calculateChecksum(final byte[] buffer) {
int calc = 0;
-
for (int i = 0; i < 256; i++) {
calc += DumpArchiveUtil.convert32(buffer, 4 * i);
}
-
return DumpArchiveConstants.CHECKSUM - (calc -
DumpArchiveUtil.convert32(buffer, 28));
}
@@ -82,7 +80,10 @@ final class DumpArchiveUtil {
* Decodes a byte array to a string.
*/
static String decode(final ZipEncoding encoding, final byte[] b, final int
offset, final int len) throws IOException {
- return encoding.decode(Arrays.copyOfRange(b, offset, offset + len));
+ if (offset > offset + len) {
+ throw new IOException("Invalid offset/length combination");
+ }
+ return encoding.decode(Arrays.copyOfRange(b, offset, offset +
len));
}
/**
@@ -102,16 +103,16 @@ final class DumpArchiveUtil {
* @return Whether the buffer contains a tape segment header.
*/
public static boolean verify(final byte[] buffer) {
+ if (buffer == null) {
+ return false;
+ }
// verify magic. for now only accept NFS_MAGIC.
final int magic = convert32(buffer, 24);
-
if (magic != DumpArchiveConstants.NFS_MAGIC) {
return false;
}
-
// verify checksum...
final int checksum = convert32(buffer, 28);
-
return checksum == calculateChecksum(buffer);
}
diff --git
a/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
b/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
index 46d1cc6ff..27c8a89cf 100644
---
a/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
+++
b/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
@@ -294,6 +294,9 @@ final class TapeInputStream extends FilterInputStream {
throw new IOException("Block with " + recsPerBlock + " records
found, must be at least 1");
}
blockSize = RECORD_SIZE * recsPerBlock;
+ if (blockSize < 1) {
+ throw new IOException("Block size cannot be less than or equal to
0: " + blockSize);
+ }
// save first block in case we need it again
final byte[] oldBuffer = blockBuffer;
diff --git
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
index bb6e41d8b..4b4c2396a 100644
---
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
@@ -87,4 +87,20 @@ public class DumpArchiveInputStreamTest extends AbstractTest
{
}
}
+ @Test
+ public void testDirectoryNullBytes() throws Exception {
+ try (InputStream is =
newInputStream("org/apache/commons/compress/dump/directory_null_bytes.dump");
+ DumpArchiveInputStream archive = new DumpArchiveInputStream(is)) {
+ assertThrows(InvalidFormatException.class, archive::getNextEntry);
+ }
+ }
+
+ @Test
+ public void testInvalidCompressType() throws Exception {
+ try (InputStream is =
newInputStream("org/apache/commons/compress/dump/invalid_compression_type.dump"))
{
+ final ArchiveException ex = assertThrows(ArchiveException.class,
() -> new DumpArchiveInputStream(is).close());
+ assertInstanceOf(UnsupportedCompressionAlgorithmException.class,
ex.getCause());
+ }
+ }
+
}
diff --git
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
index 7559a535d..ae829059d 100644
---
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
@@ -18,7 +18,11 @@
*/
package org.apache.commons.compress.archivers.dump;
+import static org.junit.Assert.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import java.io.IOException;
import org.junit.jupiter.api.Test;
@@ -38,4 +42,21 @@ public class DumpArchiveUtilTest {
public void testConvert64() {
assertEquals(0xABCDEF0123456780L, DumpArchiveUtil.convert64(new byte[]
{ (byte) 0x80, 0x67, 0x45, 0x23, 1, (byte) 0xEF, (byte) 0xCD, (byte) 0xAB },
0));
}
+
+ @Test
+ public void testDecodeInvalidArguments() {
+ assertThrows(IOException.class, () -> DumpArchiveUtil.decode(null, new
byte[10], 10, -1));
+ }
+
+ @Test
+ public void testVerifyNullArgument() {
+ assertFalse(DumpArchiveUtil.verify(null));
+ }
+
+ @Test
+ public void testVerifyNoMagic() {
+ assertFalse(DumpArchiveUtil.verify(new byte[32]));
+ }
+
}
+
diff --git
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
b/src/test/java/org/apache/commons/compress/archivers/dump/TapeInputStreamTest.java
similarity index 53%
copy from
src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
copy to
src/test/java/org/apache/commons/compress/archivers/dump/TapeInputStreamTest.java
index 7559a535d..775bb66bc 100644
---
a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveUtilTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/dump/TapeInputStreamTest.java
@@ -18,24 +18,21 @@
*/
package org.apache.commons.compress.archivers.dump;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
-import org.junit.jupiter.api.Test;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
-public class DumpArchiveUtilTest {
+import org.apache.commons.compress.AbstractTest;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
- @Test
- public void testConvert16() {
- assertEquals(0xABCD, DumpArchiveUtil.convert16(new byte[] { (byte)
0xCD, (byte) 0xAB }, 0));
- }
-
- @Test
- public void testConvert32() {
- assertEquals(0xABCDEF01, DumpArchiveUtil.convert32(new byte[] { 1,
(byte) 0xEF, (byte) 0xCD, (byte) 0xAB }, 0));
- }
-
- @Test
- public void testConvert64() {
- assertEquals(0xABCDEF0123456780L, DumpArchiveUtil.convert64(new byte[]
{ (byte) 0x80, 0x67, 0x45, 0x23, 1, (byte) 0xEF, (byte) 0xCD, (byte) 0xAB },
0));
+public class TapeInputStreamTest extends AbstractTest {
+ @ParameterizedTest
+ @ValueSource(ints = {-1, 0, Integer.MAX_VALUE / 1000, Integer.MAX_VALUE})
+ public void testResetBlockSizeWithInvalidValues(final int recsPerBlock)
throws Exception {
+ try (TapeInputStream tapeInputStream = new TapeInputStream(new
ByteArrayInputStream(new byte[1]))) {
+ assertThrows(IOException.class, () ->
tapeInputStream.resetBlockSize(recsPerBlock, true));
+ }
}
}
diff --git
a/src/test/resources/org/apache/commons/compress/dump/directory_null_bytes.dump
b/src/test/resources/org/apache/commons/compress/dump/directory_null_bytes.dump
new file mode 100644
index 000000000..66f62049f
Binary files /dev/null and
b/src/test/resources/org/apache/commons/compress/dump/directory_null_bytes.dump
differ
diff --git
a/src/test/resources/org/apache/commons/compress/dump/invalid_compression_type.dump
b/src/test/resources/org/apache/commons/compress/dump/invalid_compression_type.dump
new file mode 100644
index 000000000..77b5ebfe2
Binary files /dev/null and
b/src/test/resources/org/apache/commons/compress/dump/invalid_compression_type.dump
differ
