Index: org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java =================================================================== --- org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java (revision 1502405) +++ org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java (working copy) @@ -23,6 +23,7 @@ package org.apache.commons.compress.archivers.tar; +import java.io.BufferedInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -36,6 +37,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 +47,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 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; + + /** A buffered input stream to read from */ + protected final BufferedInputStream buffer; + + /** The meta-data about the current entry */ private TarArchiveEntry currEntry; + + /** The encoding of the file */ private final ZipEncoding encoding; /** @@ -64,7 +78,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 +88,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 +98,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 +110,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 +120,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 +133,10 @@ */ public TarArchiveInputStream(InputStream is, int blockSize, int recordSize, String encoding) { - this.buffer = new TarBuffer(is, blockSize, recordSize); - this.readBuf = null; + this.buffer = new BufferedInputStream(is, blockSize); this.hasHitEOF = false; this.encoding = ZipEncodingHelper.getZipEncoding(encoding); + this.recordSize = recordSize; } /** @@ -134,12 +149,12 @@ } /** - * 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 +189,14 @@ */ @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); + + numToSkip = Math.min(numToSkip, available); + + IOUtils.skipFully(buffer, numToSkip); + + return numToSkip; } /** @@ -217,18 +225,8 @@ } if (currEntry != null) { - long numToSkip = entrySize - entryOffset; - - while (numToSkip > 0) { - long skipped = skip(numToSkip); - if (skipped <= 0) { - throw new RuntimeException("failed to skip current tar" - + " entry"); - } - numToSkip -= skipped; - } - - readBuf = null; + skip(Long.MAX_VALUE); + skipRecordPadding(); } byte[] headerBuf = getRecord(); @@ -246,6 +244,7 @@ ioe.initCause(e); throw ioe; } + entryOffset = 0; entrySize = currEntry.getSize(); @@ -284,10 +283,24 @@ // 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.skipFully(buffer, 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 +348,44 @@ * @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 (buffer.read(record) != recordSize) + { + return null; + } + + return record; + } + private void paxHeaders() throws IOException{ Map headers = parsePaxHeaders(this); getNextEntry(); // Get the actual file entry @@ -468,10 +506,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 = buffer.markSupported(); + if (marked) { + buffer.mark(recordSize); + } + try { + shouldReset = !isEOFRecord(readRecord()); + } finally { + if (shouldReset && marked) { + buffer.reset(); + } + } + } /** * Reads bytes from the current tar archive entry. @@ -487,70 +557,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 = buffer.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; } Index: org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java =================================================================== --- org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java (revision 1502405) +++ 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,10 @@ private final byte[] recordBuf; private int assemLen; private final byte[] assemBuf; - protected final TarBuffer buffer; + protected final BufferedOutputStream buffer; private int longFileMode = LONGFILE_ERROR; private int bigNumberMode = BIGNUMBER_ERROR; + private int recordSize; private boolean closed = false; @@ -92,7 +94,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 +104,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 +113,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 +125,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 +151,11 @@ out = new CountingOutputStream(os); this.encoding = ZipEncodingHelper.getZipEncoding(encoding); - this.buffer = new TarBuffer(out, blockSize, recordSize); + this.buffer = new BufferedOutputStream(out, blockSize); this.assemLen = 0; this.assemBuf = new byte[recordSize]; this.recordBuf = new byte[recordSize]; + this.recordSize = recordSize; } /** @@ -217,7 +220,7 @@ } writeEOFRecord(); writeEOFRecord(); - buffer.flushBlock(); + buffer.flush(); finished = true; } @@ -244,7 +247,7 @@ * @return The TarBuffer record size. */ public int getRecordSize() { - return buffer.getRecordSize(); + return this.recordSize; } /** @@ -317,7 +320,7 @@ entry.writeEntryHeader(recordBuf, encoding, bigNumberMode == BIGNUMBER_STAR); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); currBytes = 0; @@ -353,7 +356,7 @@ assemBuf[i] = 0; } - buffer.writeRecord(assemBuf); + writeRecord(assemBuf); currBytes += assemLen; assemLen = 0; @@ -407,7 +410,7 @@ assemLen); System.arraycopy(wBuf, wOffset, recordBuf, assemLen, aLen); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); currBytes += recordBuf.length; wOffset += aLen; @@ -438,7 +441,7 @@ break; } - buffer.writeRecord(wBuf, wOffset); + writeRecord(wBuf, wOffset); int num = recordBuf.length; @@ -512,7 +515,7 @@ */ private void writeEOFRecord() throws IOException { Arrays.fill(recordBuf, (byte) 0); - buffer.writeRecord(recordBuf); + writeRecord(recordBuf); } @Override @@ -529,7 +532,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 + "'"); + } + + buffer.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 + "'"); + } + + buffer.write(buf, offset, recordSize); + } + private void addPaxHeadersForBigNumbers(Map paxHeaders, TarArchiveEntry entry) { addPaxHeaderForBigNumber(paxHeaders, "size", entry.getSize(), Index: org/apache/commons/compress/archivers/tar/TarConstants.java =================================================================== --- org/apache/commons/compress/archivers/tar/TarConstants.java (revision 1502405) +++ 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: org/apache/commons/compress/utils/ArchiveUtils.java =================================================================== --- org/apache/commons/compress/utils/ArchiveUtils.java (revision 1502405) +++ org/apache/commons/compress/utils/ArchiveUtils.java (working copy) @@ -230,5 +230,16 @@ final byte[] buffer2, final int offset2, final int length2){ return isEqual(buffer1, offset1, length1, buffer2, offset2, length2, true); } - + + public static boolean isArrayZero(byte[] a, int size) + { + for (int i = 0; i < size; i++) + { + if (a[i] != 0) + { + return false; + } + } + return true; + } } Index: org/apache/commons/compress/utils/IOUtils.java =================================================================== --- org/apache/commons/compress/utils/IOUtils.java (revision 1502405) +++ org/apache/commons/compress/utils/IOUtils.java (working copy) @@ -19,9 +19,11 @@ package org.apache.commons.compress.utils; import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.Reader; /** * Utility functions @@ -70,7 +72,55 @@ } return count; } + + /** + * Skip the requested number of characters or fail if there are not enough + * left. + *

+ * This allows for the possibility that {@link Reader#skip(long)} may not + * skip as many characters as requested (most likely because of reaching + * EOF). + * + * @param input + * stream to skip + * @param toSkip + * the number of characters to skip + * @see Reader#skip(long) + * + * @throws IOException + * if there is a problem reading the file + * @throws IllegalArgumentException + * if toSkip is negative + * @throws EOFException + * if the number of characters skipped was incorrect + */ + public static void skipFully(InputStream input, long toSkip) throws IOException { + long skipped = skip(input, toSkip); + if (skipped != toSkip) { + throw new EOFException("Chars to skip: " + toSkip + " actual: " + + skipped); + } + } + /** + * + * @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