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 2017/06/24 15:14:11 UTC

[1/3] commons-compress git commit: COMPRESS-407 Validate Block and Record Sizes

Repository: commons-compress
Updated Branches:
  refs/heads/master 4be9979b4 -> fb9b61804


COMPRESS-407 Validate Block and Record Sizes

Require block size >=0 that is multiple of 512 bytes.
Use block size of 512 if one is not given.
Require record size of 512 bytes. Deprecate constructors taking recordSize as parameter

Modify tests to check for enforcement of record size == 512
Modify tests to check for correct overall length  for
different block sizes including PAX default, USTAR default, and unspecified.

Signed-off-by: Simon Spero <se...@gmail.com>

(cherry picked from commit d754d89)
Signed-off-by: Simon Spero <se...@gmail.com>


Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/b6ae1d09
Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/b6ae1d09
Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/b6ae1d09

Branch: refs/heads/master
Commit: b6ae1d09c384b57115bb34ed94c90e256e4b66b9
Parents: 4be9979
Author: Simon Spero <se...@gmail.com>
Authored: Sun Jun 11 18:48:42 2017 -0400
Committer: Stefan Bodewig <bo...@apache.org>
Committed: Sat Jun 24 16:50:39 2017 +0200

----------------------------------------------------------------------
 .../archivers/tar/TarArchiveOutputStream.java   | 217 +++++++++++--------
 .../tar/TarArchiveOutputStreamTest.java         | 209 ++++++++++++------
 2 files changed, 261 insertions(+), 165 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-compress/blob/b6ae1d09/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
index 79c7bca..864d4ad 100644
--- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
@@ -61,18 +61,18 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
 
     /** POSIX/PAX extensions are used to store big numbers in the archive. */
     public static final int BIGNUMBER_POSIX = 2;
-
-    private long      currSize;
-    private String    currName;
-    private long      currBytes;
-    private final byte[]    recordBuf;
-    private int       assemLen;
-    private final byte[]    assemBuf;
-    private int       longFileMode = LONGFILE_ERROR;
-    private int       bigNumberMode = BIGNUMBER_ERROR;
+    private static final int RECORD_SIZE = 512;
+
+    private long currSize;
+    private String currName;
+    private long currBytes;
+    private final byte[] recordBuf;
+    private int assemLen;
+    private final byte[] assemBuf;
+    private int longFileMode = LONGFILE_ERROR;
+    private int bigNumberMode = BIGNUMBER_ERROR;
     private int recordsWritten;
     private final int recordsPerBlock;
-    private final int recordSize;
 
     private boolean closed = false;
 
@@ -93,12 +93,14 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
     private static final ZipEncoding ASCII =
         ZipEncodingHelper.getZipEncoding("ASCII");
 
+    private static int BLOCK_SIZE_UNSPECIFIED = -511;
+
     /**
      * Constructor for TarInputStream.
      * @param os the output stream to use
      */
     public TarArchiveOutputStream(final OutputStream os) {
-        this(os, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE);
+        this(os, BLOCK_SIZE_UNSPECIFIED);
     }
 
     /**
@@ -108,59 +110,82 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * @since 1.4
      */
     public TarArchiveOutputStream(final OutputStream os, final String encoding) {
-        this(os, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE, encoding);
+        this(os, BLOCK_SIZE_UNSPECIFIED, encoding);
     }
 
     /**
      * Constructor for TarInputStream.
      * @param os the output stream to use
-     * @param blockSize the block size to use
+     * @param blockSize the block size to use. Must be a multiple of 512 bytes.
      */
     public TarArchiveOutputStream(final OutputStream os, final int blockSize) {
-        this(os, blockSize, TarConstants.DEFAULT_RCDSIZE);
+        this(os, blockSize, null);
     }
 
+
     /**
      * Constructor for TarInputStream.
      * @param os the output stream to use
      * @param blockSize the block size to use
-     * @param encoding name of the encoding to use for file names
-     * @since 1.4
+     * @param recordSize the record size to use. Must be 512 bytes.
+     * @deprecated recordSize must always be 512 bytes. An IllegalArgumentException will be thrown
+     * if any other value is used
      */
+    @Deprecated
     public TarArchiveOutputStream(final OutputStream os, final int blockSize,
-                                  final String encoding) {
-        this(os, blockSize, TarConstants.DEFAULT_RCDSIZE, encoding);
+        final int recordSize) {
+        this(os, blockSize, recordSize, null);
     }
 
     /**
      * Constructor for TarInputStream.
      * @param os the output stream to use
-     * @param blockSize the block size to use
-     * @param recordSize the record size to use
+     * @param blockSize the block size to use . Must be a multiple of 512 bytes.
+     * @param recordSize the record size to use. Must be 512 bytes.
+     * @param encoding name of the encoding to use for file names
+     * @since 1.4
+     * @deprecated recordSize must always be 512 bytes. An IllegalArgumentException will be thrown
+     * if any other value is used.
      */
-    public TarArchiveOutputStream(final OutputStream os, final int blockSize, final int recordSize) {
-        this(os, blockSize, recordSize, null);
+    @Deprecated
+    public TarArchiveOutputStream(final OutputStream os, final int blockSize,
+        final int recordSize, final String encoding) {
+        this(os, blockSize, encoding);
+        if (recordSize != RECORD_SIZE) {
+            throw new IllegalArgumentException(
+                "Tar record size must always be 512 bytes. Attempt to set size of " + recordSize);
+        }
+
     }
 
     /**
      * Constructor for TarInputStream.
+     *
      * @param os the output stream to use
-     * @param blockSize the block size to use
-     * @param recordSize the record size to use
+     * @param blockSize the block size to use. Must be a multiple of 512 bytes.
      * @param encoding name of the encoding to use for file names
      * @since 1.4
      */
     public TarArchiveOutputStream(final OutputStream os, final int blockSize,
-                                  final int recordSize, final String encoding) {
+        final String encoding) {
+        int realBlockSize;
+        if (BLOCK_SIZE_UNSPECIFIED == blockSize) {
+            realBlockSize = RECORD_SIZE;
+        } else {
+            realBlockSize = blockSize;
+        }
+
+        if(realBlockSize <=0 || realBlockSize % RECORD_SIZE != 0) {
+            throw new IllegalArgumentException("Block size must be a multiple of 512 bytes. Attempt to use set size of " + blockSize);
+        }
         out = new CountingOutputStream(os);
         this.encoding = encoding;
         this.zipEncoding = ZipEncodingHelper.getZipEncoding(encoding);
 
         this.assemLen = 0;
-        this.assemBuf = new byte[recordSize];
-        this.recordBuf = new byte[recordSize];
-        this.recordSize = recordSize;
-        this.recordsPerBlock = blockSize / recordSize;
+        this.assemBuf = new byte[RECORD_SIZE];
+        this.recordBuf = new byte[RECORD_SIZE];
+        this.recordsPerBlock = realBlockSize / RECORD_SIZE;
     }
 
     /**
@@ -208,11 +233,11 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
 
     /**
      * Ends the TAR archive without closing the underlying OutputStream.
-     * 
+     *
      * An archive consists of a series of file entries terminated by an
-     * end-of-archive entry, which consists of two 512 blocks of zero bytes. 
+     * end-of-archive entry, which consists of two 512 blocks of zero bytes.
      * POSIX.1 requires two EOF records, like some other implementations.
-     * 
+     *
      * @throws IOException on error
      */
     @Override
@@ -251,9 +276,11 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * Get the record size being used by this stream's TarBuffer.
      *
      * @return The TarBuffer record size.
+     * @deprecated
      */
+    @Deprecated
     public int getRecordSize() {
-        return this.recordSize;
+        return RECORD_SIZE;
     }
 
     /**
@@ -278,12 +305,12 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         final Map<String, String> paxHeaders = new HashMap<>();
         final String entryName = entry.getName();
         final boolean paxHeaderContainsPath = handleLongName(entry, entryName, paxHeaders, "path",
-                                                       TarConstants.LF_GNUTYPE_LONGNAME, "file name");
+            TarConstants.LF_GNUTYPE_LONGNAME, "file name");
 
         final String linkName = entry.getLinkName();
         final boolean paxHeaderContainsLinkPath = linkName != null && linkName.length() > 0
             && handleLongName(entry, linkName, paxHeaders, "linkpath",
-                              TarConstants.LF_GNUTYPE_LONGLINK, "link name");
+            TarConstants.LF_GNUTYPE_LONGLINK, "link name");
 
         if (bigNumberMode == BIGNUMBER_POSIX) {
             addPaxHeadersForBigNumbers(paxHeaders, entry);
@@ -307,7 +334,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         }
 
         entry.writeEntryHeader(recordBuf, zipEncoding,
-                               bigNumberMode == BIGNUMBER_STAR);
+            bigNumberMode == BIGNUMBER_STAR);
         writeRecord(recordBuf);
 
         currBytes = 0;
@@ -336,7 +363,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         if (finished) {
             throw new IOException("Stream has already been finished");
         }
-        if (!haveUnclosedEntry){
+        if (!haveUnclosedEntry) {
             throw new IOException("No current entry to close");
         }
         if (assemLen > 0) {
@@ -352,9 +379,9 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
 
         if (currBytes < currSize) {
             throw new IOException("entry '" + currName + "' closed at '"
-                                  + currBytes
-                                  + "' before the '" + currSize
-                                  + "' bytes specified in the header were written");
+                + currBytes
+                + "' before the '" + currSize
+                + "' bytes specified in the header were written");
         }
         haveUnclosedEntry = false;
     }
@@ -380,9 +407,9 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         }
         if (currBytes + numToWrite > currSize) {
             throw new IOException("request to write '" + numToWrite
-                                  + "' bytes exceeds size in header of '"
-                                  + currSize + "' bytes for entry '"
-                                  + currName + "'");
+                + "' bytes exceeds size in header of '"
+                + currSize + "' bytes for entry '"
+                + currName + "'");
 
             //
             // We have to deal with assembly!!!
@@ -398,9 +425,9 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
                 final int aLen = recordBuf.length - assemLen;
 
                 System.arraycopy(assemBuf, 0, recordBuf, 0,
-                                 assemLen);
+                    assemLen);
                 System.arraycopy(wBuf, wOffset, recordBuf,
-                                 assemLen, aLen);
+                    assemLen, aLen);
                 writeRecord(recordBuf);
 
                 currBytes += recordBuf.length;
@@ -409,7 +436,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
                 assemLen = 0;
             } else {
                 System.arraycopy(wBuf, wOffset, assemBuf, assemLen,
-                                 numToWrite);
+                    numToWrite);
 
                 wOffset += numToWrite;
                 assemLen += numToWrite;
@@ -425,7 +452,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         while (numToWrite > 0) {
             if (numToWrite < recordBuf.length) {
                 System.arraycopy(wBuf, wOffset, assemBuf, assemLen,
-                                 numToWrite);
+                    numToWrite);
 
                 assemLen += numToWrite;
 
@@ -447,14 +474,14 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * @since 1.4
      */
     void writePaxHeaders(final TarArchiveEntry entry,
-                         final String entryName,
-                         final Map<String, String> headers) throws IOException {
+        final String entryName,
+        final Map<String, String> headers) throws IOException {
         String name = "./PaxHeaders.X/" + stripTo7Bits(entryName);
         if (name.length() >= TarConstants.NAMELEN) {
             name = name.substring(0, TarConstants.NAMELEN - 1);
         }
         final TarArchiveEntry pex = new TarArchiveEntry(name,
-                                                  TarConstants.LF_PAX_EXTENDED_HEADER_LC);
+            TarConstants.LF_PAX_EXTENDED_HEADER_LC);
         transferModTime(entry, pex);
 
         final StringWriter w = new StringWriter();
@@ -500,8 +527,8 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
     }
 
     /**
-     * @return true if the character could lead to problems when used
-     * inside a TarArchiveEntry name for a PAX header.
+     * @return true if the character could lead to problems when used inside a TarArchiveEntry name
+     * for a PAX header.
      */
     private boolean shouldBeReplaced(final char c) {
         return c == 0 // would be read as Trailing null
@@ -510,8 +537,8 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
     }
 
     /**
-     * Write an EOF (end of archive) record to the tar archive.
-     * An EOF record consists of a record of all zeros.
+     * Write an EOF (end of archive) record to the tar archive. An EOF record consists of a record
+     * of all zeros.
      */
     private void writeEOFRecord() throws IOException {
         Arrays.fill(recordBuf, (byte) 0);
@@ -525,13 +552,13 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
 
     @Override
     public ArchiveEntry createArchiveEntry(final File inputFile, final String entryName)
-            throws IOException {
-        if(finished) {
+        throws IOException {
+        if (finished) {
             throw new IOException("Stream has already been finished");
         }
         return new TarArchiveEntry(inputFile, entryName);
     }
-    
+
     /**
      * Write an archive record to the archive.
      *
@@ -539,17 +566,17 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * @throws IOException on error
      */
     private void writeRecord(final byte[] record) throws IOException {
-        if (record.length != recordSize) {
+        if (record.length != RECORD_SIZE) {
             throw new IOException("record to write has length '"
-                                  + record.length
-                                  + "' which is not the record size of '"
-                                  + recordSize + "'");
+                + record.length
+                + "' which is not the record size of '"
+                + RECORD_SIZE + "'");
         }
 
         out.write(record);
         recordsWritten++;
     }
-    
+
     /**
      * Write an archive record to the archive, where the record may be
      * inside of a larger array buffer. The buffer must be "offset plus
@@ -560,15 +587,15 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * @throws IOException on error
      */
     private void writeRecord(final byte[] buf, final int offset) throws IOException {
- 
-        if (offset + recordSize > buf.length) {
+
+        if (offset + RECORD_SIZE > buf.length) {
             throw new IOException("record has length '" + buf.length
-                                  + "' with offset '" + offset
-                                  + "' which is less than the record size of '"
-                                  + recordSize + "'");
+                + "' with offset '" + offset
+                + "' which is less than the record size of '"
+                + RECORD_SIZE + "'");
         }
 
-        out.write(buf, offset, recordSize);
+        out.write(buf, offset, RECORD_SIZE);
         recordsWritten++;
     }
 
@@ -582,28 +609,28 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
     }
 
     private void addPaxHeadersForBigNumbers(final Map<String, String> paxHeaders,
-                                            final TarArchiveEntry entry) {
+        final TarArchiveEntry entry) {
         addPaxHeaderForBigNumber(paxHeaders, "size", entry.getSize(),
-                                 TarConstants.MAXSIZE);
+            TarConstants.MAXSIZE);
         addPaxHeaderForBigNumber(paxHeaders, "gid", entry.getLongGroupId(),
-                                 TarConstants.MAXID);
+            TarConstants.MAXID);
         addPaxHeaderForBigNumber(paxHeaders, "mtime",
-                                 entry.getModTime().getTime() / 1000,
-                                 TarConstants.MAXSIZE);
+            entry.getModTime().getTime() / 1000,
+            TarConstants.MAXSIZE);
         addPaxHeaderForBigNumber(paxHeaders, "uid", entry.getLongUserId(),
-                                 TarConstants.MAXID);
+            TarConstants.MAXID);
         // star extensions by J\u00f6rg Schilling
         addPaxHeaderForBigNumber(paxHeaders, "SCHILY.devmajor",
-                                 entry.getDevMajor(), TarConstants.MAXID);
+            entry.getDevMajor(), TarConstants.MAXID);
         addPaxHeaderForBigNumber(paxHeaders, "SCHILY.devminor",
-                                 entry.getDevMinor(), TarConstants.MAXID);
+            entry.getDevMinor(), TarConstants.MAXID);
         // there is no PAX header for file mode
         failForBigNumber("mode", entry.getMode(), TarConstants.MAXID);
     }
 
     private void addPaxHeaderForBigNumber(final Map<String, String> paxHeaders,
-                                          final String header, final long value,
-                                          final long maxValue) {
+        final String header, final long value,
+        final long maxValue) {
         if (value < 0 || value > maxValue) {
             paxHeaders.put(header, String.valueOf(value));
         }
@@ -613,29 +640,32 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
         failForBigNumber("entry size", entry.getSize(), TarConstants.MAXSIZE);
         failForBigNumberWithPosixMessage("group id", entry.getLongGroupId(), TarConstants.MAXID);
         failForBigNumber("last modification time",
-                         entry.getModTime().getTime() / 1000,
-                         TarConstants.MAXSIZE);
+            entry.getModTime().getTime() / 1000,
+            TarConstants.MAXSIZE);
         failForBigNumber("user id", entry.getLongUserId(), TarConstants.MAXID);
         failForBigNumber("mode", entry.getMode(), TarConstants.MAXID);
         failForBigNumber("major device number", entry.getDevMajor(),
-                         TarConstants.MAXID);
+            TarConstants.MAXID);
         failForBigNumber("minor device number", entry.getDevMinor(),
-                         TarConstants.MAXID);
+            TarConstants.MAXID);
     }
 
     private void failForBigNumber(final String field, final long value, final long maxValue) {
         failForBigNumber(field, value, maxValue, "");
     }
 
-    private void failForBigNumberWithPosixMessage(final String field, final long value, final long maxValue) {
-        failForBigNumber(field, value, maxValue, " Use STAR or POSIX extensions to overcome this limit");
+    private void failForBigNumberWithPosixMessage(final String field, final long value,
+        final long maxValue) {
+        failForBigNumber(field, value, maxValue,
+            " Use STAR or POSIX extensions to overcome this limit");
     }
 
-    private void failForBigNumber(final String field, final long value, final long maxValue, final String additionalMsg) {
+    private void failForBigNumber(final String field, final long value, final long maxValue,
+        final String additionalMsg) {
         if (value < 0 || value > maxValue) {
             throw new RuntimeException(field + " '" + value //NOSONAR
-                    + "' is too big ( > "
-                    + maxValue + " )." + additionalMsg);
+                + "' is too big ( > "
+                + maxValue + " )." + additionalMsg);
         }
     }
 
@@ -661,9 +691,9 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
      * @param fieldName the name of the field
      * @return whether a pax header has been written.
      */
-    private boolean handleLongName(final TarArchiveEntry entry , final String name,
-                                   final Map<String, String> paxHeaders,
-                                   final String paxHeaderName, final byte linkType, final String fieldName)
+    private boolean handleLongName(final TarArchiveEntry entry, final String name,
+        final Map<String, String> paxHeaders,
+        final String paxHeaderName, final byte linkType, final String fieldName)
         throws IOException {
         final ByteBuffer encodedName = zipEncoding.encode(name);
         final int len = encodedName.limit() - encodedName.position();
@@ -675,7 +705,8 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
             } else if (longFileMode == LONGFILE_GNU) {
                 // create a TarEntry for the LongLink, the contents
                 // of which are the link's name
-                final TarArchiveEntry longLinkEntry = new TarArchiveEntry(TarConstants.GNU_LONGLINK, linkType);
+                final TarArchiveEntry longLinkEntry = new TarArchiveEntry(TarConstants.GNU_LONGLINK,
+                    linkType);
 
                 longLinkEntry.setSize(len + 1l); // +1 for NUL
                 transferModTime(entry, longLinkEntry);
@@ -685,8 +716,8 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
                 closeArchiveEntry();
             } else if (longFileMode != LONGFILE_TRUNCATE) {
                 throw new RuntimeException(fieldName + " '" + name //NOSONAR
-                                           + "' is too long ( > "
-                                           + TarConstants.NAMELEN + " bytes)");
+                    + "' is too long ( > "
+                    + TarConstants.NAMELEN + " bytes)");
             }
         }
         return false;

http://git-wip-us.apache.org/repos/asf/commons-compress/blob/b6ae1d09/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java
index 85cdb66..497a764 100644
--- a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java
@@ -26,6 +26,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.security.MessageDigest;
 import java.util.Calendar;
 import java.util.Date;
@@ -100,10 +101,10 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.write(new byte[10 * 1024]);
         final byte[] data = bos.toByteArray();
         assertEquals(0x80,
-                     data[TarConstants.NAMELEN
-                        + TarConstants.MODELEN
-                        + TarConstants.UIDLEN
-                        + TarConstants.GIDLEN] & 0x80);
+            data[TarConstants.NAMELEN
+                + TarConstants.MODELEN
+                + TarConstants.UIDLEN
+                + TarConstants.GIDLEN] & 0x80);
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -126,12 +127,12 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.write(new byte[10 * 1024]);
         final byte[] data = bos.toByteArray();
         assertEquals("00000000000 ",
-                     new String(data,
-                                1024 + TarConstants.NAMELEN
-                                + TarConstants.MODELEN
-                                + TarConstants.UIDLEN
-                                + TarConstants.GIDLEN, 12,
-                                CharsetNames.UTF_8));
+            new String(data,
+                1024 + TarConstants.NAMELEN
+                    + TarConstants.MODELEN
+                    + TarConstants.UIDLEN
+                    + TarConstants.GIDLEN, 12,
+                CharsetNames.UTF_8));
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -148,11 +149,11 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         m.put("a", "b");
         final byte[] data = writePaxHeader(m);
         assertEquals("00000000006 ",
-                     new String(data, TarConstants.NAMELEN
-                                + TarConstants.MODELEN
-                                + TarConstants.UIDLEN
-                                + TarConstants.GIDLEN, 12,
-                                CharsetNames.UTF_8));
+            new String(data, TarConstants.NAMELEN
+                + TarConstants.MODELEN
+                + TarConstants.UIDLEN
+                + TarConstants.GIDLEN, 12,
+                CharsetNames.UTF_8));
         assertEquals("6 a=b\n", new String(data, 512, 6, CharsetNames.UTF_8));
     }
 
@@ -160,38 +161,38 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
     public void testPaxHeadersWithLength99() throws Exception {
         final Map<String, String> m = new HashMap<>();
         m.put("a",
-              "0123456789012345678901234567890123456789"
-              + "01234567890123456789012345678901234567890123456789"
-              + "012");
+            "0123456789012345678901234567890123456789"
+                + "01234567890123456789012345678901234567890123456789"
+                + "012");
         final byte[] data = writePaxHeader(m);
         assertEquals("00000000143 ",
-                     new String(data, TarConstants.NAMELEN
-                                + TarConstants.MODELEN
-                                + TarConstants.UIDLEN
-                                + TarConstants.GIDLEN, 12,
-                                CharsetNames.UTF_8));
+            new String(data, TarConstants.NAMELEN
+                + TarConstants.MODELEN
+                + TarConstants.UIDLEN
+                + TarConstants.GIDLEN, 12,
+                CharsetNames.UTF_8));
         assertEquals("99 a=0123456789012345678901234567890123456789"
-              + "01234567890123456789012345678901234567890123456789"
-              + "012\n", new String(data, 512, 99, CharsetNames.UTF_8));
+            + "01234567890123456789012345678901234567890123456789"
+            + "012\n", new String(data, 512, 99, CharsetNames.UTF_8));
     }
 
     @Test
     public void testPaxHeadersWithLength101() throws Exception {
         final Map<String, String> m = new HashMap<>();
         m.put("a",
-              "0123456789012345678901234567890123456789"
-              + "01234567890123456789012345678901234567890123456789"
-              + "0123");
+            "0123456789012345678901234567890123456789"
+                + "01234567890123456789012345678901234567890123456789"
+                + "0123");
         final byte[] data = writePaxHeader(m);
         assertEquals("00000000145 ",
-                     new String(data, TarConstants.NAMELEN
-                                + TarConstants.MODELEN
-                                + TarConstants.UIDLEN
-                                + TarConstants.GIDLEN, 12,
-                                CharsetNames.UTF_8));
+            new String(data, TarConstants.NAMELEN
+                + TarConstants.MODELEN
+                + TarConstants.UIDLEN
+                + TarConstants.GIDLEN, 12,
+                CharsetNames.UTF_8));
         assertEquals("101 a=0123456789012345678901234567890123456789"
-              + "01234567890123456789012345678901234567890123456789"
-              + "0123\n", new String(data, 512, 101, CharsetNames.UTF_8));
+            + "01234567890123456789012345678901234567890123456789"
+            + "0123\n", new String(data, 512, 101, CharsetNames.UTF_8));
     }
 
     private byte[] writePaxHeader(final Map<String, String> m) throws Exception {
@@ -226,7 +227,7 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.closeArchiveEntry();
         final byte[] data = bos.toByteArray();
         assertEquals("160 path=" + n + "\n",
-                     new String(data, 512, 160, CharsetNames.UTF_8));
+            new String(data, 512, 160, CharsetNames.UTF_8));
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -248,11 +249,11 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.write(new byte[10 * 1024]);
         final byte[] data = bos.toByteArray();
         assertEquals((byte) 0xff,
-                     data[TarConstants.NAMELEN
-                          + TarConstants.MODELEN
-                          + TarConstants.UIDLEN
-                          + TarConstants.GIDLEN
-                          + TarConstants.SIZELEN]);
+            data[TarConstants.NAMELEN
+                + TarConstants.MODELEN
+                + TarConstants.UIDLEN
+                + TarConstants.GIDLEN
+                + TarConstants.SIZELEN]);
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -279,13 +280,13 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.write(new byte[10 * 1024]);
         final byte[] data = bos.toByteArray();
         assertEquals("00000000000 ",
-                     new String(data,
-                                1024 + TarConstants.NAMELEN
-                                + TarConstants.MODELEN
-                                + TarConstants.UIDLEN
-                                + TarConstants.GIDLEN
-                                + TarConstants.SIZELEN, 12,
-                                CharsetNames.UTF_8));
+            new String(data,
+                1024 + TarConstants.NAMELEN
+                    + TarConstants.MODELEN
+                    + TarConstants.UIDLEN
+                    + TarConstants.GIDLEN
+                    + TarConstants.SIZELEN, 12,
+                CharsetNames.UTF_8));
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -328,7 +329,7 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.close();
         final byte[] data = bos.toByteArray();
         assertEquals("11 path=" + n + "\n",
-                     new String(data, 512, 11, CharsetNames.UTF_8));
+            new String(data, 512, 11, CharsetNames.UTF_8));
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -351,7 +352,7 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tos.close();
         final byte[] data = bos.toByteArray();
         assertEquals("15 linkpath=" + n + "\n",
-                     new String(data, 512, 15, CharsetNames.UTF_8));
+            new String(data, 512, 15, CharsetNames.UTF_8));
         final TarArchiveInputStream tin =
             new TarArchiveInputStream(new ByteArrayInputStream(data));
         final TarArchiveEntry e = tin.getNextTarEntry();
@@ -399,8 +400,8 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
     @Test
     public void testWriteLongDirectoryNameErrorMode() throws Exception {
         final String n = "01234567890123456789012345678901234567890123456789"
-                + "01234567890123456789012345678901234567890123456789"
-                + "01234567890123456789012345678901234567890123456789/";
+            + "01234567890123456789012345678901234567890123456789"
+            + "01234567890123456789012345678901234567890123456789/";
 
         try {
             final TarArchiveEntry t = new TarArchiveEntry(n);
@@ -524,8 +525,8 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
     @Test
     public void testWriteLongLinkNameErrorMode() throws Exception {
         final String linkname = "01234567890123456789012345678901234567890123456789"
-                + "01234567890123456789012345678901234567890123456789"
-                + "01234567890123456789012345678901234567890123456789/test";
+            + "01234567890123456789012345678901234567890123456789"
+            + "01234567890123456789012345678901234567890123456789/test";
         final TarArchiveEntry entry = new TarArchiveEntry("test", TarConstants.LF_SYMLINK);
         entry.setLinkName(linkname);
 
@@ -548,7 +549,7 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         final String linkname = "01234567890123456789012345678901234567890123456789"
             + "01234567890123456789012345678901234567890123456789"
             + "01234567890123456789012345678901234567890123456789/";
-        final TarArchiveEntry entry = new TarArchiveEntry("test" , TarConstants.LF_SYMLINK);
+        final TarArchiveEntry entry = new TarArchiveEntry("test", TarConstants.LF_SYMLINK);
         entry.setLinkName(linkname);
 
         final ByteArrayOutputStream bos = new ByteArrayOutputStream();
@@ -607,31 +608,92 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         tin.close();
     }
 
+    @SuppressWarnings("deprecation")
+    @Test public void testRecordSize() throws IOException {
+        try {
+            TarArchiveOutputStream tos =
+                new TarArchiveOutputStream(new ByteArrayOutputStream(),512,511);
+            fail("should have rejected recordSize of 511");
+        } catch(IllegalArgumentException e) {
+            // expected;
+        }
+        try {
+            TarArchiveOutputStream tos =
+                new TarArchiveOutputStream(new ByteArrayOutputStream(),512,511,null);
+            fail("should have rejected recordSize of 511");
+        } catch(IllegalArgumentException e) {
+            // expected;
+        }
+        try (TarArchiveOutputStream tos = new TarArchiveOutputStream(new ByteArrayOutputStream(),
+            512, 512)) {
+            assertEquals("recordSize",512,tos.getRecordSize());
+        }
+        try (TarArchiveOutputStream tos = new TarArchiveOutputStream(new ByteArrayOutputStream(),
+            512, 512, null)) {
+            assertEquals("recordSize",512,tos.getRecordSize());
+        }
+    }
     @Test
-    public void testPadsOutputToFullBlockLength() throws Exception {
+    public void testBlockSizes() throws Exception {
+        String fileName = "/test1.xml";
+        byte[] contents = getResourceContents(fileName);
+        testPadding(TarConstants.DEFAULT_BLKSIZE, fileName, contents); // USTAR / pre-pax
+        testPadding(5120, fileName, contents); // PAX default
+        testPadding(1<<15, fileName, contents); //PAX max
+        testPadding(-2, fileName, contents);    // don't specify a block size -> use minimum length
+        try {
+            testPadding(511, fileName, contents);    // don't specify a block size -> use minimum length
+            fail("should have thrown an illegal argument exception");
+        } catch (IllegalArgumentException e) {
+            //expected
+       }
+        try {
+            testPadding(0, fileName, contents);    // don't specify a block size -> use minimum length
+            fail("should have thrown an illegal argument exception");
+        } catch (IllegalArgumentException e) {
+            //expected
+       }
+    }
+
+    private void testPadding(int blockSize, String fileName, byte[] contents) throws IOException {
         final File f = File.createTempFile("commons-compress-padding", ".tar");
         f.deleteOnExit();
         final FileOutputStream fos = new FileOutputStream(f);
-        final TarArchiveOutputStream tos = new TarArchiveOutputStream(fos);
-        final File file1 = getFile("test1.xml");
-        final TarArchiveEntry sEntry = new TarArchiveEntry(file1, file1.getName());
+        final TarArchiveOutputStream tos;
+        if(blockSize != -2) {
+            tos = new TarArchiveOutputStream(fos, blockSize);
+        } else {
+            blockSize = 512;
+            tos = new TarArchiveOutputStream(fos);
+        }
+        TarArchiveEntry sEntry;
+        sEntry = new TarArchiveEntry(fileName);
+        sEntry.setSize(contents.length);
         tos.putArchiveEntry(sEntry);
-        final FileInputStream in = new FileInputStream(file1);
-        IOUtils.copy(in, tos);
-        in.close();
+        tos.write(contents);
         tos.closeArchiveEntry();
         tos.close();
-        // test1.xml is small enough to fit into the default block size
-        assertEquals(TarConstants.DEFAULT_BLKSIZE, f.length());
+        int fileRecordsSize = (int)Math.ceil((double) contents.length / 512)*512;
+        final int headerSize = 512;
+        final int endOfArchiveSize = 1024;
+        int unpaddedSize = headerSize + fileRecordsSize + endOfArchiveSize;
+        int paddedSize = (int)Math.ceil((double)unpaddedSize/blockSize)*blockSize;
+        assertEquals(paddedSize, f.length());
+    }
+
+    private byte[] getResourceContents(String name) throws IOException {
+        ByteArrayOutputStream bos;
+        try (InputStream resourceAsStream = getClass().getResourceAsStream(name)) {
+            bos = new ByteArrayOutputStream();
+            IOUtils.copy(resourceAsStream, bos);
+        }
+        return bos.toByteArray();
     }
 
     /**
-     * When using long file names the longLinkEntry included the
-     * current timestamp as the Entry modification date. This was
-     * never exposed to the client but it caused identical archives to
+     * When using long file names the longLinkEntry included the current timestamp as the Entry
+     * modification date. This was never exposed to the client but it caused identical archives to
      * have different MD5 hashes.
-     *
-     * @throws Exception
      */
     @Test
     public void testLongNameMd5Hash() throws Exception {
@@ -655,15 +717,18 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase {
         // do I still have the correct modification date?
         // let a second elapse so we don't get the current time
         Thread.sleep(1000);
-        final TarArchiveInputStream tarIn = new TarArchiveInputStream(new ByteArrayInputStream(archive2));
+        final TarArchiveInputStream tarIn = new TarArchiveInputStream(
+            new ByteArrayInputStream(archive2));
         final ArchiveEntry nextEntry = tarIn.getNextEntry();
         assertEquals(longFileName, nextEntry.getName());
         // tar archive stores modification time to second granularity only (floored)
-        assertEquals(modificationDate.getTime() / 1000, nextEntry.getLastModifiedDate().getTime() / 1000);
+        assertEquals(modificationDate.getTime() / 1000,
+            nextEntry.getLastModifiedDate().getTime() / 1000);
         tarIn.close();
     }
 
-    private static byte[] createTarArchiveContainingOneDirectory(final String fname, final Date modificationDate) throws IOException {
+    private static byte[] createTarArchiveContainingOneDirectory(final String fname,
+        final Date modificationDate) throws IOException {
         final ByteArrayOutputStream baos = new ByteArrayOutputStream();
         final TarArchiveOutputStream tarOut = new TarArchiveOutputStream(baos, 1024);
         tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_GNU);


[2/3] commons-compress git commit: COMPRESS-407 tweaks

Posted by bo...@apache.org.
COMPRESS-407 tweaks


Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/e57007c0
Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/e57007c0
Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/e57007c0

Branch: refs/heads/master
Commit: e57007c00d577c8f66a5d93681058a528613645c
Parents: b6ae1d0
Author: Stefan Bodewig <bo...@apache.org>
Authored: Sat Jun 24 17:02:30 2017 +0200
Committer: Stefan Bodewig <bo...@apache.org>
Committed: Sat Jun 24 17:02:30 2017 +0200

----------------------------------------------------------------------
 .../commons/compress/archivers/tar/TarArchiveOutputStream.java   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-compress/blob/e57007c0/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
index 864d4ad..6b31705 100644
--- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
@@ -93,7 +93,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
     private static final ZipEncoding ASCII =
         ZipEncodingHelper.getZipEncoding("ASCII");
 
-    private static int BLOCK_SIZE_UNSPECIFIED = -511;
+    private static final int BLOCK_SIZE_UNSPECIFIED = -511;
 
     /**
      * Constructor for TarInputStream.
@@ -175,7 +175,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream {
             realBlockSize = blockSize;
         }
 
-        if(realBlockSize <=0 || realBlockSize % RECORD_SIZE != 0) {
+        if (realBlockSize <=0 || realBlockSize % RECORD_SIZE != 0) {
             throw new IllegalArgumentException("Block size must be a multiple of 512 bytes. Attempt to use set size of " + blockSize);
         }
         out = new CountingOutputStream(os);


[3/3] commons-compress git commit: COMPRESS-407 record backwards incompatible change

Posted by bo...@apache.org.
COMPRESS-407 record backwards incompatible change

closes #40 && closes #41 && closes #42


Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/fb9b6180
Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/fb9b6180
Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/fb9b6180

Branch: refs/heads/master
Commit: fb9b6180428694c45a8befab3ca32834d99acece
Parents: e57007c
Author: Stefan Bodewig <bo...@apache.org>
Authored: Sat Jun 24 17:13:24 2017 +0200
Committer: Stefan Bodewig <bo...@apache.org>
Committed: Sat Jun 24 17:13:24 2017 +0200

----------------------------------------------------------------------
 src/changes/changes.xml | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-compress/blob/fb9b6180/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 0ec4038..b9dafea 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -43,7 +43,13 @@ The <action> type attribute can be add,update,fix,remove.
   </properties>
   <body>
     <release version="1.15" date="not released, yet"
-             description="Release 1.15">
+             description="Release 1.15
+----------------------------------------
+
+TarArchiveOutputStream now ensures record size is 512 and block size is
+a multiple of 512 as any other value would create invalid tar
+archives. This may break compatibility for code that deliberately
+wanted to create such files.">
       <action issue="COMPRESS-394" type="fix" date="2017-05-22">
         Make sure "version needed to extract" in local file header and
         central directory of a ZIP archive agree with each other.
@@ -77,6 +83,13 @@ The <action> type attribute can be add,update,fix,remove.
         Made sure ChecksumCalculatingInputStream receives valid
         checksum and input stream instances via the cnstructor.
       </action>
+      <action issue="COMPRESS-407" type="fix" date="2017-06-24"
+              due-to="Simon Spero ">
+        TarArchiveOutputStream now verifies the block and record sizes
+        specified at construction time are compatible with the tar
+        specification. In particular 512 is the only recird size
+        accepted and the block size must be a multiple of 512.
+      </action>
     </release>
     <release version="1.14" date="2017-05-14"
              description="Release 1.14">