[HER-1878] NullPointerException when parsing (some?) WARC files over slow connections

* GZIPMembersInputStream
    override OpenJDK7GZIPInputStream.readTrailer to not rely on available() as indicator of end-of-stream
    fix membersIterator hasNext() to also not rely on available() as indicator of end-of-stream
* ArchiveReader
    change innerHasNext to peek one byte (using mark/reset) rather than rely on available() as indicator of end-of-stream
    remove redundant getInputStream accessor in favor of field-name-matching getIn/setIn
* ArchiveReaderFactory, (W)ARCReaderFactory
    remove no-longer-necessary RepositionableStream references
    use getIn() rather than getInputStream()
This commit is contained in:
gojomo
2011-04-11 22:58:05 +00:00
parent bd45720bae
commit 6388dc7605
5 changed files with 50 additions and 49 deletions
@@ -66,10 +66,10 @@ public abstract class ArchiveReader implements ArchiveFileConstants, Iterable<Ar
*
* Keep it around so we can close it when done.
*
* <p>Set in constructor. Must support {@link RepositionableStream}
* interface. Make it protected so subclasses have access.
* <p>Set in constructor. Should support at least 1 byte mark/reset.
* Make it protected so subclasses have access.
*/
private InputStream in = null;
protected InputStream in = null;
/**
* Maximum amount of recoverable exceptions in a row.
@@ -316,10 +316,6 @@ public abstract class ArchiveReader implements ArchiveFileConstants, Iterable<Ar
return Logger.getLogger(this.getClass().getName());
}
protected InputStream getInputStream() {
return this.in;
}
/**
* Returns an ArchiveRecord iterator.
* Of note, on IOException, especially if ZipException reading compressed
@@ -467,12 +463,16 @@ public abstract class ArchiveReader implements ArchiveFileConstants, Iterable<Ar
return innerHasNext();
}
protected boolean innerHasNext() {
protected boolean innerHasNext(){
try {
return getInputStream().available() > 0;
getIn().mark(1);
int c = getIn().read();
getIn().reset();
return c > -1;
} catch (IOException e) {
throw new RuntimeException(e);
}
logger.log(Level.WARNING,"problem probing for more content",e);
return false;
}
}
/**
@@ -18,8 +18,6 @@
*/
package org.archive.io;
import it.unimi.dsi.fastutil.io.RepositionableStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
@@ -147,26 +145,10 @@ public class ArchiveReaderFactory implements ArchiveFileConstants {
atFirstRecord);
}
/**
* @param is
* @return If passed <code>is</code> is
* {@link RepositionableInputStream}, returns <code>is</code>, else we
* wrap <code>is</code> with {@link RepositionableStream}.
*/
protected InputStream asRepositionable(final InputStream is) {
if (is instanceof RepositionableStream) {
return is;
}
// RepositionableInputStream calls mark on each read so can back up at
// least the read amount. Needed for gzip inflater overinflations
// reading into the next gzip member.
return new RepositionableInputStream(is, 16 * 1024);
}
protected ArchiveReader getArchiveReader(final String id,
final InputStream is, final boolean atFirstRecord)
throws IOException {
final InputStream stream = asRepositionable(is);
final InputStream stream = is;
if (ARCReaderFactory.isARCSuffix(id)) {
return ARCReaderFactory.get(id, stream, atFirstRecord);
} else if (WARCReaderFactory.isWARCSuffix(id)) {
@@ -204,8 +186,7 @@ public class ArchiveReaderFactory implements ArchiveFileConstants {
connection.addRequestProperty("Range", "bytes=" + offset + "-");
}
return getArchiveReader(f.toString(), connection.getInputStream(),
(offset == 0));
return getArchiveReader(f.toString(), connection.getInputStream(), (offset == 0));
}
/**
@@ -17,11 +17,14 @@
* limitations under the License.
*/
package org.archive.io;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.SequenceInputStream;
import java.util.Iterator;
import java.util.NoSuchElementException;
import java.util.zip.Inflater;
import java.util.zip.ZipException;
import org.archive.util.zip.OpenJDK7GZIPInputStream;
@@ -112,9 +115,34 @@ public class GZIPMembersInputStream extends OpenJDK7GZIPInputStream {
@Override
protected boolean readTrailer() throws IOException {
int c = inf.getRemaining();
currentMemberEnd = ((CountingInputStream)in).getCount()-(c-8);
// return super.readTrailer();
// REIMPLEMENTED TO FIX MISUSE OF available()
InputStream in = this.in;
int n = inf.getRemaining();
currentMemberEnd = ((CountingInputStream)in).getCount()-(n-8);
return super.readTrailer();
if (n > 0) {
in = new SequenceInputStream(
new ByteArrayInputStream(buf, len - n, n), in);
}
// Uses left-to-right evaluation order
if ((readUInt(in) != crc.getValue()) ||
// rfc1952; ISIZE is the input size modulo 2^32
(readUInt(in) != (inf.getBytesWritten() & 0xffffffffL)))
throw new ZipException("Corrupt GZIP trailer");
// always try concatenated case; EOF or other IOException
// will let us know if we're wrong
int m = 8; // this.trailer
try {
m += readHeader(in); // next.header
} catch (IOException ze) {
return true; // ignore any malformed, do nothing
}
inf.reset();
if (n > m)
inf.setInput(buf, len - n + m, n - m);
return false;
}
/**
@@ -267,11 +295,9 @@ public class GZIPMembersInputStream extends OpenJDK7GZIPInputStream {
@Override
public boolean hasNext() {
try {
return (in.available()>0 || inf.getRemaining() > 8);
} catch (IOException e) {
throw new RuntimeException(e);
}
// because readTrailer also reads into next header
// resetting inflater when there's more content, this works
return !inf.finished();
}
@Override
@@ -37,9 +37,7 @@ import org.archive.io.GzipHeader;
import org.archive.io.NoGzipMagicException;
import org.archive.util.FileUtils;
import com.google.common.io.ByteStreams;
import com.google.common.io.CountingInputStream;
import com.google.common.io.NullOutputStream;
/**
@@ -136,8 +134,7 @@ implements ARCConstants {
throws IOException {
// For now, assume stream is compressed. Later add test of input
// stream or handle exception thrown when figure not compressed stream.
return new CompressedARCReader(arc, asRepositionable(is),
atFirstRecord);
return new CompressedARCReader(arc, is, atFirstRecord);
}
/**
@@ -393,7 +390,7 @@ implements ARCConstants {
*/
return new ArchiveRecordIterator() {
private GZIPMembersInputStream gis =
(GZIPMembersInputStream)getInputStream();
(GZIPMembersInputStream)getIn();
private Iterator<GZIPMembersInputStream> gzipIterator = this.gis.memberIterator();
@@ -26,18 +26,15 @@ import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Iterator;
import java.util.logging.Level;
import org.archive.io.ArchiveReader;
import org.archive.io.ArchiveReaderFactory;
import org.archive.io.ArchiveRecord;
import org.archive.io.ArchiveRecordHeader;
import org.archive.io.GZIPMembersInputStream;
import org.archive.io.warc.WARCConstants;
import org.archive.util.ArchiveUtils;
import org.archive.util.FileUtils;
import com.google.common.io.ByteStreams;
import com.google.common.io.CountingInputStream;
/**
@@ -108,7 +105,7 @@ implements WARCConstants {
throws IOException {
// For now, assume stream is compressed. Later add test of input
// stream or handle exception thrown when figure not compressed stream.
return new CompressedWARCReader(f, asRepositionable(is), atFirstRecord);
return new CompressedWARCReader(f, is, atFirstRecord);
}
public static WARCReader get(final URL arcUrl, final long offset)
@@ -270,7 +267,7 @@ implements WARCConstants {
*/
return new ArchiveRecordIterator() {
private GZIPMembersInputStream gis =
(GZIPMembersInputStream)getInputStream();
(GZIPMembersInputStream)getIn();
private Iterator<GZIPMembersInputStream> gzipIterator = this.gis.memberIterator();