This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4809-stage-6 in repository https://gitbox.apache.org/repos/asf/tika.git
commit 2c9544739bf23b41d1701571995b37a14b0593cd Author: tallison <[email protected]> AuthorDate: Mon Aug 10 10:06:37 2026 -0400 TIKA-4809: Bound the request path -- spool lifetime, fork heap, temp-file suffix --- docs/modules/ROOT/pages/pipes/cpu-sizing.adoc | 11 +++ .../tika/pipes/core/PerClientServerManager.java | 41 +++++++++- .../server/core/resource/PipesParsingHelper.java | 31 +++++++- .../tika/server/core/resource/TikaResource.java | 93 ++++++++++++---------- .../tika/server/core/MaxRequestSizeFilterTest.java | 16 ++++ 5 files changed, 148 insertions(+), 44 deletions(-) diff --git a/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc b/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc index a076f9ac99..b1721b0cdb 100644 --- a/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc +++ b/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc @@ -166,6 +166,17 @@ report `autoCap=user-set in forkedJvmArgs`. [#heap-per-worker] == Heap per worker — rule of thumb +Heap is auto-sized the same way CPU is. Left to itself, every forked JVM takes its own +default max heap — a fixed fraction of host or container memory — so `numClients` forks have +a combined ceiling well above what the host actually has. When `numClients > 1` and you have +not set `-Xmx` (or `-XX:MaxRAMPercentage`/`-XX:MaxRAMFraction`) yourself, Tika injects +`-XX:MaxRAMPercentage=75/numClients`, leaving the remainder for the parent JVM and the OS. +The `pipes-cpu-sizing` summary line reports the decision as `heap=...`. + +Set `-Xmx` explicitly when you know your workload: the auto-slice is a safe default, not a +tuned one, and a fork that legitimately needs more than its slice will OOM where an untuned +JVM might have grown into spare memory. + A reasonable starting point is **~2 GB of heap per forked worker** (passed via `-Xmx2g` in `forkedJvmArgs`). The number falls out of three independent constraints any of which can dominate: * **Worst-case PDF parsing.** A handful of pathological PDFs in any reasonably large corpus will allocate hundreds of MB of intermediate object data per document — large image streams, deeply nested form fields, big embedded fonts. Smaller heaps OOM on those documents; larger heaps just let GC clean up between docs. diff --git a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PerClientServerManager.java b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PerClientServerManager.java index fdc661d83b..073994271e 100644 --- a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PerClientServerManager.java +++ b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/PerClientServerManager.java @@ -65,6 +65,23 @@ public class PerClientServerManager implements ServerManager { * formula could otherwise produce slice=1. */ private static final int MIN_AUTO_CAP_SLICE = 2; + /** Share of host/container memory the forks may collectively claim; the remainder + * is left for the parent JVM and the OS. */ + private static final int FORK_HEAP_BUDGET_PERCENT = 75; + + /** Never hand a fork a smaller slice than this, however high numClients goes. */ + private static final int MIN_FORK_HEAP_PERCENT = 5; + + private static boolean userSetHeap(List<String> args) { + return args.stream().anyMatch(a -> a.startsWith("-Xmx") + || a.startsWith("-XX:MaxRAMPercentage") + || a.startsWith("-XX:MaxRAMFraction")); + } + + private static int forkHeapPercentage(int numClients) { + return Math.max(MIN_FORK_HEAP_PERCENT, FORK_HEAP_BUDGET_PERCENT / numClients); + } + private final PipesConfig pipesConfig; private final Path tikaConfigPath; private final int clientId; @@ -134,8 +151,17 @@ public class PerClientServerManager implements ServerManager { ? "slice=" + slice : "skipped (slice<" + MIN_AUTO_CAP_SLICE + ")"; } + String heapDecision; + if (userSetHeap(pipesConfig.getForkedJvmArgs())) { + heapDecision = "user-set in forkedJvmArgs"; + } else if (numClients <= 1) { + heapDecision = "n/a (single fork; JVM default)"; + } else { + heapDecision = "MaxRAMPercentage=" + forkHeapPercentage(numClients); + } LOG.info("pipes-cpu-sizing: hostCores={}, numClients={}, parentReserved={}, " + - "autoCap={}", hostCores, numClients, PARENT_RESERVED_CORES, capDecision); + "autoCap={}, heap={}", hostCores, numClients, PARENT_RESERVED_CORES, + capDecision, heapDecision); } @Override @@ -442,6 +468,19 @@ public class PerClientServerManager implements ServerManager { .toAbsolutePath()); } + // Heap gets the same treatment as CPU. Left alone, every fork independently + // takes the JVM's own default max heap (a fixed fraction of host/container + // memory), so numClients forks have a combined ceiling well above what the + // host has -- the same "each JVM thinks it owns the machine" problem the + // ActiveProcessorCount cap solves. Give each fork a slice of a fixed budget + // instead, leaving the remainder for the parent and the OS. + if (!userSetHeap(configArgs) && pipesConfig.getNumClients() > 1) { + int pct = forkHeapPercentage(pipesConfig.getNumClients()); + configArgs.add("-XX:MaxRAMPercentage=" + pct); + LOG.debug("clientId={}: auto-injected -XX:MaxRAMPercentage={} (numClients={})", + clientId, pct, pipesConfig.getNumClients()); + } + // If the user hasn't explicitly set -XX:ActiveProcessorCount, size each // forked JVM's view of CPUs to a fair slice of the host. Otherwise each // JVM defaults its GC, JIT, and common ForkJoinPool to "all cores", which 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 2d87de0605..75fb2e8989 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 @@ -190,21 +190,46 @@ public class PipesParsingHelper { } } + /** Longest suffix carried over from a client filename; keeps well clear of NAME_MAX. */ + private static final int MAX_SUFFIX_LENGTH = 20; + /** - * Extracts file suffix from metadata (resource name or content-type). + * Extracts a file suffix from the resource name for the spool file. + * <p> + * The resource name is client-supplied ({@code Content-Disposition} / {@code File-Name}), + * so the suffix is sanitized here rather than left for {@code Files.createTempFile} to + * reject: a suffix containing a path separator makes it throw {@code IllegalArgumentException} + * — not a traversal, since the JDK refuses it, but an uncaught 500 driven by a request + * header. An over-long suffix likewise fails at the filesystem. The suffix is a parser + * hint, so anything unusable is simply dropped in favour of {@code .tmp}. */ private String getSuffix(Metadata metadata) { String resourceName = metadata.get(TikaCoreProperties.RESOURCE_NAME_KEY); if (resourceName != null) { int lastDot = resourceName.lastIndexOf('.'); if (lastDot > 0 && lastDot < resourceName.length() - 1) { - return resourceName.substring(lastDot); + String suffix = resourceName.substring(lastDot); + if (isUsableSuffix(suffix)) { + return suffix; + } } } - // Default suffix return ".tmp"; } + private static boolean isUsableSuffix(String suffix) { + if (suffix.length() > MAX_SUFFIX_LENGTH) { + return false; + } + for (int i = 0; i < suffix.length(); i++) { + char c = suffix.charAt(i); + if (c == '/' || c == '\\' || c == '�' || Character.isISOControl(c)) { + return false; + } + } + return true; + } + /** * Builds a JSON error response carrying a subset of the {@code PipesResult} * serialization. By default the body is just {@code {"status": "TIMEOUT"}}. The 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 bf2f8582bd..1115b1a5e7 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 @@ -442,10 +442,13 @@ public class TikaResource { @Produces("text/xml") public Response getXhtml(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "xml"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "xml"); + } } /** @@ -457,10 +460,13 @@ public class TikaResource { @Path("text") public Response getText(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "text"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "text"); + } } /** @@ -472,10 +478,13 @@ public class TikaResource { @Path("html") public Response getHtml(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "html"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "html"); + } } /** @@ -487,10 +496,13 @@ public class TikaResource { @Path("xml") public Response getXml(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "xml"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "xml"); + } } /** @@ -502,10 +514,13 @@ public class TikaResource { @Path("md") public Response getMarkdown(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "md"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceRawOutput(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "md"); + } } /** @@ -517,10 +532,13 @@ public class TikaResource { @Path("json") public Metadata getJsonDefault(final InputStream is, @Context HttpHeaders httpHeaders) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceJson(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "text"); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceJson(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), "text"); + } } /** @@ -535,10 +553,13 @@ public class TikaResource { public Metadata getJson(final InputStream is, @Context HttpHeaders httpHeaders, @PathParam(HANDLER_TYPE_PARAM) String handlerTypeName) throws IOException { - TikaInputStream tis = TikaInputStream.get(is); - tis.getPath(); // Spool to temp file for pipes-based parsing - ParseContext context = createParseContext(); - return produceJson(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), handlerTypeName); + // try-with-resources: the spooled temp file must be deleted even if + // context setup or metadata filling throws before the parse begins. + try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing + ParseContext context = createParseContext(); + return produceJson(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), handlerTypeName); + } } // ==================== POST endpoints (multipart with optional config) ==================== @@ -713,12 +734,8 @@ public class TikaResource { // Parse with pipes using CONTENT_ONLY mode - the metadata filter in // EmitHandler will strip everything except tk:content - List<Metadata> metadataList; - try { - metadataList = parseWithPipes(tis, metadata, context, ParseMode.CONTENT_ONLY); - } finally { - tis.close(); - } + List<Metadata> metadataList = + parseWithPipes(tis, metadata, context, ParseMode.CONTENT_ONLY); LOG.debug("produceRawOutput: parseWithPipes returned {} metadata objects", metadataList.size()); @@ -798,12 +815,8 @@ public class TikaResource { // Ensure content handler factory is set (config may have set it) setupContentHandlerFactoryIfNeeded(context, handlerTypeName); - List<Metadata> metadataList; - try { - metadataList = parseWithPipes(tis, metadata, context, ParseMode.CONCATENATE); - } finally { - tis.close(); - } + List<Metadata> metadataList = + parseWithPipes(tis, metadata, context, ParseMode.CONCATENATE); if (metadataList.isEmpty()) { return Metadata.newInstance(context); diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/MaxRequestSizeFilterTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/MaxRequestSizeFilterTest.java index 263486fc3f..a6be0374ef 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/MaxRequestSizeFilterTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/MaxRequestSizeFilterTest.java @@ -72,6 +72,22 @@ public class MaxRequestSizeFilterTest extends CXFTestBase { "a body well under the limit must not be rejected"); } + /** + * A filename whose extension contains a path separator previously reached + * Files.createTempFile and threw IllegalArgumentException, surfacing as a 500 + * driven entirely by a request header. + */ + @Test + public void testHostileFilenameDoesNotError() throws Exception { + Response response = WebClient + .create(endPoint + TIKA_PATH + "/text") + .header("Content-Disposition", "attachment; filename=\"a.b/../../c\"") + .put(new ByteArrayInputStream(body(50))); + + assertNotEquals(500, response.getStatus(), + "a hostile filename suffix must not produce a server error"); + } + /** * Chunked uploads carry no usable Content-Length, so the declared-length check cannot * fire and the counting stream is the only thing enforcing the limit.
