This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4808-simplify-server-serialization in repository https://gitbox.apache.org/repos/asf/tika.git
commit 659f88ab8f978011dbfb959df6376b03714bdbdb Author: tallison <[email protected]> AuthorDate: Mon Aug 17 07:35:16 2026 -0400 TIKA-4808: tika-server sends only request deltas to the pipes worker --- CHANGES.txt | 7 + .../server/core/resource/DetectorResource.java | 4 +- .../server/core/resource/MetadataResource.java | 23 ++- .../server/core/resource/PipesParsingHelper.java | 8 +- .../core/resource/RecursiveMetadataResource.java | 18 +- .../tika/server/core/resource/TikaResource.java | 107 +++++++++-- .../server/core/resource/UnpackerResource.java | 27 ++- .../org/apache/tika/server/core/CXFTestBase.java | 43 +++-- .../core/resource/RequestContextIsolationTest.java | 214 +++++++++++++++++++++ .../standard/resource/XMPMetadataResource.java | 8 +- 10 files changed, 384 insertions(+), 75 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5ae4bb52ed..382d18f807 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -138,6 +138,13 @@ Release 4.0.0 - ??? * tika-server: /meta now runs through the same pipes-backed parser as the other extraction endpoints (TIKA-4809). + * tika-server requests now carry only their own parse-context entries to the + forked worker, which supplies the config defaults itself. Previously the + server sent its config's parse-context along with every request, so the + worker -- which clamps request-supplied timeout limits but trusts its own + config's -- treated the operator's timeout-limits as caller input and + clamped them at pipes.maxTotalTaskTimeoutMillis (TIKA-4808). + * tika-grpc's fetchAndParseServerSideStreaming now completes the call after delivering its reply, instead of leaving the client waiting for a terminal signal that never came (TIKA-4804). diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/DetectorResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/DetectorResource.java index 12b52c11c7..e3d2c9588d 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/DetectorResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/DetectorResource.java @@ -54,8 +54,8 @@ public class DetectorResource { @Consumes("*/*") @Produces("text/plain") public String detect(final InputStream is, @Context HttpHeaders httpHeaders, @Context final UriInfo info) { - ParseContext parseContext = tikaResource.createParseContext(); - Metadata met = Metadata.newInstance(parseContext); + ParseContext parseContext = tikaResource.createRequestContext(); + Metadata met = tikaResource.newRequestMetadata(); String filename = TikaResource.detectFilename(httpHeaders.getRequestHeaders()); LOG.debug("Detecting media type for Filename: {}", filename); diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/MetadataResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/MetadataResource.java index 8af6c93762..4477c3cdda 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/MetadataResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/MetadataResource.java @@ -63,6 +63,11 @@ public class MetadataResource { protected MetadataResource() { } + /** For subclasses in other modules; they need it for request contexts and metadata. */ + protected TikaResource getTikaResource() { + return tikaResource; + } + protected void setTikaResource(TikaResource tikaResource) { this.tikaResource = tikaResource; } @@ -72,10 +77,10 @@ public class MetadataResource { @Produces({"application/json", "text/csv"}) @Path("form") public Response getMetadataFromMultipart(Attachment att, @Context UriInfo info) throws Exception { - ParseContext context = tikaResource.createParseContext(); + ParseContext context = tikaResource.createRequestContext(); try (TikaInputStream tis = TikaInputStream.get(att.getObject(InputStream.class))) { return Response - .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), context)) + .ok(parseMetadata(tis, tikaResource.newRequestMetadata(), att.getHeaders(), context)) .build(); } } @@ -93,8 +98,8 @@ public class MetadataResource { @Context HttpHeaders httpHeaders) throws Exception { // Load default context from config, then overlay with request config - ParseContext context = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = tikaResource.setupMultipartConfig(attachments, metadata, context)) { TikaResource.logRequest(LOG, "/meta/config", metadata); // Null headers: multipart request headers describe the envelope and would @@ -106,8 +111,8 @@ public class MetadataResource { @PUT @Produces({"application/json", "text/csv"}) public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext context = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = TikaInputStream.get(is)) { return Response .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), context)) @@ -140,10 +145,10 @@ public class MetadataResource { @Path("{field}") @Produces({"application/json", "text/csv", "text/plain"}) public Response getMetadataField(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info, @PathParam("field") String field) throws Exception { - ParseContext context = tikaResource.createParseContext(); + ParseContext context = tikaResource.createRequestContext(); Metadata metadata; try (TikaInputStream tis = TikaInputStream.get(is)) { - metadata = parseMetadata(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), context); + metadata = parseMetadata(tis, tikaResource.newRequestMetadata(), httpHeaders.getRequestHeaders(), context); } String containerException = metadata.get(TikaCoreProperties.CONTAINER_EXCEPTION); @@ -191,7 +196,7 @@ public class MetadataResource { TikaResource.logRequest(LOG, "/meta", metadata); List<Metadata> metadataList = tikaResource.parseWithPipes(tis, metadata, context, ParseMode.RMETA); if (metadataList.isEmpty()) { - return Metadata.newInstance(context); + return tikaResource.newRequestMetadata(); } return metadataList.get(0); } diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesParsingHelper.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesParsingHelper.java index c4fb5b6b27..2eb7e830f6 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesParsingHelper.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesParsingHelper.java @@ -362,12 +362,10 @@ public class PipesParsingHelper { LOG.debug("Parse returned empty result, status: {}", result.status()); String message = result.message(); if (message != null && !message.isEmpty()) { - // Plain ParseContext, not TikaResource.createParseContext() -- this class is + // Unbounded Metadata: this holds only our own error message, and this class is // constructed before TikaResource (which takes it as a constructor arg), so - // depending back on TikaResource here would be circular. Only used to build - // an error-result Metadata object; no actual parsing happens on this path. - ParseContext context = new ParseContext(); - Metadata errorMetadata = Metadata.newInstance(context); + // reaching back for the configured write limiter would be circular. + Metadata errorMetadata = new Metadata(); errorMetadata.add(TikaCoreProperties.CONTAINER_EXCEPTION, message); return Collections.singletonList(errorMetadata); } diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/RecursiveMetadataResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/RecursiveMetadataResource.java index eb7e7a1455..4bd0b95f21 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/RecursiveMetadataResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/RecursiveMetadataResource.java @@ -17,8 +17,6 @@ package org.apache.tika.server.core.resource; import static org.apache.tika.server.core.resource.TikaResource.fillMetadata; -import static org.apache.tika.server.core.resource.TikaResource.setupContentHandlerFactory; -import static org.apache.tika.server.core.resource.TikaResource.setupContentHandlerFactoryIfNeeded; import java.io.InputStream; import java.util.List; @@ -66,12 +64,12 @@ public class RecursiveMetadataResource { String handlerTypeName) throws Exception { - final ParseContext context = tikaResource.createParseContext(); + final ParseContext context = tikaResource.createRequestContext(); fillMetadata(null, metadata, httpHeaders); TikaResource.logRequest(LOG, "/rmeta", metadata); - setupContentHandlerFactory(context, handlerTypeName); + tikaResource.setupContentHandlerFactory(context, handlerTypeName); // Filtering is done in child process, no need to filter again return tikaResource.parseWithPipes(tis, metadata, context, ParseMode.RMETA); @@ -107,9 +105,8 @@ public class RecursiveMetadataResource { @Produces({"application/json"}) @Path("form{" + HANDLER_TYPE_PARAM + " : (\\w+)?}") public Response getMetadataFromMultipart(Attachment att, @PathParam(HANDLER_TYPE_PARAM) String handlerTypeName) throws Exception { - ParseContext context = tikaResource.createParseContext(); try (TikaInputStream tis = TikaInputStream.get(att.getObject(InputStream.class))) { - List<Metadata> metadataList = parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), + List<Metadata> metadataList = parseMetadata(tis, tikaResource.newRequestMetadata(), att.getHeaders(), handlerTypeName); return Response.ok(new MetadataList(metadataList)).build(); } @@ -128,8 +125,8 @@ public class RecursiveMetadataResource { List<Attachment> attachments, @Context HttpHeaders httpHeaders) throws Exception { - ParseContext context = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = tikaResource.setupMultipartConfig(attachments, metadata, context)) { TikaResource.logRequest(LOG, "/rmeta/config", metadata); @@ -142,7 +139,7 @@ public class RecursiveMetadataResource { private MetadataList parseMetadataWithContext(TikaInputStream tis, Metadata metadata, String handlerTypeName, ParseContext context) throws Exception { - setupContentHandlerFactoryIfNeeded(context, handlerTypeName); + tikaResource.setupContentHandlerFactoryIfNeeded(context, handlerTypeName); // Filtering is done in child process, no need to filter again List<Metadata> metadataList = tikaResource.parseWithPipes(tis, metadata, context, ParseMode.RMETA); @@ -177,8 +174,7 @@ public class RecursiveMetadataResource { @Produces("application/json") @Path("{" + HANDLER_TYPE_PARAM + " : (\\w+)?}") public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @PathParam(HANDLER_TYPE_PARAM) String handlerTypeName) throws Exception { - ParseContext context = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(context); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = TikaInputStream.get(is)) { List<Metadata> metadataList = parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), handlerTypeName); diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/TikaResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/TikaResource.java index 49feb7d247..c86aa7fef9 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/TikaResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/TikaResource.java @@ -52,14 +52,17 @@ import org.slf4j.LoggerFactory; import org.apache.tika.Tika; import org.apache.tika.config.JsonConfig; +import org.apache.tika.config.OutputLimits; import org.apache.tika.config.loader.TikaLoader; import org.apache.tika.exception.TikaConfigException; import org.apache.tika.io.TikaInputStream; import org.apache.tika.metadata.Metadata; import org.apache.tika.metadata.TikaCoreProperties; +import org.apache.tika.metadata.writelimiter.MetadataWriteLimiterFactory; import org.apache.tika.parser.ParseContext; import org.apache.tika.parser.Parser; import org.apache.tika.pipes.api.ParseMode; +import org.apache.tika.pipes.core.extractor.UnpackConfig; import org.apache.tika.sax.BasicContentHandlerFactory; import org.apache.tika.sax.ContentHandlerFactory; import org.apache.tika.serialization.ParseContextUtils; @@ -84,6 +87,13 @@ public class TikaResource { // Enforced in setupMultipartConfig so every config-consuming endpoint honors it. private final boolean allowPerRequestConfig; + // Config-level parse-context defaults, resolved once at startup and kept as values rather + // than as a shared ParseContext. Requests carry only their own deltas (see + // createRequestContext); the forked worker loads these same defaults from the same config. + private final MetadataWriteLimiterFactory configMetadataWriteLimiterFactory; + private final OutputLimits configOutputLimits; + private final boolean configSuppliesContentHandlerFactory; + /** * @param tikaLoader the Tika loader * @param serverStatus server status tracker @@ -96,6 +106,12 @@ public class TikaResource { this.serverStatus = serverStatus; this.pipesParsingHelper = pipesParsingHelper; this.allowPerRequestConfig = allowPerRequestConfig; + + ParseContext configDefaults = loadConfigDefaults(); + this.configMetadataWriteLimiterFactory = configDefaults.get(MetadataWriteLimiterFactory.class); + this.configOutputLimits = OutputLimits.get(configDefaults); + this.configSuppliesContentHandlerFactory = + configDefaults.get(ContentHandlerFactory.class) != null; } /** @@ -108,12 +124,11 @@ public class TikaResource { } /** - * Creates a new ParseContext with defaults loaded from tika-config. - * This loads components from "parse-context" such as DigesterFactory and MetadataWriteLimiterFactory. - * - * @return a new ParseContext with defaults applied + * Reads the config's {@code parse-context} section. Private and called once: the values we + * need are cached above, and a request must not carry these defaults (see + * {@link #createRequestContext()}). */ - public ParseContext createParseContext() { + private ParseContext loadConfigDefaults() { try { return tikaLoader.loadParseContext(); } catch (TikaConfigException e) { @@ -123,6 +138,47 @@ public class TikaResource { } } + /** + * Creates a ParseContext holding only what this request itself specifies. + * <p> + * Config-level {@code parse-context} defaults are deliberately absent. The forked worker + * loads them from the same config and overlays the request on top, so sending them is + * redundant -- and worse than redundant at the trust boundary: the worker clamps + * request-supplied timeout limits but trusts its own config's, so a default that arrives + * as request data gets treated as caller input and clamped. + * + * @return an empty, request-scoped ParseContext + */ + public ParseContext createRequestContext() { + return new ParseContext(); + } + + /** + * Creates request metadata bounded by the config's metadata write limiter. + * <p> + * The limiter no longer rides in the request context, so it is applied here instead. This + * metadata holds caller-supplied values (filename, headers), which is exactly what the + * limiter is meant to bound. + */ + public Metadata newRequestMetadata() { + return configMetadataWriteLimiterFactory == null ? new Metadata() + : new Metadata(configMetadataWriteLimiterFactory.newInstance()); + } + + /** + * A fresh copy of the config's {@code unpack-config}, or null if none is declared. + * <p> + * The unpack path is the one place a config default must still travel: it is mutated + * per request (zip, suffix strategy, emitter) and so overrides whatever the worker would + * have loaded. Starting from a default-constructed instance instead would silently reset + * operator settings the request never touches -- {@code maxUnpackBytes}, for one. Re-read + * per call because the caller mutates the result; unpack requests are heavyweight enough + * that the config read does not register. + */ + public UnpackConfig newConfigUnpackConfig() { + return loadConfigDefaults().get(UnpackConfig.class); + } + public TikaLoader getTikaLoader() { return tikaLoader; @@ -361,7 +417,7 @@ public class TikaResource { * @param context the ParseContext to configure * @param handlerTypeName the handler type name (text, html, xml, ignore), may be null for default */ - public static void setupContentHandlerFactory(ParseContext context, String handlerTypeName) { + public void setupContentHandlerFactory(ParseContext context, String handlerTypeName) { BasicContentHandlerFactory.HANDLER_TYPE type; try { type = BasicContentHandlerFactory.parseHandlerType(handlerTypeName, DEFAULT_HANDLER_TYPE); @@ -369,8 +425,17 @@ public class TikaResource { // The name comes from the URL path, so this is the caller's typo, not our failure. throw new BadRequestException(e.getMessage()); } + // Request-supplied limits (per-request config) win; the cached config defaults are the + // fallback. Neither can be left to OutputLimits.get(context) alone: it returns plain + // defaults when absent, so a deltas-only context would silently drop the operator's + // configured write limit -- and this factory is the one the worker honors. + OutputLimits limits = context.get(OutputLimits.class); + if (limits == null) { + limits = configOutputLimits; + } context.set(ContentHandlerFactory.class, - BasicContentHandlerFactory.newInstance(type, context)); + new BasicContentHandlerFactory(type, limits.getWriteLimit(), + limits.isThrowOnWriteLimit(), context)); } /** @@ -380,8 +445,12 @@ public class TikaResource { * @param context the ParseContext to configure * @param handlerTypeName the handler type name */ - public static void setupContentHandlerFactoryIfNeeded(ParseContext context, String handlerTypeName) { - if (context.get(ContentHandlerFactory.class) == null) { + public void setupContentHandlerFactoryIfNeeded(ParseContext context, String handlerTypeName) { + // A config-declared factory still takes precedence; it is no longer visible in the + // request context, so leaving the context untouched lets the worker resolve it from + // the same config. + if (context.get(ContentHandlerFactory.class) == null + && !configSuppliesContentHandlerFactory) { setupContentHandlerFactory(context, handlerTypeName); } } @@ -402,8 +471,7 @@ public class TikaResource { private Response putRaw(InputStream is, HttpHeaders httpHeaders, String handlerTypeName) throws IOException { try (TikaInputStream tis = TikaInputStream.get(is)) { - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), + return produceRawOutput(tis, newRequestMetadata(), httpHeaders.getRequestHeaders(), handlerTypeName); } } @@ -411,8 +479,7 @@ public class TikaResource { private Metadata putJson(InputStream is, HttpHeaders httpHeaders, String handlerTypeName) throws IOException { try (TikaInputStream tis = TikaInputStream.get(is)) { - ParseContext context = createParseContext(); - return produceJson(tis, Metadata.newInstance(context), + return produceJson(tis, newRequestMetadata(), httpHeaders.getRequestHeaders(), handlerTypeName); } } @@ -504,8 +571,8 @@ public class TikaResource { private Response postConfigured(List<Attachment> attachments, String handlerTypeName) throws IOException, TikaConfigException { - ParseContext context = createParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = createRequestContext(); + Metadata metadata = newRequestMetadata(); try (TikaInputStream tis = setupMultipartConfig(attachments, metadata, context)) { return produceRawOutput(tis, metadata, context, handlerTypeName); } @@ -513,8 +580,8 @@ public class TikaResource { private Metadata postConfiguredJson(List<Attachment> attachments, String handlerTypeName) throws IOException, TikaConfigException { - ParseContext context = createParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = createRequestContext(); + Metadata metadata = newRequestMetadata(); try (TikaInputStream tis = setupMultipartConfig(attachments, metadata, context)) { return produceJson(tis, metadata, context, handlerTypeName); } @@ -606,7 +673,7 @@ public class TikaResource { MultivaluedMap<String, String> httpHeaders, String handlerTypeName) throws IOException { fillMetadata(null, metadata, httpHeaders); - ParseContext context = createParseContext(); + ParseContext context = createRequestContext(); setupContentHandlerFactory(context, handlerTypeName); return produceRawOutputWithContext(tis, metadata, context, handlerTypeName); } @@ -685,7 +752,7 @@ public class TikaResource { MultivaluedMap<String, String> headers, String handlerTypeName) throws IOException { fillMetadata(null, metadata, headers); - ParseContext context = createParseContext(); + ParseContext context = createRequestContext(); setupContentHandlerFactory(context, handlerTypeName); return produceJsonWithContext(tis, metadata, context, handlerTypeName); } @@ -714,7 +781,7 @@ public class TikaResource { parseWithPipes(tis, metadata, context, ParseMode.CONCATENATE); if (metadataList.isEmpty()) { - return Metadata.newInstance(context); + return newRequestMetadata(); } return metadataList.get(0); } diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/UnpackerResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/UnpackerResource.java index 5a5ef25250..f6ccd35bec 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/UnpackerResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/UnpackerResource.java @@ -40,6 +40,7 @@ import org.slf4j.LoggerFactory; import org.apache.tika.io.TikaInputStream; import org.apache.tika.metadata.Metadata; import org.apache.tika.parser.ParseContext; +import org.apache.tika.pipes.core.extractor.UnpackConfig; /** * JAX-RS resource for unpacking embedded documents from container files. @@ -163,8 +164,8 @@ public class UnpackerResource { @PUT @Produces("application/zip") public Response unpack(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext pc = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(pc); + ParseContext pc = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = TikaInputStream.get(is)) { fillMetadata(null, metadata, httpHeaders.getRequestHeaders()); TikaResource.logRequest(LOG, "/unpack", metadata); @@ -186,8 +187,8 @@ public class UnpackerResource { @Consumes("multipart/form-data") @Produces("application/zip") public Response unpackWithConfig(List<Attachment> attachments, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext pc = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(pc); + ParseContext pc = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = tikaResource.setupMultipartConfig(attachments, metadata, pc)) { TikaResource.logRequest(LOG, "/unpack", metadata); return doUnpack(tis, metadata, pc, false); @@ -207,8 +208,8 @@ public class UnpackerResource { @PUT @Produces("application/zip") public Response unpackAll(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext pc = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(pc); + ParseContext pc = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = TikaInputStream.get(is)) { fillMetadata(null, metadata, httpHeaders.getRequestHeaders()); TikaResource.logRequest(LOG, "/unpack/all", metadata); @@ -230,8 +231,8 @@ public class UnpackerResource { @Consumes("multipart/form-data") @Produces("application/zip") public Response unpackAllWithConfig(List<Attachment> attachments, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext pc = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(pc); + ParseContext pc = tikaResource.createRequestContext(); + Metadata metadata = tikaResource.newRequestMetadata(); try (TikaInputStream tis = tikaResource.setupMultipartConfig(attachments, metadata, pc)) { TikaResource.logRequest(LOG, "/unpack/all", metadata); return doUnpack(tis, metadata, pc, true); @@ -254,6 +255,16 @@ public class UnpackerResource { throw new WebApplicationException("Pipes-based parsing is not enabled", Response.Status.SERVICE_UNAVAILABLE); } + // parseUnpack mutates this and so overrides the worker's own config; seed it from the + // config's unpack-config (a per-request instance) rather than from defaults. A config + // supplied by the request itself already sits in pc and wins, as before. + if (pc.get(UnpackConfig.class) == null) { + UnpackConfig fromConfig = tikaResource.newConfigUnpackConfig(); + if (fromConfig != null) { + pc.set(UnpackConfig.class, fromConfig); + } + } + PipesParsingHelper.UnpackResult result = helper.parseUnpack(tis, metadata, pc, saveAll); Path zipFile = result.zipFile(); diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java index 79b100fe75..b7a158363f 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/CXFTestBase.java @@ -315,19 +315,7 @@ public abstract class CXFTestBase { */ private Path createDefaultTestConfig(Path tikaConfigPath) throws IOException { Path pluginsDir = Paths.get("target/plugins").toAbsolutePath(); - - // Read tika config to check for metadata-filters - String metadataFiltersJson = ""; - try { - ObjectMapper mapper = new ObjectMapper(); - JsonNode tikaConfig = mapper.readTree(tikaConfigPath.toFile()); - JsonNode metadataFilters = tikaConfig.get("metadata-filters"); - if (metadataFilters != null && !metadataFilters.isEmpty()) { - metadataFiltersJson = ",\n \"metadata-filters\": " + mapper.writeValueAsString(metadataFilters); - } - } catch (Exception e) { - LOG.debug("Could not read metadata-filters from tika config: {}", e.getMessage()); - } + ObjectMapper mapper = new ObjectMapper(); String configJson = String.format(Locale.ROOT, """ { @@ -346,12 +334,35 @@ public abstract class CXFTestBase { "progressTimeoutMillis": 60000 } }, - "plugin-roots": "%s"%s + "plugin-roots": "%s" + } + """, pluginsDir.toString().replace("\\", "/")); + + com.fasterxml.jackson.databind.node.ObjectNode root = + (com.fasterxml.jackson.databind.node.ObjectNode) mapper.readTree(configJson); + + // Production hands the forked worker a ConfigMerger'd copy of the server's own config, + // so the worker resolves the same parse-context and metadata-filters. Carry them across + // here too: requests ship only their own entries, so anything the harness leaves out of + // the worker's config is simply absent rather than arriving over the wire. + try { + JsonNode tikaConfig = mapper.readTree(tikaConfigPath.toFile()); + JsonNode parseContext = tikaConfig.get("parse-context"); + if (parseContext != null && parseContext.isObject()) { + com.fasterxml.jackson.databind.node.ObjectNode target = + (com.fasterxml.jackson.databind.node.ObjectNode) root.get("parse-context"); + parseContext.properties().forEach(e -> target.set(e.getKey(), e.getValue())); } - """, pluginsDir.toString().replace("\\", "/"), metadataFiltersJson); + JsonNode metadataFilters = tikaConfig.get("metadata-filters"); + if (metadataFilters != null && !metadataFilters.isEmpty()) { + root.set("metadata-filters", metadataFilters); + } + } catch (Exception e) { + LOG.debug("Could not carry config into the worker config: {}", e.getMessage()); + } Path tempConfig = Files.createTempFile("tika-test-default-config-", ".json"); - Files.writeString(tempConfig, configJson); + Files.writeString(tempConfig, mapper.writeValueAsString(root)); return tempConfig; } diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/resource/RequestContextIsolationTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/resource/RequestContextIsolationTest.java new file mode 100644 index 0000000000..2c6bb1351a --- /dev/null +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/resource/RequestContextIsolationTest.java @@ -0,0 +1,214 @@ +/* + * 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.server.core.resource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import org.apache.tika.config.OutputLimits; +import org.apache.tika.config.TimeoutLimits; +import org.apache.tika.config.loader.TikaLoader; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.parser.ParseContext; +import org.apache.tika.pipes.core.extractor.UnpackConfig; +import org.apache.tika.sax.BasicContentHandlerFactory; +import org.apache.tika.sax.ContentHandlerFactory; +import org.apache.tika.server.core.ServerStatus; + +/** + * A request carries only what the request itself specifies; config-level + * {@code parse-context} defaults stay on the server and are re-supplied by the forked worker + * from the same config. + * <p> + * This is a correctness boundary, not just wire economy: the worker clamps request-supplied + * timeout limits and trusts its own config's, so a config default that travels as request data + * is silently downgraded to caller input. That the clamp then leaves such a request alone is + * pinned by ServerProtocolIOTest#testServerConfigLimitsAreTrustedAndNeverClamped; together the + * two cover the path end to end. + */ +public class RequestContextIsolationTest { + + private static final String CONFIG = """ + { + "parse-context": { + "timeout-limits": {"totalTaskTimeoutMillis": 7200000}, + "output-limits": {"writeLimit": 12345, "throwOnWriteLimit": false}, + "standard-metadata-limiter-factory": {"excludeFields": ["dropped-field"]} + } + } + """; + + @TempDir + Path tmp; + + private TikaResource newTikaResource(String configJson) throws Exception { + Path configPath = tmp.resolve("tika-config-" + configJson.hashCode() + ".json"); + Files.writeString(configPath, configJson); + return new TikaResource(TikaLoader.load(configPath), new ServerStatus(), null, true); + } + + private TikaLoader newLoader(String configJson) throws Exception { + Path configPath = tmp.resolve("loader-config-" + configJson.hashCode() + ".json"); + Files.writeString(configPath, configJson); + return TikaLoader.load(configPath); + } + + /** + * The core invariant. Asserts against the loader first so the test cannot pass vacuously + * on a config whose defaults never resolved. + */ + @Test + public void configDefaultsDoNotTravelInTheRequestContext() throws Exception { + ParseContext configDefaults = newLoader(CONFIG).loadParseContext(); + assertNotNull(configDefaults.get(TimeoutLimits.class), + "precondition: config should declare timeout-limits"); + assertNotNull(configDefaults.get(OutputLimits.class), + "precondition: config should declare output-limits"); + + ParseContext request = newTikaResource(CONFIG).createRequestContext(); + + assertNull(request.get(TimeoutLimits.class), + "config timeout limits must not reach the worker as request data -- it clamps " + + "request-supplied limits but trusts its own config's"); + assertNull(request.get(OutputLimits.class)); + assertTrue(request.getContextMap().isEmpty(), + "request context should carry only this request's own entries"); + } + + /** + * OutputLimits.get() falls back to defaults when absent, so sourcing the handler factory's + * limits from the now-empty request context would silently swap the operator's write limit + * for the default -- and this factory is the one the worker honors. + */ + @Test + public void configuredWriteLimitStillReachesTheContentHandlerFactory() throws Exception { + TikaResource tikaResource = newTikaResource(CONFIG); + ParseContext request = tikaResource.createRequestContext(); + + tikaResource.setupContentHandlerFactory(request, "text"); + + BasicContentHandlerFactory chf = + (BasicContentHandlerFactory) request.get(ContentHandlerFactory.class); + assertEquals(12345, chf.getWriteLimit(), "configured writeLimit must survive"); + assertEquals(BasicContentHandlerFactory.HANDLER_TYPE.TEXT, chf.getType()); + } + + /** The write limiter no longer rides in the context, so it must be applied to the metadata. */ + @Test + public void configuredMetadataLimiterStillBoundsRequestMetadata() throws Exception { + Metadata metadata = newTikaResource(CONFIG).newRequestMetadata(); + + metadata.set("dropped-field", "value"); + metadata.set("kept-field", "value"); + + assertNull(metadata.get("dropped-field"), + "excluded field should be dropped by the configured write limiter"); + assertEquals("value", metadata.get("kept-field")); + } + + /** No configured limiter: plain metadata, no NPE on the null-factory path. */ + @Test + public void requestMetadataWorksWithoutAConfiguredLimiter() throws Exception { + Metadata metadata = newTikaResource("{}").newRequestMetadata(); + metadata.set("kept-field", "value"); + assertEquals("value", metadata.get("kept-field")); + } + + /** + * A config-declared handler factory still wins over the endpoint default. It is no longer + * visible in the request context, so precedence is preserved by leaving the context empty + * and letting the worker resolve the same factory from the same config. + */ + @Test + public void configContentHandlerFactoryStillWinsOverEndpointDefault() throws Exception { + String withHandler = """ + { + "parse-context": { + "basic-content-handler-factory": {"type": "HTML"} + } + } + """; + TikaResource tikaResource = newTikaResource(withHandler); + ParseContext request = tikaResource.createRequestContext(); + + tikaResource.setupContentHandlerFactoryIfNeeded(request, "text"); + + assertNull(request.get(ContentHandlerFactory.class), + "endpoint default must not override the config-declared factory"); + } + + /** + * The unpack path mutates UnpackConfig and so overrides the worker's own, which makes it the + * one config default that must still travel. Starting from a default-constructed instance + * would silently reset operator settings the request never touches -- here the + * {@code maxUnpackBytes} cap. + */ + @Test + public void configUnpackConfigIsHandedOutPerRequestAndKeepsOperatorValues() throws Exception { + String withUnpack = """ + { + "parse-context": { + "unpack-config": {"maxUnpackBytes": 4242, "zeroPadName": 7} + } + } + """; + TikaResource tikaResource = newTikaResource(withUnpack); + + UnpackConfig first = tikaResource.newConfigUnpackConfig(); + assertNotNull(first, "config-declared unpack-config should be available to the request"); + assertEquals(4242, first.getMaxUnpackBytes()); + assertEquals(7, first.getZeroPadName()); + + // The unpack path mutates what it is given, so each request needs its own instance. + first.setZipEmbeddedFiles(true); + UnpackConfig second = tikaResource.newConfigUnpackConfig(); + assertNotSame(first, second, "each request must get its own mutable copy"); + assertFalse(second.isZipEmbeddedFiles(), + "one request's mutation must not leak into the next"); + assertEquals(4242, second.getMaxUnpackBytes()); + } + + /** No config-declared unpack-config: null, and the unpack path falls back as before. */ + @Test + public void noConfigUnpackConfigYieldsNull() throws Exception { + assertNull(newTikaResource("{}").newConfigUnpackConfig()); + } + + /** Without a config-declared factory, the endpoint's handler type is installed as before. */ + @Test + public void endpointHandlerTypeIsInstalledWhenConfigDeclaresNone() throws Exception { + TikaResource tikaResource = newTikaResource("{}"); + ParseContext request = tikaResource.createRequestContext(); + + tikaResource.setupContentHandlerFactoryIfNeeded(request, "text"); + + BasicContentHandlerFactory chf = + (BasicContentHandlerFactory) request.get(ContentHandlerFactory.class); + assertNotNull(chf); + assertEquals(BasicContentHandlerFactory.HANDLER_TYPE.TEXT, chf.getType()); + } +} diff --git a/tika-server/tika-server-standard/src/main/java/org/apache/tika/server/standard/resource/XMPMetadataResource.java b/tika-server/tika-server-standard/src/main/java/org/apache/tika/server/standard/resource/XMPMetadataResource.java index 4d7abd492c..9d3b798882 100644 --- a/tika-server/tika-server-standard/src/main/java/org/apache/tika/server/standard/resource/XMPMetadataResource.java +++ b/tika-server/tika-server-standard/src/main/java/org/apache/tika/server/standard/resource/XMPMetadataResource.java @@ -71,10 +71,10 @@ public class XMPMetadataResource extends MetadataResource implements TikaResourc @Produces({"application/rdf+xml"}) @Path("form") public Response getMetadataFromMultipart(Attachment att, @Context UriInfo info) throws Exception { - ParseContext context = new ParseContext(); + ParseContext context = getTikaResource().createRequestContext(); try (TikaInputStream tis = TikaInputStream.get(att.getObject(InputStream.class))) { return Response - .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), context)) + .ok(parseMetadata(tis, getTikaResource().newRequestMetadata(), att.getHeaders(), context)) .build(); } } @@ -82,8 +82,8 @@ public class XMPMetadataResource extends MetadataResource implements TikaResourc @PUT @Produces({"application/rdf+xml"}) public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { - ParseContext context = new ParseContext(); - Metadata metadata = Metadata.newInstance(context); + ParseContext context = getTikaResource().createRequestContext(); + Metadata metadata = getTikaResource().newRequestMetadata(); try (TikaInputStream tis = TikaInputStream.get(is)) { return Response .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), context))
