Skip to content

Commit 47bf566

Browse files
garydgregorygarydgregory
committed
[COMPRESS-714] Internal IllegalArgumentException in ZipFile and TarFile
creation is not caught
1 parent d1471bb commit 47bf566

4 files changed

Lines changed: 107 additions & 9 deletions

File tree

‎src/main/java/org/apache/commons/compress/archivers/tar/TarFile.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,13 +191,16 @@ private TarFile(final Builder builder) throws IOException {
191191
while ((entry = getNextTarEntry()) != null) {
192192
entries.add(entry);
193193
}
194-
} catch (final IOException ex) {
194+
} catch (final IOException | IllegalArgumentException e) {
195+
final IOException buildException = e instanceof IOException
196+
? (IOException) e
197+
: new ArchiveException("Error reading Zip content from " + builder, (Throwable) e);
195198
try {
196199
this.archive.close();
197-
} catch (final IOException e) {
198-
ex.addSuppressed(e);
200+
} catch (final IOException closeException) {
201+
buildException.addSuppressed(closeException);
199202
}
200-
throw ex;
203+
throw buildException;
201204
}
202205
}
203206

‎src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -755,15 +755,15 @@ private ZipFile(final Builder builder) throws IOException {
755755
resolveLocalFileHeaderData(entriesWithoutUTF8Flag);
756756
}
757757
fillNameMap();
758-
} catch (final IOException e) {
759-
final ArchiveException archiveException = e instanceof ArchiveException
760-
? (ArchiveException) e
758+
} catch (final IOException | IllegalArgumentException e) {
759+
final IOException archiveException = e instanceof IOException
760+
? (IOException) e
761761
: new ArchiveException("Error reading Zip content from " + builder.getName(), (Throwable) e);
762762
this.closed = true;
763763
try {
764764
this.archive.close();
765-
} catch (final IOException ioException) {
766-
archiveException.addSuppressed(ioException);
765+
} catch (final IOException closeException) {
766+
archiveException.addSuppressed(closeException);
767767
}
768768
throw archiveException;
769769
}

‎src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.commons.compress.archivers.tar;
21+
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
import java.io.IOException;
25+
26+
import org.apache.commons.compress.archivers.ArchiveException;
27+
import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
28+
import org.junit.jupiter.api.Test;
29+
30+
/**
31+
* Tests https://issues.apache.org/jira/browse/COMPRESS-714
32+
*/
33+
public class TarCompress714Test {
34+
35+
@Test
36+
public void testIllegalPosition() throws IOException {
37+
final byte[] data = { 46, 101, 97, 115, 97, 47, 120, -64, 72, 98, -18, 2, 53, 101, 112, 0, 0, 115, 8, 0, 0, 0, 112, 115, 40, 1, 0, 36, 2, 108, 0, -1,
38+
-1, 0, 0, 0, 74, 0, 0, 0, 0, -1, 0, 1, 67, -2, 8, 0, 0, 0, 0, -64, -1, -1, -34, 9, 0, -120, -120, -120, -120, -120, -120, -120, -120, -120,
39+
-120, -120, -120, -120, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 63, 0, 0, 0, 0, 0, 0, -65, -1, -126, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128,
40+
-1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, 0, 0, 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, -120, 55, 55,
41+
55, 55, 55, 55, 55, 55, 55, 55, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -105, 0, 0, 0, 49, 36, -1, -1, -1, -1, -1, -1, -126, 0, 0, 0, 0, 0,
42+
0, 0, 53, 0, 0, 0, -128, 0, 0, 0, 0, 16, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 46, 101, 97, 115, 97, 47, 120, -64, 72, 98, -18, 2, 53,
43+
101, 112, 0, 0, 115, 8, 0, 0, 0, 112, 115, 40, 1, 0, 2, 108, 0, -1, -1, 0, 0, 0, 74, 0, 0, 0, 0, -1, 0, 1, 67, -2, 8, 0, 0, 0, 0, -64, -1, -1,
44+
-34, 9, 0, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 63, 0, 0,
45+
0, 0, 0, 0, -65, -1, -126, 0, 26, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0,
46+
0, 0, 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, -120, 55, 55, 55, 55, 55, 55, 55, 55, 55, 55, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -105, 0, 0,
47+
0, 49, 36, -1, -1, -1, -1, -1, -1, -126, 0, 0, 0, 0, 0, 0, 0, 53, 0, 0, 0, -128, 0, 0, 0, 0, 16, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
48+
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -65,
49+
-1, -126, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, 0, 0,
50+
0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, 0 };
51+
assertThrows(ArchiveException.class, () -> TarFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get());
52+
assertThrows(ArchiveException.class, () -> TarFile.builder().setByteArray(data).get());
53+
}
54+
}

‎src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.commons.compress.archivers.zip;
21+
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
import java.io.IOException;
25+
26+
import org.apache.commons.compress.archivers.ArchiveException;
27+
import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
28+
import org.junit.jupiter.api.Test;
29+
30+
/**
31+
* Tests https://issues.apache.org/jira/browse/COMPRESS-714
32+
*/
33+
public class ZipCompress714Test {
34+
35+
@Test
36+
public void testIllegalPosition() throws IOException {
37+
final byte[] data = { 80, 75, 5, 6, -127, 80, 75, 5, 6, 7, -127, -127, -127, 80, 74, 7, 8, -127, -127, -127, -127, -127 };
38+
assertThrows(ArchiveException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get());
39+
assertThrows(ArchiveException.class, () -> ZipFile.builder().setByteArray(data).get());
40+
}
41+
}

0 commit comments

Comments
 (0)