TIKA-4819: Embedded extractor stateless (#3021)

Author: tballison

CHANGES.txt

@@ -30,6 +30,18 @@ Release 4.0.0 - ???
      API cleanups (cf: prefix, writelimiter package, smaller removals) are
      enumerated in the migration guide, migrating-to-4x.adoc (TIKA-4816).
 
+   * Parsing with a concrete parser (not AutoDetectParser) and an empty
+     ParseContext no longer auto-generates an AutoDetectParser to handle
+     embedded files. Users must set the Parser of their choice for embedded
+     files. EmbeddedDocumentExtractorFactory and friends are removed;
+     ParsingEmbeddedDocumentExtractor/UnpackExtractor are now stateless
+     singletons (TIKA-4819).
+
+   * Parsing with a concrete parser (not AutoDetectParser) and an empty
+     ParseContext no longer auto-generates a Detector to identify embedded
+     files; they are reported as application/octet-stream instead. Users
+     must set the Detector of their choice for embedded files (TIKA-4819).
+
    * tika-server: the /translate endpoints have been removed (TIKA-4809).
 
    * tika-server: /detect/stream and /language/stream and /language/string have

docs/modules/ROOT/pages/advanced/setting-limits.adoc

@@ -342,7 +342,8 @@ malicious files that may expand to enormous sizes when unpacked.
 |`maxUnpackBytes`
 |10 GB
 |Maximum total bytes to extract from all embedded documents per file. Set to -1 for
-unlimited (not recommended for untrusted input).
+unlimited (not recommended for untrusted input). 0 is not unlimited -- it means zero
+bytes.
 |===
 
 === Behavior

docs/modules/ROOT/pages/developers/serialization.adoc

@@ -130,7 +130,7 @@ one of these known interfaces:
 * `Parser`, `Detector`, `EncodingDetector`
 * `MetadataFilter`, `Translator`, `Renderer`
 * `DigesterFactory`, `ContentHandlerFactory`
-* `EmbeddedDocumentExtractorFactory`, `MetadataWriteLimiterFactory`
+* `MetadataWriteLimiterFactory`, `EmbeddedDocumentExtractor`
 
 [source,java]
 ----
@@ -175,7 +175,6 @@ Components not implementing first-class interfaces go to `parse-context.idx`:
 
 * `DigesterFactory` - Digest/checksum calculation
 * `ContentHandlerFactory` - SAX content handler creation
-* `EmbeddedDocumentExtractorFactory` - Embedded document handling
 * `MetadataWriteLimiterFactory` - Metadata write limiting
 
 == Self-Configuring Components
@@ -284,8 +283,8 @@ fail-closed gate on top of the registry:
   `ContentHandlerDecoratorFactory`, `DigesterFactory`,
   `MetadataWriteLimiterFactory`, `UnpackSelector`
 * Types with exec/IO/network capability or control over which components run
-  are blocked: `Parser`, `Detector`, `EncodingDetector`, `Renderer`,
-  `Translator`, `EmbeddedDocumentExtractorFactory`
+  are blocked: `Parser`, `Detector`, `EncodingDetector`, `Renderer`, `Translator`,
+  `EmbeddedDocumentExtractor`
 * The check is fail-closed: a newly added context-key interface is blocked until
   it is consciously allow-listed
 * The whole tree is scanned *before* any component is constructed

docs/modules/ROOT/pages/migration-to-4x/migrating-to-4x.adoc

@@ -242,6 +242,121 @@ External parsers must now be explicitly configured via JSON. See
 xref:configuration/parsers/external-parser.adoc[External Parser Configuration]
 for details.
 
+=== EmbeddedDocumentExtractor is now stateless
+
+[WARNING]
+====
+If you call a concrete parser directly instead of going through `AutoDetectParser`, you
+must set `Parser.class` in the `ParseContext` yourself or embedded documents are
+**silently skipped** -- no content, no exception. The same applies to `Detector.class`:
+without it, embedded documents are reported as `application/octet-stream` instead of
+being identified. See <<bare-context-embedded-skip,Behavior change>> below.
+====
+
+`ParsingEmbeddedDocumentExtractor` (and Tika Pipes' `UnpackExtractor`) no longer capture a
+`ParseContext` at construction. Every method now takes the `ParseContext` of the enclosing
+parse as a parameter, and a single shared instance is reused across parses instead of one
+being built per parse.
+
+**3.x / early 4.x:**
+[source,java]
+----
+EmbeddedDocumentExtractor extractor = new ParsingEmbeddedDocumentExtractor(context);
+if (extractor.shouldParseEmbedded(metadata)) {
+    extractor.parseEmbedded(tis, handler, metadata, outputHtml);
+}
+----
+
+**4.x:**
+[source,java]
+----
+EmbeddedDocumentExtractor extractor = ParsingEmbeddedDocumentExtractor.INSTANCE;
+if (extractor.shouldParseEmbedded(metadata, context)) {
+    extractor.parseEmbedded(tis, handler, metadata, context, outputHtml);
+}
+----
+
+This affects:
+
+* `EmbeddedDocumentExtractor#shouldParseEmbedded(Metadata)` -> `shouldParseEmbedded(Metadata, ParseContext)`.
+  Any custom `EmbeddedDocumentExtractor` implementation must add the parameter.
+* `ParsingEmbeddedDocumentExtractor`'s public constructor is gone -- use the
+  `ParsingEmbeddedDocumentExtractor.INSTANCE` singleton (or `UnpackExtractor.INSTANCE` in
+  Tika Pipes). A subclass that called `super(context)` should drop the constructor and read
+  `ParseContext` from the method parameter instead of a captured field.
+* `ParsingEmbeddedDocumentExtractor#checkEmbeddedLimits(ParseRecord)` ->
+  `checkEmbeddedLimits(ParseRecord, ParseContext)`, and `isWriteFileNameToContent()` ->
+  `isWriteFileNameToContent(ParseContext)`. A subclass overriding either must add the
+  parameter -- without `@Override`, the old signature silently becomes a dead, unused overload
+  instead of a compile error.
+* `EmbeddedDocumentExtractorFactory`, `EmbeddedDocumentByteStoreExtractorFactory`,
+  `StandardExtractorFactory`, and Tika Pipes' `UnpackExtractorFactory` are deleted -- there is
+  no longer a per-parse object to build. Code that supplied a custom factory should instead
+  bind an `EmbeddedDocumentExtractor` instance directly:
++
+[source,java]
+----
+// 3.x/early 4.x
+context.set(EmbeddedDocumentExtractorFactory.class, new MyExtractorFactory());
+
+// 4.x
+context.set(EmbeddedDocumentExtractor.class, MyExtractor.INSTANCE);
+----
+* `EmbeddedDocumentUtil`'s instance API is removed (the constructor, and the instance methods
+  `getPasswordProvider()`, `getDetector()`, `getMimeTypes()`, `getExtension(TikaInputStream,
+  Metadata)`, `shouldParseEmbedded(Metadata)`, `parseEmbedded(...)`). Use the static
+  replacements, which take `ParseContext` explicitly: `EmbeddedDocumentUtil.getDetector(context)`,
+  `EmbeddedDocumentUtil.getMimeTypes(context)`,
+  `EmbeddedDocumentUtil.getExtension(tis, metadata, context)`, or call the methods on the
+  `EmbeddedDocumentExtractor` obtained from `EmbeddedDocumentUtil.getEmbeddedDocumentExtractor(context)`
+  directly. `getPasswordProvider()` had no replacement added since it had no callers;
+  use `context.get(PasswordProvider.class)`.
+
+[[bare-context-embedded-skip]]
+==== Behavior change: parsing a concrete parser directly with a bare `ParseContext`
+
+[WARNING]
+====
+Calling a concrete parser directly (bypassing `AutoDetectParser`) with a `ParseContext` that
+has no `Parser.class` set now **silently skips embedded documents** instead of constructing an
+SPI-discovered `AutoDetectParser` to parse them:
+
+[source,java]
+----
+new PDFParser().parse(tis, handler, metadata, new ParseContext());
+// 3.x/early 4.x: embedded documents parsed by an SPI-discovered AutoDetectParser
+//                (bypassing whatever parser/limits/selectors the caller actually configured)
+// 4.x: embedded documents are skipped -- no content, no exception
+----
+
+If you rely on embedded documents being parsed, set `Parser.class` in the `ParseContext`
+(typically to an `AutoDetectParser`) before calling a concrete parser directly, or go through
+`AutoDetectParser` in the first place, which does this for you automatically.
+====
+
+[[bare-context-detector-noop]]
+==== Behavior change: identifying embedded files with a bare `ParseContext`
+
+[WARNING]
+====
+Some container parsers (`OpenDocumentParser`, the POIFS-based Office parsers, `RFC822Parser`)
+detect each embedded file's media type to label it in the output metadata. With a bare
+`ParseContext` (no `Detector.class` set), this now reports every embedded file as
+`application/octet-stream` instead of constructing an SPI-discovered `DefaultDetector`:
+
+[source,java]
+----
+new OpenDocumentParser().parse(tis, handler, metadata, new ParseContext());
+// 3.x/early 4.x: embedded pictures identified by an SPI-discovered DefaultDetector
+//                (e.g. image/jpeg, image/png)
+// 4.x: embedded pictures are all labeled application/octet-stream
+----
+
+If you rely on embedded files being identified, set `Detector.class` in the `ParseContext`
+before calling a concrete parser directly, or go through `AutoDetectParser` in the first
+place, which does this for you automatically.
+====
+
 == Timeout Model Changes
 
 4.x replaces the previous ad hoc, per-parser timeout handling with a single unified model
@@ -286,3 +401,11 @@ the full list of behavioral changes and required config edits.
   URL, XPATH}` -- dead enum constants, never produced by any `Property` factory
 * `Property.internalClosedChoise`/`internalOpenChoise`/`externalClosedChoise`/`externalOpenChoise`
   -- renamed to `...Choice` (typo-fix rename, no forwarders)
+* `EmbeddedDocumentExtractorFactory`, `EmbeddedDocumentByteStoreExtractorFactory`,
+  `StandardExtractorFactory`, `UnpackExtractorFactory` -- deleted; bind an
+  `EmbeddedDocumentExtractor` instance directly instead (see
+  <<EmbeddedDocumentExtractor is now stateless>> above)
+* `ParsingEmbeddedDocumentExtractor(ParseContext)` constructor -- removed; use the
+  `ParsingEmbeddedDocumentExtractor.INSTANCE` singleton
+* `EmbeddedDocumentUtil`'s instance API (constructor and all instance methods) -- removed;
+  use the static equivalents, which now take `ParseContext` explicitly

docs/modules/ROOT/pages/pipes/unpack-config.adoc

@@ -83,7 +83,9 @@ All options below are fields of the `unpack-config` block — nest them inside
 |`maxUnpackBytes`
 |long
 |10GB
-|Maximum total bytes to extract per file. Set to `-1` for unlimited (not recommended).
+|Maximum total bytes to extract per file. Set to `-1` for unlimited (not recommended). `0`
+is not unlimited -- it means zero bytes, so the first embedded file's extraction is
+immediately capped.
 
 |`includeOriginal`
 |boolean

tika-core/src/main/java/org/apache/tika/detect/NoOpDetector.java

@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.detect;
+
+import java.io.IOException;
+
+import org.apache.tika.annotation.TikaComponent;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.mime.MediaType;
+import org.apache.tika.parser.ParseContext;
+
+/**
+ * Dummy detector that always reports {@code application/octet-stream} without even
+ * reading the given document stream. Useful as a sentinel default: reporting an honest
+ * "unknown" is preferable to a partially-informed guess from an unconfigured detector.
+ */
+@TikaComponent(spi = false)
+public class NoOpDetector implements Detector {
+
+    public static final NoOpDetector INSTANCE = new NoOpDetector();
+
+    private static final long serialVersionUID = 1L;
+
+    @Override
+    public MediaType detect(TikaInputStream tis, Metadata metadata, ParseContext parseContext)
+            throws IOException {
+        // Honor the Detector contract (mark before reading, reset before returning) even though
+        // nothing is read: callers may rely on the mark still being valid for their own reset().
+        if (tis != null) {
+            tis.mark(1);
+            tis.reset();
+        }
+        return MediaType.OCTET_STREAM;
+    }
+}

tika-core/src/main/java/org/apache/tika/extractor/EmbeddedDocumentByteStoreExtractorFactory.java

@@ -25,17 +25,23 @@
 import org.apache.tika.metadata.Metadata;
 import org.apache.tika.parser.ParseContext;
 
+/**
+ * Implementations are typically bound as a single shared instance reused across concurrent
+ * parses (see {@link ParsingEmbeddedDocumentExtractor#INSTANCE}) -- do not cache per-parse
+ * state on {@code this}; read it from the {@link ParseContext} parameter on each call instead.
+ */
 public interface EmbeddedDocumentExtractor {
     /**
      * Determines whether the given embedded document should be parsed.
      * <p>
      * Note: Implementations may throw {@link org.apache.tika.exception.EmbeddedLimitReachedException}
      * (a RuntimeException) if a limit is exceeded and throwing is configured.
      *
-     * @param metadata the metadata for the embedded document
+     * @param metadata     the metadata for the embedded document
+     * @param parseContext the parse context of the enclosing parse
      * @return true if the embedded document should be parsed
      */
-    boolean shouldParseEmbedded(Metadata metadata);
+    boolean shouldParseEmbedded(Metadata metadata, ParseContext parseContext);
 
     /**
      * Processes the supplied embedded resource, calling the delegating