Index: main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java =================================================================== --- main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java (revision 1505872) +++ main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java (working copy) @@ -36,6 +36,7 @@ import org.apache.commons.compress.archivers.zip.ZipEncodingHelper; import org.apache.commons.compress.utils.ArchiveUtils; import org.apache.commons.compress.utils.CharsetNames; +import org.apache.commons.compress.utils.IOUtils; /** * The TarInputStream reads a UNIX tar archive as an InputStream. @@ -45,18 +46,30 @@ * @NotThreadSafe */ public class TarArchiveInputStream extends ArchiveInputStream { + private static final int SMALL_BUFFER_SIZE = 256; - private static final int BUFFER_SIZE = 8 * 1024; - private final byte[] SKIP_BUF = new byte[BUFFER_SIZE]; private final byte[] SMALL_BUF = new byte[SMALL_BUFFER_SIZE]; + /** The size the TAR header */ + private final int recordSize; + + /** True of file has hit EOF */ private boolean hasHitEOF; + + /** Size of the current entry */ private long entrySize; + + /** How far into the entry the stream is at */ private long entryOffset; - private byte[] readBuf; - protected final TarBuffer buffer; + + /** An input stream to read from */ + private final InputStream is; + + /** The meta-data about the current entry */ private TarArchiveEntry currEntry; + + /** The encoding of the file */ private final ZipEncoding encoding; /** @@ -64,7 +77,7 @@ * @param is the input stream to use */ public TarArchiveInputStream(InputStream is) { - this(is, TarBuffer.DEFAULT_BLKSIZE, TarBuffer.DEFAULT_RCDSIZE); + this(is, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE); } /** @@ -74,7 +87,8 @@ * @since 1.4 */ public TarArchiveInputStream(InputStream is, String encoding) { - this(is, TarBuffer.DEFAULT_BLKSIZE, TarBuffer.DEFAULT_RCDSIZE, encoding); + this(is, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE, + encoding); } /** @@ -83,7 +97,7 @@ * @param blockSize the block size to use */ public TarArchiveInputStream(InputStream is, int blockSize) { - this(is, blockSize, TarBuffer.DEFAULT_RCDSIZE); + this(is, blockSize, TarConstants.DEFAULT_RCDSIZE); } /** @@ -95,7 +109,7 @@ */ public TarArchiveInputStream(InputStream is, int blockSize, String encoding) { - this(is, blockSize, TarBuffer.DEFAULT_RCDSIZE, encoding); + this(is, blockSize, TarConstants.DEFAULT_RCDSIZE, encoding); } /** @@ -105,7 +119,7 @@ * @param recordSize the record size to use */ public TarArchiveInputStream(InputStream is, int blockSize, int recordSize) { - this(is, blockSize, recordSize, null); + this(is, blockSize, recordSize, null); } /** @@ -118,10 +132,10 @@ */ public TarArchiveInputStream(InputStream is, int blockSize, int recordSize, String encoding) { - this.buffer = new TarBuffer(is, blockSize, recordSize); - this.readBuf = null; + this.is = is; this.hasHitEOF = false; this.encoding = ZipEncodingHelper.getZipEncoding(encoding); + this.recordSize = recordSize; } /** @@ -130,16 +144,16 @@ */ @Override public void close() throws IOException { - buffer.close(); + is.close(); } /** - * Get the record size being used by this stream's TarBuffer. + * Get the record size being used by this stream's buffer. * * @return The TarBuffer record size. */ public int getRecordSize() { - return buffer.getRecordSize(); + return recordSize; } /** @@ -174,21 +188,17 @@ */ @Override public long skip(long numToSkip) throws IOException { - // REVIEW - // This is horribly inefficient, but it ensures that we - // properly skip over bytes via the TarBuffer... - // - long skip = numToSkip; - while (skip > 0) { - int realSkip = (int) (skip > SKIP_BUF.length - ? SKIP_BUF.length : skip); - int numRead = read(SKIP_BUF, 0, realSkip); - if (numRead == -1) { - break; - } - skip -= numRead; - } - return (numToSkip - skip); + + long available = (entrySize - entryOffset); + long skipped = 0; + + numToSkip = Math.min(numToSkip, available); + + skipped = IOUtils.skip(is, numToSkip); + + entryOffset += skipped; + + return skipped; } /** @@ -217,18 +227,11 @@ } if (currEntry != null) { - long numToSkip = entrySize - entryOffset; + /* Skip will only go to the end of the current entry */ + skip(Long.MAX_VALUE); - while (numToSkip > 0) { - long skipped = skip(numToSkip); - if (skipped <= 0) { - throw new RuntimeException("failed to skip current tar" - + " entry"); - } - numToSkip -= skipped; - } - - readBuf = null; + /* Each archive should end on an even block */ + skipRecordPadding(); } byte[] headerBuf = getRecord(); @@ -246,6 +249,7 @@ ioe.initCause(e); throw ioe; } + entryOffset = 0; entrySize = currEntry.getSize(); @@ -284,10 +288,23 @@ // information, we update entrySize here so that it contains // the correct value. entrySize = currEntry.getSize(); + return currEntry; } + + /** + * The last record block should be written at the full size, so skip any + * additional space used to fill a record after an entry + */ + private void skipRecordPadding() throws IOException { + if (this.entrySize > 0) { + long numRecords = (this.entrySize / this.recordSize) + 1; + long padding = (numRecords * this.recordSize) - this.entrySize; + IOUtils.skip(is, padding); + } + } - /** + /** * Get the next entry in this tar archive as longname data. * * @return The next entry in the archive as longname data, or null. @@ -335,19 +352,43 @@ * @throws IOException on error */ private byte[] getRecord() throws IOException { - byte[] headerBuf = null; - if (!hasHitEOF) { - headerBuf = buffer.readRecord(); - hasHitEOF = buffer.isEOFRecord(headerBuf); - if (hasHitEOF && headerBuf != null) { - buffer.tryToConsumeSecondEOFRecord(); - headerBuf = null; - } + byte[] headerBuf = readRecord(); + hasHitEOF = isEOFRecord(headerBuf); + if (hasHitEOF && headerBuf != null) { + tryToConsumeSecondEOFRecord(); + headerBuf = null; } - return headerBuf; } + /** + * Determine if an archive record indicate End of Archive. End of + * archive is indicated by a record that consists entirely of null bytes. + * + * @param record The record data to check. + * @return true if the record data is an End of Archive + */ + protected boolean isEOFRecord(byte[] record) { + return record == null || ArchiveUtils.isArrayZero(record, recordSize); + } + + /** + * Read a record from the input stream and return the data. + * + * @return The record data or null if EOF has been hit. + * @throws IOException on error + */ + protected byte[] readRecord() throws IOException { + + byte[] record = new byte[recordSize]; + + if (is.read(record) != recordSize) { + return null; + } + + return record; + } + private void paxHeaders() throws IOException{ Map headers = parsePaxHeaders(this); getNextEntry(); // Get the actual file entry @@ -468,10 +509,42 @@ } } + /** + * Returns the next Archive Entry in this Stream. + * + * @return the next entry, + * or {@code null} if there are no more entries + * @throws IOException if the next entry could not be read + */ @Override public ArchiveEntry getNextEntry() throws IOException { return getNextTarEntry(); } + + /** + * Tries to read the next record rewinding the stream if it is not a EOF record. + * + *

This is meant to protect against cases where a tar + * implementation has written only one EOF record when two are + * expected. Actually this won't help since a non-conforming + * implementation likely won't fill full blocks consisting of - by + * default - ten records either so we probably have already read + * beyond the archive anyway.

+ */ + void tryToConsumeSecondEOFRecord() throws IOException { + boolean shouldReset = true; + boolean marked = is.markSupported(); + if (marked) { + is.mark(recordSize); + } + try { + shouldReset = !isEOFRecord(readRecord()); + } finally { + if (shouldReset && marked) { + is.reset(); + } + } + } /** * Reads bytes from the current tar archive entry. @@ -487,70 +560,28 @@ * @throws IOException on error */ @Override - public int read(byte[] buf, int offset, int numToRead) throws IOException { - int totalRead = 0; + public int read(byte[] buf, int offset, int numToRead) throws IOException + { + int totalRead = 0; - if (entryOffset >= entrySize) { + if (hasHitEOF || entryOffset >= entrySize) + { return -1; } - if ((numToRead + entryOffset) > entrySize) { - numToRead = (int) (entrySize - entryOffset); + numToRead = Math.min(numToRead, available()); + + totalRead = is.read(buf, offset, numToRead); + + if (totalRead == -1) + { + hasHitEOF = true; } - - if (readBuf != null) { - int sz = (numToRead > readBuf.length) ? readBuf.length - : numToRead; - - System.arraycopy(readBuf, 0, buf, offset, sz); - - if (sz >= readBuf.length) { - readBuf = null; - } else { - int newLen = readBuf.length - sz; - byte[] newBuf = new byte[newLen]; - - System.arraycopy(readBuf, sz, newBuf, 0, newLen); - - readBuf = newBuf; - } - - totalRead += sz; - numToRead -= sz; - offset += sz; + else + { + entryOffset += (long) totalRead; } - while (numToRead > 0) { - byte[] rec = buffer.readRecord(); - - if (rec == null) { - // Unexpected EOF! - throw new IOException("unexpected EOF with " + numToRead - + " bytes unread. Occured at byte: " + getBytesRead()); - } - count(rec.length); - int sz = numToRead; - int recLen = rec.length; - - if (recLen > sz) { - System.arraycopy(rec, 0, buf, offset, sz); - - readBuf = new byte[recLen - sz]; - - System.arraycopy(rec, sz, readBuf, 0, recLen - sz); - } else { - sz = recLen; - - System.arraycopy(rec, 0, buf, offset, recLen); - } - - totalRead += sz; - numToRead -= sz; - offset += sz; - } - - entryOffset += totalRead; - return totalRead; } @@ -568,18 +599,34 @@ return false; } + /** + * Get the current TAR Archive Entry that this input stream is processing + * + * @return The current Archive Entry + */ protected final TarArchiveEntry getCurrentEntry() { return currEntry; } + /** + * Set the current TAR Archive Entry that this input stream is processing + */ protected final void setCurrentEntry(TarArchiveEntry e) { currEntry = e; } + /** + * Check if the archive is at EOF + * + * @return true if the archive is at EOF + */ protected final boolean isAtEOF() { return hasHitEOF; } + /** + * Set if the archive is at EOF + */ protected final void setAtEOF(boolean b) { hasHitEOF = b; } Index: main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java =================================================================== --- main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java (revision 1505872) +++ main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java (working copy) @@ -18,6 +18,7 @@ */ package org.apache.commons.compress.archivers.tar; +import java.io.BufferedOutputStream; import java.io.File; import java.io.IOException; import java.io.OutputStream; @@ -67,9 +68,9 @@ private final byte[] recordBuf; private int assemLen; private final byte[] assemBuf; - protected final TarBuffer buffer; private int longFileMode = LONGFILE_ERROR; private int bigNumberMode = BIGNUMBER_ERROR; + private final int recordSize; private boolean closed = false; @@ -92,7 +93,7 @@ * @param os the output stream to use */ public TarArchiveOutputStream(OutputStream os) { - this(os, TarBuffer.DEFAULT_BLKSIZE, TarBuffer.DEFAULT_RCDSIZE); + this(os, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE); } /** @@ -102,7 +103,7 @@ * @since 1.4 */ public TarArchiveOutputStream(OutputStream os, String encoding) { - this(os, TarBuffer.DEFAULT_BLKSIZE, TarBuffer.DEFAULT_RCDSIZE, encoding); + this(os, TarConstants.DEFAULT_BLKSIZE, TarConstants.DEFAULT_RCDSIZE, encoding); } /** @@ -111,7 +112,7 @@ * @param blockSize the block size to use */ public TarArchiveOutputStream(OutputStream os, int blockSize) { - this(os, blockSize, TarBuffer.DEFAULT_RCDSIZE); + this(os, blockSize, TarConstants.DEFAULT_RCDSIZE); } /** @@ -123,7 +124,7 @@ */ public TarArchiveOutputStream(OutputStream os, int blockSize, String encoding) { - this(os, blockSize, TarBuffer.DEFAULT_RCDSIZE, encoding); + this(os, blockSize, TarConstants.DEFAULT_RCDSIZE, encoding); } /** @@ -149,10 +150,10 @@ out = new CountingOutputStream(os); this.encoding = ZipEncodingHelper.getZipEncoding(encoding); - this.buffer = new TarBuffer(out, blockSize, recordSize); this.assemLen = 0; this.assemBuf = new byte[recordSize]; this.recordBuf = new byte[recordSize]; + this.recordSize = recordSize; } /** @@ -217,7 +218,7 @@ } writeEOFRecord(); writeEOFRecord(); - buffer.flushBlock(); + out.flush(); finished = true; } @@ -232,8 +233,8 @@ } if (!closed) { - buffer.close(); out.close(); + out.close(); closed = true; } } @@ -244,7 +245,7 @@ * @return The TarBuffer record size. */ public int getRecordSize() { - return buffer.getRecordSize(); + return this.recordSize; } /** @@ -317,7 +318,7 @@ entry.writeEntryHeader(recordBuf, encoding, bigNumberMode == BIGNUMBER_STAR); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); currBytes = 0; @@ -353,7 +354,7 @@ assemBuf[i] = 0; } - buffer.writeRecord(assemBuf); + writeRecord(assemBuf); currBytes += assemLen; assemLen = 0; @@ -407,7 +408,7 @@ assemLen); System.arraycopy(wBuf, wOffset, recordBuf, assemLen, aLen); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); currBytes += recordBuf.length; wOffset += aLen; @@ -438,7 +439,7 @@ break; } - buffer.writeRecord(wBuf, wOffset); + writeRecord(wBuf, wOffset); int num = recordBuf.length; @@ -512,7 +513,7 @@ */ private void writeEOFRecord() throws IOException { Arrays.fill(recordBuf, (byte) 0); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); } @Override @@ -529,7 +530,46 @@ } return new TarArchiveEntry(inputFile, entryName); } + + /** + * Write an archive record to the archive. + * + * @param record The record data to write to the archive. + * @throws IOException on error + */ + private void writeRecord(byte[] record) throws IOException { + if (record.length != recordSize) { + throw new IOException("record to write has length '" + + record.length + + "' which is not the record size of '" + + recordSize + "'"); + } + + out.write(record); + } + + /** + * Write an archive record to the archive, where the record may be + * inside of a larger array buffer. The buffer must be "offset plus + * record size" long. + * + * @param buf The buffer containing the record data to write. + * @param offset The offset of the record data within buf. + * @throws IOException on error + */ + private void writeRecord(byte[] buf, int offset) throws IOException { + + if ((offset + recordSize) > buf.length) { + throw new IOException("record has length '" + buf.length + + "' with offset '" + offset + + "' which is less than the record size of '" + + recordSize + "'"); + } + + out.write(buf, offset, recordSize); + } + private void addPaxHeadersForBigNumbers(Map paxHeaders, TarArchiveEntry entry) { addPaxHeaderForBigNumber(paxHeaders, "size", entry.getSize(), Index: main/java/org/apache/commons/compress/archivers/tar/TarConstants.java =================================================================== --- main/java/org/apache/commons/compress/archivers/tar/TarConstants.java (revision 1505872) +++ main/java/org/apache/commons/compress/archivers/tar/TarConstants.java (working copy) @@ -27,6 +27,12 @@ // CheckStyle:InterfaceIsTypeCheck OFF (bc) public interface TarConstants { + /** Default record size */ + int DEFAULT_RCDSIZE = (512); + + /** Default block size */ + int DEFAULT_BLKSIZE = (DEFAULT_RCDSIZE * 20); + /** * GNU format as per before tar 1.12. */ Index: main/java/org/apache/commons/compress/utils/ArchiveUtils.java =================================================================== --- main/java/org/apache/commons/compress/utils/ArchiveUtils.java (revision 1505872) +++ main/java/org/apache/commons/compress/utils/ArchiveUtils.java (working copy) @@ -230,5 +230,22 @@ final byte[] buffer2, final int offset2, final int length2){ return isEqual(buffer1, offset1, length1, buffer2, offset2, length2, true); } - + + /** + * Returns true if the first N bytes of an array are all zero + * + * @param a + * The array to check + * @param size + * The number of characters to check (not the size of the array) + * @return true if the first N bytes are zero + */ + public static boolean isArrayZero(byte[] a, int size) { + for (int i = 0; i < size; i++) { + if (a[i] != 0) { + return false; + } + } + return true; + } } Index: main/java/org/apache/commons/compress/utils/IOUtils.java =================================================================== --- main/java/org/apache/commons/compress/utils/IOUtils.java (revision 1505872) +++ main/java/org/apache/commons/compress/utils/IOUtils.java (working copy) @@ -18,6 +18,7 @@ */ package org.apache.commons.compress.utils; +import java.io.BufferedInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -70,8 +71,43 @@ } return count; } + + /** + * Wraps an InputStream in a BufferedInputStream if it isn't already + * + * @param inputStream The InputStream to wrap + * @return A BufferedInputStream if required or the input if not + */ + public static BufferedInputStream asBufferedInputStream( + final InputStream inputStream) { + // reject null early on rather than waiting for IO operation to fail + if (inputStream == null) { // not checked by BufferedInputStream + throw new NullPointerException(); + } + return inputStream instanceof BufferedInputStream ? (BufferedInputStream) inputStream + : new BufferedInputStream(inputStream); + } + /** + * + * @param numToSkip + * @return + * @throws IOException + */ + public static long skip(InputStream input, long numToSkip) + throws IOException { + long available = numToSkip; + while (numToSkip != 0) { + long skipped = input.skip(numToSkip); + if (skipped == 0) { + break; + } + numToSkip -= skipped; + } + return (available - numToSkip); + } + // toByteArray(InputStream) copied from: // commons/proper/io/trunk/src/main/java/org/apache/commons/io/IOUtils.java?revision=1428941 // January 8th, 2013 Index: test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java =================================================================== --- test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java (revision 1505872) +++ test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java (working copy) @@ -27,8 +27,8 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; -import java.io.IOException; import java.util.Calendar; import java.util.Date; import java.util.Map; @@ -41,7 +41,8 @@ @Test public void readSimplePaxHeader() throws Exception { - final TarArchiveInputStream tais = new TarArchiveInputStream(null); + final InputStream is = new ByteArrayInputStream(new byte[1]); + final TarArchiveInputStream tais = new TarArchiveInputStream(is); Map headers = tais .parsePaxHeaders(new ByteArrayInputStream("30 atime=1321711775.972059463\n" .getBytes(CharsetNames.UTF_8))); @@ -52,7 +53,8 @@ @Test public void readPaxHeaderWithEmbeddedNewline() throws Exception { - final TarArchiveInputStream tais = new TarArchiveInputStream(null); + final InputStream is = new ByteArrayInputStream(new byte[1]); + final TarArchiveInputStream tais = new TarArchiveInputStream(is); Map headers = tais .parsePaxHeaders(new ByteArrayInputStream("28 comment=line1\nline2\nand3\n" .getBytes(CharsetNames.UTF_8))); @@ -66,7 +68,8 @@ String ae = "\u00e4"; String line = "11 path="+ ae + "\n"; assertEquals(11, line.getBytes(CharsetNames.UTF_8).length); - final TarArchiveInputStream tais = new TarArchiveInputStream(null); + final InputStream is = new ByteArrayInputStream(new byte[1]); + final TarArchiveInputStream tais = new TarArchiveInputStream(is); Map headers = tais .parsePaxHeaders(new ByteArrayInputStream(line.getBytes(CharsetNames.UTF_8))); assertEquals(1, headers.size());