You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tika.apache.org by ta...@apache.org on 2020/03/09 01:45:23 UTC
[tika] branch branch_1x updated: TIKA-3064 -- improve docs on
BoundedInputStream and use more often.
This is an automated email from the ASF dual-hosted git repository.
tallison pushed a commit to branch branch_1x
in repository https://gitbox.apache.org/repos/asf/tika.git
The following commit(s) were added to refs/heads/branch_1x by this push:
new 16a8b52 TIKA-3064 -- improve docs on BoundedInputStream and use more often.
16a8b52 is described below
commit 16a8b5296b4d003e94ff764f81fe9c387cb9661d
Author: tallison <ta...@apache.org>
AuthorDate: Sun Mar 8 21:28:21 2020 -0400
TIKA-3064 -- improve docs on BoundedInputStream and use more often.
---
.../org/apache/tika/io/BoundedInputStream.java | 7 +++
.../src/main/java/org/apache/tika/io/IOUtils.java | 50 ++++++++++++++++++++++
.../java/org/apache/tika/parser/NetworkParser.java | 1 -
.../tika/parser/pdf/ImageGraphicsEngine.java | 41 ++++++++++++++----
.../parser/pkg/StreamingZipContainerDetector.java | 16 ++++---
.../apache/tika/parser/rtf/RTFObjDataParser.java | 19 +++++---
.../tika/server/resource/UnpackerResource.java | 15 ++++++-
7 files changed, 126 insertions(+), 23 deletions(-)
diff --git a/tika-core/src/main/java/org/apache/tika/io/BoundedInputStream.java b/tika-core/src/main/java/org/apache/tika/io/BoundedInputStream.java
index dabedf5..4dbdee1 100644
--- a/tika-core/src/main/java/org/apache/tika/io/BoundedInputStream.java
+++ b/tika-core/src/main/java/org/apache/tika/io/BoundedInputStream.java
@@ -22,6 +22,9 @@ import java.io.InputStream;
/**
* Very slight modification of Commons' BoundedInputStream
* so that we can figure out if this hit the bound or not.
+ *
+ * This relies on IOUtils' skip and read to try to fully
+ * read/skip inputstream.
*/
public class BoundedInputStream extends InputStream {
@@ -62,6 +65,8 @@ public class BoundedInputStream extends InputStream {
/**
* Invokes the delegate's <code>read(byte[], int, int)</code> method.
*
+ * This does not have the same guarantees as IOUtil's readFully()...be careful.
+ *
* @param b the buffer to read the bytes into
* @param off The start offset
* @param len The number of bytes to read
@@ -87,6 +92,8 @@ public class BoundedInputStream extends InputStream {
/**
* Invokes the delegate's <code>skip(long)</code> method.
+ * As with InputStream generally, this does not guarantee reading n bytes.
+ * Use IOUtils' skipFully for that functionality.
*
* @param n the number of bytes to skip
* @return the actual number of bytes skipped
diff --git a/tika-core/src/main/java/org/apache/tika/io/IOUtils.java b/tika-core/src/main/java/org/apache/tika/io/IOUtils.java
index a05176f..18e86f1 100644
--- a/tika-core/src/main/java/org/apache/tika/io/IOUtils.java
+++ b/tika-core/src/main/java/org/apache/tika/io/IOUtils.java
@@ -80,6 +80,12 @@ public class IOUtils {
public static final Charset UTF_8 = java.nio.charset.StandardCharsets.UTF_8;
/**
+ * The default buffer size to use for the skip() methods.
+ */
+ private static final int SKIP_BUFFER_SIZE = 2048;
+
+ private static byte[] SKIP_BYTE_BUFFER;
+ /**
* The default buffer size to use.
*/
private static final int DEFAULT_BUFFER_SIZE = 1024 * 4;
@@ -1214,4 +1220,48 @@ public class IOUtils {
return length - remaining;
}
+ /**
+ * Skips bytes from an input byte stream.
+ * This implementation guarantees that it will read as many bytes
+ * as possible before giving up; this may not always be the case for
+ * skip() implementations in subclasses of {@link InputStream}.
+ * <p>
+ * Note that the implementation uses {@link InputStream#read(byte[], int, int)} rather
+ * than delegating to {@link InputStream#skip(long)}.
+ * This means that the method may be considerably less efficient than using the actual skip implementation,
+ * this is done to guarantee that the correct number of bytes are skipped.
+ * </p>
+ *
+ * @param input byte stream to skip
+ * @param toSkip number of bytes to skip.
+ * @return number of bytes actually skipped.
+ * @throws IOException if there is a problem reading the file
+ * @throws IllegalArgumentException if toSkip is negative
+ * @see InputStream#skip(long)
+ * @see <a href="https://issues.apache.org/jira/browse/IO-203">IO-203 - Add skipFully() method for InputStreams</a>
+ */
+ public static long skip(final InputStream input, final long toSkip) throws IOException {
+ if (toSkip < 0) {
+ throw new IllegalArgumentException("Skip count must be non-negative, actual: " + toSkip);
+ }
+ /*
+ * N.B. no need to synchronize this because: - we don't care if the buffer is created multiple times (the data
+ * is ignored) - we always use the same size buffer, so if it it is recreated it will still be OK (if the buffer
+ * size were variable, we would need to synch. to ensure some other thread did not create a smaller one)
+ */
+ if (SKIP_BYTE_BUFFER == null) {
+ SKIP_BYTE_BUFFER = new byte[SKIP_BUFFER_SIZE];
+ }
+ long remain = toSkip;
+ while (remain > 0) {
+ // See https://issues.apache.org/jira/browse/IO-203 for why we use read() rather than delegating to skip()
+ final long n = input.read(SKIP_BYTE_BUFFER, 0, (int) Math.min(remain, SKIP_BUFFER_SIZE));
+ if (n < 0) { // EOF
+ break;
+ }
+ remain -= n;
+ }
+ return toSkip - remain;
+ }
+
}
diff --git a/tika-core/src/main/java/org/apache/tika/parser/NetworkParser.java b/tika-core/src/main/java/org/apache/tika/parser/NetworkParser.java
index 65b5d53..4390b0d 100644
--- a/tika-core/src/main/java/org/apache/tika/parser/NetworkParser.java
+++ b/tika-core/src/main/java/org/apache/tika/parser/NetworkParser.java
@@ -43,7 +43,6 @@ import org.xml.sax.ContentHandler;
import org.xml.sax.SAXException;
import org.xml.sax.helpers.DefaultHandler;
-import javax.xml.parsers.SAXParser;
public class NetworkParser extends AbstractParser {
diff --git a/tika-parsers/src/main/java/org/apache/tika/parser/pdf/ImageGraphicsEngine.java b/tika-parsers/src/main/java/org/apache/tika/parser/pdf/ImageGraphicsEngine.java
index 3f0c9d0..a469de8 100644
--- a/tika-parsers/src/main/java/org/apache/tika/parser/pdf/ImageGraphicsEngine.java
+++ b/tika-parsers/src/main/java/org/apache/tika/parser/pdf/ImageGraphicsEngine.java
@@ -39,8 +39,11 @@ import org.apache.pdfbox.pdmodel.graphics.state.RenderingMode;
import org.apache.pdfbox.tools.imageio.ImageIOUtil;
import org.apache.pdfbox.util.Matrix;
import org.apache.pdfbox.util.Vector;
+import org.apache.tika.exception.TikaException;
+import org.apache.tika.exception.TikaMemoryLimitException;
import org.apache.tika.extractor.EmbeddedDocumentExtractor;
import org.apache.tika.extractor.EmbeddedDocumentUtil;
+import org.apache.tika.io.BoundedInputStream;
import org.apache.tika.io.IOExceptionWithCause;
import org.apache.tika.io.TikaInputStream;
import org.apache.tika.metadata.Metadata;
@@ -68,6 +71,9 @@ import java.util.concurrent.atomic.AtomicInteger;
*/
class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
+ //We're currently copying images to byte[]. We should
+ //limit the length to avoid OOM on crafted files.
+ private static final long MAX_IMAGE_LENGTH_BYTES = 100*1024*1024;
private static final List<String> JPEG = Arrays.asList(
COSName.DCT_DECODE.getName(),
@@ -167,7 +173,7 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
//duplicates if the pdImage is not a PDImageXObject?
try {
processImage(pdImage, imageNumber);
- } catch (SAXException e) {
+ } catch (TikaException|SAXException e) {
throw new IOExceptionWithCause(e);
} catch (IOException e) {
handleCatchableIOE(e);
@@ -265,7 +271,7 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
}
}
- private void processImage(PDImage pdImage, int imageNumber) throws IOException, SAXException {
+ private void processImage(PDImage pdImage, int imageNumber) throws IOException, TikaException, SAXException {
//this is the metadata for this particular image
Metadata metadata = new Metadata();
String suffix = getSuffix(pdImage, metadata);
@@ -365,7 +371,7 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
//nearly directly copied from PDFBox ExtractImages
private static void writeToBuffer(PDImage pdImage, String suffix, boolean directJPEG, OutputStream out)
- throws IOException {
+ throws IOException, TikaException {
if ("jpg".equals(suffix)) {
@@ -375,8 +381,11 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
PDDeviceRGB.INSTANCE.getName().equals(colorSpaceName))) {
// RGB or Gray colorspace: get and write the unmodified JPEG stream
InputStream data = pdImage.createInputStream(JPEG);
- IOUtils.copy(data, out);
- IOUtils.closeQuietly(data);
+ try {
+ copyUpToMaxLength(data, out);
+ } finally {
+ IOUtils.closeQuietly(data);
+ }
} else {
BufferedImage image = pdImage.getImage();
if (image != null) {
@@ -392,8 +401,11 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
PDDeviceRGB.INSTANCE.getName().equals(colorSpaceName))) {
// RGB or Gray colorspace: get and write the unmodified JPEG2000 stream
InputStream data = pdImage.createInputStream(JP2);
- IOUtils.copy(data, out);
- IOUtils.closeQuietly(data);
+ try {
+ copyUpToMaxLength(data, out);
+ } finally {
+ IOUtils.closeQuietly(data);
+ }
} else {
// for CMYK and other "unusual" colorspaces, the image will be converted
BufferedImage image = pdImage.getImage();
@@ -422,8 +434,11 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
ImageIOUtil.writeImage(bitonalImage, suffix, out);
} else if ("jb2".equals(suffix)) {
InputStream data = pdImage.createInputStream(JB2);
- org.apache.pdfbox.io.IOUtils.copy(data, out);
- org.apache.pdfbox.io.IOUtils.closeQuietly(data);
+ try {
+ copyUpToMaxLength(data, out);
+ } finally {
+ IOUtils.closeQuietly(data);
+ }
} else {
BufferedImage image = pdImage.getImage();
if (image == null) {
@@ -435,6 +450,14 @@ class ImageGraphicsEngine extends PDFGraphicsStreamEngine {
out.flush();
}
+ private static void copyUpToMaxLength(InputStream is, OutputStream os) throws IOException, TikaException {
+ BoundedInputStream bis = new BoundedInputStream(MAX_IMAGE_LENGTH_BYTES, is);
+ IOUtils.copy(bis, os);
+ if (bis.hasHitBound()) {
+ throw new TikaMemoryLimitException("Image size is larger than allowed (" + MAX_IMAGE_LENGTH_BYTES + ")");
+ }
+
+ }
private static boolean hasMasks(PDImage pdImage) throws IOException {
if (pdImage instanceof PDImageXObject) {
PDImageXObject ximg = (PDImageXObject) pdImage;
diff --git a/tika-parsers/src/main/java/org/apache/tika/parser/pkg/StreamingZipContainerDetector.java b/tika-parsers/src/main/java/org/apache/tika/parser/pkg/StreamingZipContainerDetector.java
index a39f9f4..5570e06 100644
--- a/tika-parsers/src/main/java/org/apache/tika/parser/pkg/StreamingZipContainerDetector.java
+++ b/tika-parsers/src/main/java/org/apache/tika/parser/pkg/StreamingZipContainerDetector.java
@@ -34,6 +34,7 @@ import org.apache.poi.xslf.usermodel.XSLFRelation;
import org.apache.poi.xssf.usermodel.XSSFRelation;
import org.apache.poi.xwpf.usermodel.XWPFRelation;
import org.apache.tika.detect.Detector;
+import org.apache.tika.io.BoundedInputStream;
import org.apache.tika.io.CloseShieldInputStream;
import org.apache.tika.metadata.Metadata;
import org.apache.tika.mime.MediaType;
@@ -49,6 +50,9 @@ import org.xml.sax.helpers.DefaultHandler;
public class StreamingZipContainerDetector extends ZipContainerDetectorBase implements Detector {
+ private static final int MAX_MIME_TYPE = 1024;
+ private static final int MAX_MANIFEST = 20 * 1024 * 1024;
+
static Map<String, MediaType> OOXML_CONTENT_TYPES = new ConcurrentHashMap<>();
static {
OOXML_CONTENT_TYPES.put(XWPFRelation.DOCUMENT.getContentType(), DOCX);
@@ -132,11 +136,11 @@ public class StreamingZipContainerDetector extends ZipContainerDetectorBase impl
//can't rely on zae.getSize to determine if there is any
//content here. :(
ByteArrayOutputStream bos = new ByteArrayOutputStream();
- IOUtils.copy(zipArchiveInputStream, bos);
-
+ BoundedInputStream bis = new BoundedInputStream(MAX_MIME_TYPE, zipArchiveInputStream);
+ IOUtils.copy(bis, bos);
+ //do anything with an inputstream > MAX_MIME_TYPE?
if (bos.toByteArray().length > 0) {
- //odt -- TODO -- bound the read and check that the results are
- //valid
+ //odt -- TODO -- check that the results are valid
return MediaType.parse(new String(bos.toByteArray(), UTF_8));
}
} else if (name.equals("META-INF/manifest.xml")) {
@@ -145,7 +149,9 @@ public class StreamingZipContainerDetector extends ZipContainerDetectorBase impl
//without exception or warning. So, copy the full stream, then
//process. TIKA-3061
ByteArrayOutputStream bos = new ByteArrayOutputStream();
- IOUtils.copy(zipArchiveInputStream, bos);
+ BoundedInputStream bis = new BoundedInputStream(MAX_MANIFEST, zipArchiveInputStream);
+ IOUtils.copy(bis, bos);
+ //TODO: do something if the full stream hasn't been read?
MediaType mt = detectStarOfficeX(new ByteArrayInputStream(bos.toByteArray()));
if (mt != null) {
return mt;
diff --git a/tika-parsers/src/main/java/org/apache/tika/parser/rtf/RTFObjDataParser.java b/tika-parsers/src/main/java/org/apache/tika/parser/rtf/RTFObjDataParser.java
index 1f4e29e..7d6d24f 100644
--- a/tika-parsers/src/main/java/org/apache/tika/parser/rtf/RTFObjDataParser.java
+++ b/tika-parsers/src/main/java/org/apache/tika/parser/rtf/RTFObjDataParser.java
@@ -40,8 +40,8 @@ import org.apache.poi.util.IOUtils;
import org.apache.tika.exception.TikaException;
import org.apache.tika.exception.TikaMemoryLimitException;
import org.apache.tika.extractor.EmbeddedDocumentUtil;
+import org.apache.tika.io.BoundedInputStream;
import org.apache.tika.io.EndianUtils;
-import org.apache.tika.io.TikaInputStream;
import org.apache.tika.metadata.Metadata;
import org.apache.tika.metadata.RTFMetadata;
import org.apache.tika.metadata.TikaCoreProperties;
@@ -137,7 +137,7 @@ class RTFObjDataParser {
//can return null byte[]
private byte[] handleEmbeddedPOIFS(InputStream is, Metadata metadata,
AtomicInteger unknownFilenameCount)
- throws IOException {
+ throws TikaException, IOException {
byte[] ret = null;
try (POIFSFileSystem fs = new POIFSFileSystem(is)) {
@@ -150,11 +150,14 @@ class RTFObjDataParser {
if (root.hasEntry("Package")) {
Entry ooxml = root.getEntry("Package");
- TikaInputStream stream = TikaInputStream.get(new DocumentInputStream((DocumentEntry) ooxml));
-
ByteArrayOutputStream out = new ByteArrayOutputStream();
-
- IOUtils.copy(stream, out);
+ try (BoundedInputStream bis = new BoundedInputStream(memoryLimitInKb*1024,
+ new DocumentInputStream((DocumentEntry)ooxml))) {
+ IOUtils.copy(bis, out);
+ if (bis.hasHitBound()) {
+ throw new TikaMemoryLimitException("Hit memory limit exception. Tried to copy > "+memoryLimitInKb*1024);
+ }
+ }
ret = out.toByteArray();
} else {
//try poifs
@@ -184,7 +187,11 @@ class RTFObjDataParser {
ByteArrayOutputStream out = new ByteArrayOutputStream();
is.reset();
+ BoundedInputStream bis = new BoundedInputStream(memoryLimitInKb*1024, is);
IOUtils.copy(is, out);
+ if (bis.hasHitBound()) {
+ throw new TikaMemoryLimitException("Hit memory limit exception. Tried to copy > "+memoryLimitInKb*1024);
+ }
ret = out.toByteArray();
metadata.set(Metadata.RESOURCE_NAME_KEY, "file_" + unknownFilenameCount.getAndIncrement() + "." + type.getExtension());
metadata.set(Metadata.CONTENT_TYPE, type.getType().toString());
diff --git a/tika-server/src/main/java/org/apache/tika/server/resource/UnpackerResource.java b/tika-server/src/main/java/org/apache/tika/server/resource/UnpackerResource.java
index 15b7486..270166b 100644
--- a/tika-server/src/main/java/org/apache/tika/server/resource/UnpackerResource.java
+++ b/tika-server/src/main/java/org/apache/tika/server/resource/UnpackerResource.java
@@ -50,7 +50,10 @@ import org.apache.poi.poifs.filesystem.Ole10Native;
import org.apache.poi.poifs.filesystem.Ole10NativeException;
import org.apache.poi.poifs.filesystem.POIFSFileSystem;
import org.apache.poi.util.IOUtils;
+import org.apache.tika.exception.TikaMemoryLimitException;
import org.apache.tika.extractor.EmbeddedDocumentExtractor;
+import org.apache.tika.io.BoundedInputStream;
+import org.apache.tika.io.IOExceptionWithCause;
import org.apache.tika.io.TikaInputStream;
import org.apache.tika.metadata.Metadata;
import org.apache.tika.metadata.TikaMetadataKeys;
@@ -69,6 +72,8 @@ import org.xml.sax.helpers.DefaultHandler;
@Path("/unpack")
public class UnpackerResource {
+ private static final long MAX_ATTACHMENT_BYTES = 100*1024*1024;
+
public static final String TEXT_FILENAME = "__TEXT__";
private static final String META_FILENAME = "__METADATA__";
@@ -172,9 +177,15 @@ public class UnpackerResource {
return true;
}
- public void parseEmbedded(InputStream inputStream, ContentHandler contentHandler, Metadata metadata, boolean b) throws SAXException, IOException {
+ public void parseEmbedded(InputStream inputStream, ContentHandler contentHandler, Metadata metadata, boolean b)
+ throws SAXException, IOException {
ByteArrayOutputStream bos = new ByteArrayOutputStream();
- IOUtils.copy(inputStream, bos);
+ BoundedInputStream bis = new BoundedInputStream(MAX_ATTACHMENT_BYTES, inputStream);
+ IOUtils.copy(bis, bos);
+ if (bis.hasHitBound()) {
+ throw new IOExceptionWithCause(
+ new TikaMemoryLimitException("Tried to read beyond "+MAX_ATTACHMENT_BYTES + "bytes"));
+ }
byte[] data = bos.toByteArray();
String name = metadata.get(TikaMetadataKeys.RESOURCE_NAME_KEY);