diff --git a/pom.xml b/pom.xml index 498a0d52dea..2b105a1fec3 100644 --- a/pom.xml +++ b/pom.xml @@ -89,6 +89,8 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. ${basedir}/src/conf/checkstyle/checkstyle.xml ${basedir}/src/conf/checkstyle/checkstyle-suppressions.xml LICENSE.txt, NOTICE.txt, **/maven-archiver/pom.properties + + 0.24.1 jira @@ -289,21 +291,6 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. maven-bundle-plugin ${commons.felix.version} - - com.github.siom79.japicmp - japicmp-maven-plugin - - - - - org.apache.commons.compress.harmony.pack200.Segment - org.apache.commons.compress.harmony.pack200.SegmentMethodVisitor - org.apache.commons.compress.harmony.pack200.SegmentAnnotationVisitor - org.apache.commons.compress.harmony.pack200.SegmentFieldVisitor - - - - org.apache.maven.plugins maven-checkstyle-plugin diff --git a/src/changes/changes.xml b/src/changes/changes.xml index b23dc64dc05..6576e1a11d6 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -127,6 +127,8 @@ The type attribute can be add,update,fix,remove. Bump org.apache.commons:commons-parent from 85 to 88 #707. Bump org.apache.commons:commons-lang3 from 3.18.0 to 3.19.0. + + Makes TarUtils final and cleans up protected methods #712. diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java index 3938ce2f3ba..75738de370f 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java @@ -48,8 +48,7 @@ * * @Immutable */ -// CheckStyle:HideUtilityClassConstructorCheck OFF (bc) -public class TarUtils { +public final class TarUtils { private static final Pattern HEADER_STRINGS_PATTERN = Pattern.compile(","); @@ -461,7 +460,7 @@ public static boolean parseBoolean(final byte[] buffer, final int offset) { * @throws IOException Corrupted TAR archive. * @since 1.21 */ - protected static List parseFromPAX01SparseHeaders(final String sparseMap) throws IOException { + static List parseFromPAX01SparseHeaders(final String sparseMap) throws IOException { final List sparseHeaders = new ArrayList<>(); final String[] sparseHeaderStrings = HEADER_STRINGS_PATTERN.split(sparseMap); if (sparseHeaderStrings.length % 2 == 1) { @@ -614,30 +613,6 @@ public static long parseOctalOrBinary(final byte[] buffer, final int offset, fin return parseBinaryBigInteger(buffer, offset, length, negative); } - /** - * For PAX Format 0.1, the sparse headers are stored in a single variable : GNU.sparse.map - * - *

- * GNU.sparse.map: Map of non-null data chunks. It is a string consisting of comma-separated values "offset,size[,offset-1,size-1...]" - *

- *

- * Will internally invoke {@link #parseFromPAX01SparseHeaders} and map IOExceptions to a RzuntimeException, You should use - * {@link #parseFromPAX01SparseHeaders} directly instead. - *

- * - * @param sparseMap the sparse map string consisting of comma-separated values "offset,size[,offset-1,size-1...]". - * @return sparse headers parsed from sparse map. - * @deprecated use #parseFromPAX01SparseHeaders instead. - */ - @Deprecated - protected static List parsePAX01SparseHeaders(final String sparseMap) { - try { - return parseFromPAX01SparseHeaders(sparseMap); - } catch (final IOException ex) { - throw new UncheckedIOException(ex.getMessage(), ex); - } - } - /** * For PAX Format 1.X: The sparse map itself is stored in the file data block, preceding the actual file data. It consists of a series of decimal numbers * delimited by newlines. The map is padded with nulls to the nearest block boundary. The first number gives the number of entries in the map. Following are @@ -648,7 +623,7 @@ protected static List parsePAX01SparseHeaders(final Stri * @return sparse headers. * @throws IOException if an I/O error occurs. */ - protected static List parsePAX1XSparseHeaders(final InputStream inputStream, final int recordSize) throws IOException { + static List parsePAX1XSparseHeaders(final InputStream inputStream, final int recordSize) throws IOException { // for 1.X PAX Headers final List sparseHeaders = new ArrayList<>(); long bytesRead = 0; @@ -681,37 +656,6 @@ protected static List parsePAX1XSparseHeaders(final Inpu return sparseHeaders; } - /** - * For PAX Format 0.0, the sparse headers(GNU.sparse.offset and GNU.sparse.numbytes) may appear multi times, and they look like: - * - *
-     * GNU.sparse.size=size
-     * GNU.sparse.numblocks=numblocks
-     * repeat numblocks times
-     *   GNU.sparse.offset=offset
-     *   GNU.sparse.numbytes=numbytes
-     * end repeat
-     * 
- *

- * For PAX Format 0.1, the sparse headers are stored in a single variable: GNU.sparse.map - *

- *

- * GNU.sparse.map: Map of non-null data chunks. It is a string consisting of comma-separated values "offset,size[,offset-1,size-1...]" - *

- * - * @param inputStream input stream to read keys and values. - * @param sparseHeaders used in PAX Format 0.0 & 0.1, as it may appear multiple times, the sparse headers need to be stored in an array, not a map. - * @param globalPaxHeaders global PAX headers of the tar archive. - * @return map of PAX headers values found inside the current (local or global) PAX headers tar entry. - * @throws IOException if an I/O error occurs. - * @deprecated use the four-arg version instead. - */ - @Deprecated - protected static Map parsePaxHeaders(final InputStream inputStream, final List sparseHeaders, - final Map globalPaxHeaders) throws IOException { - return parsePaxHeaders(inputStream, sparseHeaders, globalPaxHeaders, -1); - } - /** * For PAX Format 0.0, the sparse headers(GNU.sparse.offset and GNU.sparse.numbytes) may appear multi times, and they look like: * @@ -733,13 +677,17 @@ protected static Map parsePaxHeaders(final InputStream inputStre * @param inputStream input stream to read keys and values * @param sparseHeaders used in PAX Format 0.0 & 0.1, as it may appear multiple times, the sparse headers need to be stored in an array, not a map * @param globalPaxHeaders global PAX headers of the tar archive - * @param headerSize total size of the PAX header, will be ignored if negative + * @param headerSize total size of the PAX header * @return map of PAX headers values found inside the current (local or global) PAX headers tar entry. * @throws IOException if an I/O error occurs. * @since 1.21 */ - protected static Map parsePaxHeaders(final InputStream inputStream, final List sparseHeaders, - final Map globalPaxHeaders, final long headerSize) throws IOException { + static Map parsePaxHeaders( + final InputStream inputStream, + final List sparseHeaders, + final Map globalPaxHeaders, + final long headerSize) + throws IOException { final Map headers = new HashMap<>(globalPaxHeaders); Long offset = null; // Format is "length keyword=value\n"; @@ -760,7 +708,7 @@ protected static Map parsePaxHeaders(final InputStream inputStre while ((ch = inputStream.read()) != -1) { read++; totalRead++; - if (totalRead < 0 || headerSize >= 0 && totalRead >= headerSize) { + if (totalRead < 0 || totalRead >= headerSize) { break; } if (ch == '=') { // end of keyword @@ -769,7 +717,7 @@ protected static Map parsePaxHeaders(final InputStream inputStre final int restLen = len - read; if (restLen <= 1) { // only NL headers.remove(keyword); - } else if (headerSize >= 0 && restLen > headerSize - totalRead) { + } else if (restLen > headerSize - totalRead) { throw new ArchiveException("PAX header value size %,d exceeds size of header record.", restLen); } else { final byte[] rest = IOUtils.readRange(inputStream, restLen); @@ -791,9 +739,10 @@ protected static Map parsePaxHeaders(final InputStream inputStre sparseHeaders.add(new TarArchiveStructSparse(offset, 0)); } try { - offset = Long.valueOf(value); - } catch (final NumberFormatException ex) { - throw new ArchiveException("Failed to read PAX header: Offset %s contains a non-numeric value.", + offset = ParsingUtils.parseLongValue(value); + } catch (final IOException ex) { + throw new ArchiveException( + "Failed to read PAX header: Offset %s contains a non-numeric value.", TarGnuSparseKeys.OFFSET); } if (offset < 0) { @@ -806,7 +755,14 @@ protected static Map parsePaxHeaders(final InputStream inputStre throw new ArchiveException("Failed to read PAX header: %s is expected before GNU.sparse.numbytes shows up.", TarGnuSparseKeys.OFFSET); } - final long numbytes = ParsingUtils.parseLongValue(value); + final long numbytes; + try { + numbytes = ParsingUtils.parseLongValue(value); + } catch (final IOException ex) { + throw new ArchiveException( + "Failed to read PAX header: Numbytes %s contains a non-numeric value.", + TarGnuSparseKeys.NUMBYTES); + } if (numbytes < 0) { throw new ArchiveException("Failed to read PAX header: %s contains negative value.", TarGnuSparseKeys.NUMBYTES); } diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java index c9375513616..acfa4b3b25d 100644 --- a/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java @@ -20,6 +20,8 @@ package org.apache.commons.compress.archivers.tar; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -29,12 +31,9 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -291,7 +290,7 @@ void testParseOctalInvalid() { @Test void testParsePAX01SparseHeadersRejectsOddNumberOfEntries() { final String map = "0,10,20,0,20"; - assertThrows(UncheckedIOException.class, () -> TarUtils.parsePAX01SparseHeaders(map)); + assertThrows(ArchiveException.class, () -> TarUtils.parseFromPAX01SparseHeaders(map)); } @Test @@ -399,7 +398,9 @@ void testParseTarWithSpecialPaxHeaders() throws IOException { @Test void testPaxHeaderEntryWithEmptyValueRemovesKey() throws Exception { - final Map headers = TarUtils.parsePaxHeaders(new ByteArrayInputStream("11 foo=bar\n7 foo=\n".getBytes(UTF_8)), null, new HashMap<>()); + final byte[] bytes = "11 foo=bar\n7 foo=\n".getBytes(UTF_8); + final Map headers = + TarUtils.parsePaxHeaders(new ByteArrayInputStream(bytes), emptyList(), emptyMap(), bytes.length); assertEquals(0, headers.size()); } @@ -459,17 +460,19 @@ void testReadNegativeBinary8Byte() { void testReadNonAsciiPaxHeader() throws Exception { final String ae = "\u00e4"; final String line = "11 path=" + ae + "\n"; - assertEquals(11, line.getBytes(UTF_8).length); - final Map headers = TarUtils.parsePaxHeaders(new ByteArrayInputStream(line.getBytes(UTF_8)), null, new HashMap<>()); + final byte[] bytes = line.getBytes(UTF_8); + assertEquals(11, bytes.length); + final Map headers = + TarUtils.parsePaxHeaders(new ByteArrayInputStream(bytes), emptyList(), emptyMap(), 11); assertEquals(1, headers.size()); assertEquals(ae, headers.get("path")); } @Test void testReadPax00SparseHeader() throws Exception { - final String header = "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=10\n"; + final byte[] header = "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=10\n".getBytes(UTF_8); final List sparseHeaders = new ArrayList<>(); - TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), sparseHeaders, Collections.emptyMap()); + TarUtils.parsePaxHeaders(new ByteArrayInputStream(header), sparseHeaders, emptyMap(), header.length); assertEquals(1, sparseHeaders.size()); assertEquals(0, sparseHeaders.get(0).getOffset()); assertEquals(10, sparseHeaders.get(0).getNumbytes()); @@ -477,9 +480,9 @@ void testReadPax00SparseHeader() throws Exception { @Test void testReadPax00SparseHeaderMakesNumbytesOptional() throws Exception { - final String header = "23 GNU.sparse.offset=0\n24 GNU.sparse.offset=10\n"; + final byte[] header = "23 GNU.sparse.offset=0\n24 GNU.sparse.offset=10\n".getBytes(UTF_8); final List sparseHeaders = new ArrayList<>(); - TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), sparseHeaders, Collections.emptyMap()); + TarUtils.parsePaxHeaders(new ByteArrayInputStream(header), sparseHeaders, emptyMap(), header.length); assertEquals(2, sparseHeaders.size()); assertEquals(0, sparseHeaders.get(0).getOffset()); assertEquals(0, sparseHeaders.get(0).getNumbytes()); @@ -487,48 +490,50 @@ void testReadPax00SparseHeaderMakesNumbytesOptional() throws Exception { assertEquals(0, sparseHeaders.get(1).getNumbytes()); } - @Test - void testReadPax00SparseHeaderRejectsNegativeNumbytes() throws Exception { - final String header = "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=-1\n"; - assertThrows(ArchiveException.class, () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), null, Collections.emptyMap())); - } - - @Test - void testReadPax00SparseHeaderRejectsNegativeOffset() throws Exception { - final String header = "24 GNU.sparse.offset=-1\n26 GNU.sparse.numbytes=10\n"; - assertThrows(ArchiveException.class, () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), null, Collections.emptyMap())); - } - - @Test - void testReadPax00SparseHeaderRejectsNonNumericNumbytes() throws Exception { - final String header = "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=1a\n"; - assertThrows(IOException.class, () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), null, Collections.emptyMap())); - } - - @Test - void testReadPax00SparseHeaderRejectsNonNumericOffset() throws Exception { - final String header = "23 GNU.sparse.offset=a\n26 GNU.sparse.numbytes=10\n"; - assertThrows(ArchiveException.class, () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream(header.getBytes(UTF_8)), null, Collections.emptyMap())); + static Stream testReadPaxHeaderInvalidCases() { + return Stream.of( + Arguments.of( + "Negative numbytes in PAX 00 sparse header", + "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=-1\n"), + Arguments.of( + "Negative offset in PAX 00 sparse header", + "24 GNU.sparse.offset=-1\n26 GNU.sparse.numbytes=10\n"), + Arguments.of( + "Non-numeric numbytes in PAX 00 sparse header", + "23 GNU.sparse.offset=0\n26 GNU.sparse.numbytes=1a\n"), + Arguments.of( + "Non-numeric offset in PAX 00 sparse header", + "23 GNU.sparse.offset=a\n26 GNU.sparse.numbytes=10\n"), + Arguments.of( + "Numbytes in PAX 00 sparse header without offset", + "26 GNU.sparse.numbytes=10\n" + ), + Arguments.of("Missing trailing newline in PAX header", "30 atime=1321711775.9720594634")); + } + + @ParameterizedTest(name = "{0}") + @MethodSource + void testReadPaxHeaderInvalidCases(final String description, final String header) { + final byte[] bytes = header.getBytes(UTF_8); + assertThrows( + ArchiveException.class, + () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream(bytes), emptyList(), emptyMap(), bytes.length)); } @Test void testReadPaxHeaderWithEmbeddedNewline() throws Exception { - final Map headers = TarUtils.parsePaxHeaders(new ByteArrayInputStream("28 comment=line1\nline2\nand3\n".getBytes(UTF_8)), null, - new HashMap<>()); + final byte[] header = "28 comment=line1\nline2\nand3\n".getBytes(UTF_8); + final Map headers = + TarUtils.parsePaxHeaders(new ByteArrayInputStream(header), emptyList(), emptyMap(), header.length); assertEquals(1, headers.size()); assertEquals("line1\nline2\nand3", headers.get("comment")); } - @Test - void testReadPaxHeaderWithoutTrailingNewline() throws Exception { - assertThrows(ArchiveException.class, - () -> TarUtils.parsePaxHeaders(new ByteArrayInputStream("30 atime=1321711775.9720594634".getBytes(UTF_8)), null, Collections.emptyMap())); - } - @Test void testReadSimplePaxHeader() throws Exception { - final Map headers = TarUtils.parsePaxHeaders(new ByteArrayInputStream("30 atime=1321711775.972059463\n".getBytes(UTF_8)), null, - new HashMap<>()); + final byte[] header = "30 atime=1321711775.972059463\n".getBytes(UTF_8); + final Map headers = + TarUtils.parsePaxHeaders(new ByteArrayInputStream(header), emptyList(), emptyMap(), header.length); assertEquals(1, headers.size()); assertEquals("1321711775.972059463", headers.get("atime")); } @@ -643,8 +648,9 @@ void testRoundTripOctalOrBinary8_ValueTooBigForBinary() { @Test void testSecondEntryWinsWhenPaxHeaderContainsDuplicateKey() throws Exception { - final Map headers = TarUtils.parsePaxHeaders(new ByteArrayInputStream("11 foo=bar\n11 foo=baz\n".getBytes(UTF_8)), null, - new HashMap<>()); + final byte[] header = "11 foo=bar\n11 foo=baz\n".getBytes(UTF_8); + final Map headers = + TarUtils.parsePaxHeaders(new ByteArrayInputStream(header), emptyList(), emptyMap(), header.length); assertEquals(1, headers.size()); assertEquals("baz", headers.get("foo")); }