diff --git a/docs/modules/ROOT/pages/migration-to-4x/migrating-tika-server-4x.adoc b/docs/modules/ROOT/pages/migration-to-4x/migrating-tika-server-4x.adoc index e1bc589937..fad6b193e4 100644 --- a/docs/modules/ROOT/pages/migration-to-4x/migrating-tika-server-4x.adoc +++ b/docs/modules/ROOT/pages/migration-to-4x/migrating-tika-server-4x.adoc @@ -21,7 +21,7 @@ == Overview -Tika Server 4.x introduces pipes-based parsing for the main content-extraction endpoints (`/tika`, `/rmeta`, `/unpack`), which provides process isolation for those operations. This improves stability and resource management but introduces some breaking changes. A few endpoints (notably `/meta`) still parse in-process in the request-handling JVM. +Tika Server 4.x introduces pipes-based parsing for the main content-extraction endpoints (`/tika`, `/rmeta`, `/unpack`, `/meta`), which provides process isolation for those operations. This improves stability and resource management but introduces some breaking changes. == New `/tika` Endpoint Structure @@ -112,6 +112,29 @@ The HTTP status codes are also more precise: that branch only on HTTP status code are unaffected unless they were treating `UNSPECIFIED_CRASH` as a `500`. +=== `/meta` Is Now Pipes-Backed + +`/meta` previously parsed in-process, in the request-handling JVM, with no crash +isolation and its own ad hoc error handling (`500` for most parse failures, `400` +for a field that couldn't be extracted from an incompletely-parsed document). It +now shares the same pipes-backed `PipesParser` as `/tika`, `/rmeta`, and `/unpack` +(see +xref:using-tika/server/index.adoc#_endpoints_and_forked_process_groups[Endpoints +and Forked-Process Groups]), with the same crash isolation and the same +per-document exception handling as those endpoints (see +xref:using-tika/server/index.adoc#_error_responses[Error Responses]): + +* `/meta`, `/meta/form`, `/meta/config` now return `200 OK` with the exception + embedded in `tk:exception:container-exception`, instead of `500`. +* `/meta/\{field}` now returns `422 Unprocessable Entity` for a genuine parse + exception, instead of `500` or `400`. + +**Migration:** clients that treated any non-`200` from `/meta` as "parse failed" +should check the new status codes above. Clients that inspected the response body +for error text should check `tk:exception:container-exception` (full-object +endpoints) or the `422` body (`/meta/\{field}`, populated only when +`returnStackTrace=true`). + === Accept Header Routing Removed The `/tika` endpoint no longer routes based on `Accept` headers. Use explicit paths instead: diff --git a/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc b/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc index 7c1d066d8d..831ed445d0 100644 --- a/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc +++ b/docs/modules/ROOT/pages/pipes/cpu-sizing.adoc @@ -119,17 +119,20 @@ Everything above describes sizing for *one* `PipesParser` — one `pipes` config section, one set of forked workers. The auto-sizer has no visibility into anything else running in the same JVM. -This matters concretely for tika-server: `/tika`+`/rmeta`+`/unpack` and -`/pipes`+`/async` are backed by two *independent* `PipesParser` groups when -both are enabled in the same server. Each group's auto-sizer computes its -slice from `Runtime.availableProcessors()` as if it were the only consumer on -the host — it does not know a sibling group in the same process is about to -fork its own `numClients` workers too. The result: with `numClients=2` on -both, you get 4 total forked JVMs, each capped assuming exclusive access to -the whole host. Whether that's *actually* oversubscribed depends on your -host's real core count relative to those combined `numClients` values — it's -not automatic, but the auto-sizer also won't warn you, because each group -looks correctly sized from its own perspective alone. See +This matters concretely for tika-server: `/tika`+`/rmeta`+`/unpack`+`/meta`+ +`/pipes` and `/async` are backed by *two independent* groups when both are +enabled in the same server — the first five endpoints share one `PipesParser` +instance, and `/async` manages its own forked-worker pool directly (not via +`PipesParser` at all, though it uses the same underlying auto-sizer). Each +group's auto-sizer computes its slice from `Runtime.availableProcessors()` as +if it were the only consumer on the host — it does not know the sibling group +in the same process is about to fork its own `numClients` workers too. The +result: with `numClients=2` on both, you get 4 total forked JVMs, each capped +assuming exclusive access to the whole host. Whether that's *actually* +oversubscribed depends on your host's real core count relative to those +combined `numClients` values — it's not automatic, but the auto-sizer also +won't warn you, because each group looks correctly sized from its own +perspective alone. See xref:using-tika/server/index.adoc#_endpoints_and_forked_process_groups[Endpoints and Forked-Process Groups] for the tika-server-specific guidance. @@ -138,10 +141,10 @@ directly and constructing more than one instance in a single JVM — the auto-sizer will size each independently, with the same caveat. There is no automatic fix for this today: unlike the single-group case, where -Tika detects and warns about bad provisioning, a *second* group has no way to -learn what a sibling group already claimed. Mitigate it explicitly — either -run only one group per process, or set `-XX:ActiveProcessorCount` yourself -(next section) with the combined total in mind. +Tika detects and warns about bad provisioning, each group has no way to learn +what its siblings already claimed. Mitigate it explicitly — either run fewer +groups per process, or set `-XX:ActiveProcessorCount` yourself (next section) +with the combined total in mind. == Disabling or overriding diff --git a/docs/modules/ROOT/pages/using-tika/server/index.adoc b/docs/modules/ROOT/pages/using-tika/server/index.adoc index b04868f137..113cdb6071 100644 --- a/docs/modules/ROOT/pages/using-tika/server/index.adoc +++ b/docs/modules/ROOT/pages/using-tika/server/index.adoc @@ -24,25 +24,23 @@ This section covers running Apache Tika as a REST server via `tika-server`. Tika Server provides a RESTful HTTP interface for parsing documents and extracting content. It can be deployed as a standalone service or in a containerized environment. -In Tika 4.x, the main content-extraction endpoints — `/tika`, `/rmeta`, and -`/unpack` — parse in forked child processes via the Tika Pipes infrastructure. -This provides process isolation (a parser crash or OOM in a child cannot take -down the request-handling process) at the cost of requiring a Pipes -configuration. A few endpoints (notably `/meta`) still parse in-process in the -request-handling JVM; treat those as best-effort under load. See +In Tika 4.x, the main content-extraction endpoints — `/tika`, `/rmeta`, +`/unpack`, and `/meta` — parse in forked child processes via the Tika Pipes +infrastructure. This provides process isolation (a parser crash or OOM in a +child cannot take down the request-handling process) at the cost of requiring +a Pipes configuration. See xref:migration-to-4x/migrating-tika-server-4x.adoc[Migrating Tika Server to 4.x] for the full breaking-change list when upgrading from 3.x. [IMPORTANT] ==== This is not opt-in the way `/pipes` and `/async` are (those require -`allowPipes=true` and refuse to start without it). `/tika`, `/rmeta`, and -`/unpack` are **on by default** — the moment you run a basic `tika-server` and -PUT a document to `/tika`, you are running Tika Pipes, with a real forked -child process behind it. (`/meta` is the exception among the main -content-extraction endpoints — it still parses in-process; see below.) If -you're upgrading from 3.x, where these endpoints parsed in-process in a -single JVM, this is a profound change: `pipes.numClients` now controls both +`allowPipes=true` and refuse to start without it). `/tika`, `/rmeta`, +`/unpack`, and `/meta` are **on by default** — the moment you run a basic +`tika-server` and PUT a document to `/tika`, you are running Tika Pipes, with +a real forked child process behind it. If you're upgrading from 3.x, where +these endpoints parsed in-process in a single JVM, this is a profound change: +`pipes.numClients` now controls both how many requests these endpoints can serve concurrently and how many forked JVMs run at once, and it's easy to size it thinking about only one of those two things. Undersized for your request volume, and callers start waiting — @@ -200,6 +198,12 @@ is a plain opt-in endpoint — enable it simply by listing it under `endpoints`. == Error Responses +tika-server distinguishes two different kinds of failure: the forked worker itself +dying, and the worker running fine but catching an exception while parsing one +particular document. They get different treatment. + +=== Process-level failures + When parsing fails due to a process-level problem — the forked child process timed out, ran out of memory, or crashed unexpectedly — the server returns an HTTP error with a JSON body whose shape matches the `PipesResult` status: @@ -239,10 +243,54 @@ crashing" — you can tell them apart from the status code alone. document on the same server is unlikely to succeed without a configuration fix. |=== -NOTE: A successful parse that encountered internal parser errors (e.g. a truncated -embedded document) still returns `200 OK`. The partial-parse exception is surfaced -in the `tk:exception:container-exception` metadata field of the response, not as an -HTTP error code. +=== Per-document parse exceptions + +A process-level failure (above) means the worker itself is gone — nothing was parsed. +A per-document parse exception is different: the worker ran to completion and simply +caught an exception while parsing this one document (an encrypted file with no +password, a malformed embedded object, an NPE in a specific parser). The worker is +healthy, and whatever content it managed to extract is still available. + +Which HTTP status this gets depends on whether the response shape has room to embed +the exception alongside content: + +[cols="1,1,3"] +|=== +|Endpoints |Status |Behavior + +|`/rmeta`, `/tika/json`, `/meta`'s full-object endpoints +|`200 OK` +|The exception is embedded in the response's `tk:exception:container-exception` +field (or `tk:exception:embedded-exception` on an individual embedded document +within an `/rmeta` list), alongside whatever content and metadata were captured. +Partial success is meaningful here — a batch/list response, or a structured object +with room for an extra field. + +|`/tika`'s raw endpoints (`text`, `html`, `xml`, `md`) +|`422 Unprocessable Entity` +|A raw byte-stream response has no field to embed the exception in, so the status +itself signals the failure — but the body still carries whatever content was +actually extracted, not an empty or generic error body. + +|`/meta/\{field}` +|`422 Unprocessable Entity` +|A single scalar value has nowhere to embed the exception either, so it's thrown +rather than silently returned as if the field were simply absent. + +|`/unpack` +|`422 Unprocessable Entity` +|Same reasoning as the raw endpoints, but content is *not* currently preserved — +any files already unpacked before the exception are discarded. This is a known +gap, not yet addressed. +|=== + +By default (`returnStackTrace=false`), any exception text exposed this way is trimmed +to just the exception's class and message — not the full stack trace, which can +reveal internal file paths and library internals. For the `200 OK` family the +trimmed field is still always present when a failure occurred, so callers can detect +it either way; for the `422` family, the body carries no exception text at all unless +`returnStackTrace=true`. Set `returnStackTrace=true` to get the full trace — useful +in development, best left off in production. == Configuration @@ -260,7 +308,7 @@ Server behavior beyond host/port is controlled by a JSON config file passed via |`endpoints` |_all defaults_ -|Which endpoints to expose. Leave unset to get the full default set (includes `/tika` and `/rmeta`). Explicitly listing endpoints also controls how many independent forked-process groups you run — see <<_endpoints_and_forked_process_groups,Endpoints and Forked-Process Groups>> below before combining `/tika`/`/rmeta` with `/pipes`/`/async`. +|Which endpoints to expose. Leave unset to get the full default set (includes `/tika` and `/rmeta`). Explicitly listing endpoints also controls how many independent forked-process groups you run — see <<_endpoints_and_forked_process_groups,Endpoints and Forked-Process Groups>> below before combining `/tika`/`/rmeta`/`/unpack`/`/meta`/`/pipes` with `/async`. |`allowPerRequestConfig` |`false` @@ -298,21 +346,19 @@ xref:migration-to-4x/migrating-tika-server-4x.adoc#_configuration_changes[Config [#_endpoints_and_forked_process_groups] == Endpoints and Forked-Process Groups -Two independent pipes-backed process groups exist, plus one endpoint that -isn't pipes-backed at all: - -* **`/tika` + `/rmeta` + `/unpack`** share one group — all three go through -the same `PipesParsingHelper`/`PipesParser`, sized by `pipes.numClients`. -* **`/pipes` + `/async`** share a separate group (gated behind `allowPipes`), -sized by the same `pipes.numClients` setting in the same config, but as an -independent set of forked processes. -* **`/meta` is not pipes-backed** — it still parses in-process, in the -request-handling JVM, as in 3.x. It isn't bound by `numClients` and doesn't -participate in anything below, but it also has no crash/OOM isolation: a -hostile or pathological document sent to `/meta` can affect the -request-handling process itself, unlike the pipes-backed endpoints where the -same document only takes down a forked child. Treat `/meta` as best-effort -under adversarial input. +Two independent forked-process groups exist: + +* **`/tika` + `/rmeta` + `/unpack` + `/meta` + `/pipes`** share one group — +all five go through the same `PipesParsingHelper`/`PipesParser`, sized by +`pipes.numClients`. `/pipes` still requires `allowPipes` to actually start +(the server refuses to start if it's listed without that flag) even though it +shares its parser with the always-on endpoints; the others don't require +`allowPipes`. +* **`/async`** is a separate group (gated behind `allowPipes`) — it doesn't +share a `PipesParser` with the group above at all. It manages its own +forked-worker pool directly (queued/background processing, results delivered +via a configured `PipesReporter` rather than in the HTTP response), sized by +its own read of `pipes.numClients` from the same config. Within a pipes-backed group, `numClients` does two *separate* jobs, and it's worth understanding both before picking a value. @@ -340,14 +386,15 @@ Independently of the above, each group also auto-sizes its forked JVMs' `-XX:ActiveProcessorCount` from `numClients` and the host's core count — see xref:pipes/cpu-sizing.adoc[Forked-JVM CPU Sizing] for the full mechanics. This part *can* go wrong across groups: the auto-sizer for one group has no -visibility into another group running in the same process, so if you enable -both `/tika`/`/rmeta`/`/unpack` *and* `/pipes`/`/async` together — a config -listing all of them, or simply leaving `endpoints` unset while -`allowPipes=true` — each group's auto-sizer computes its slice as if it owned -the whole host. Whether that actually causes oversubscription depends on your -`numClients` values relative to the host's core count; it's not automatic, but -it's also not something the auto-sizer will warn you about, because from -either group's perspective alone the sizing looks fine. See +visibility into the other group running in the same process, so if you enable +`/async` alongside the shared group — a config listing `async` together with +any of `tika`/`rmeta`/`unpack`/`meta`/`pipes`, or simply leaving `endpoints` +unset while `allowPipes=true` gives you both groups at once — each group's +auto-sizer computes its slice as if it owned the whole host. Whether that +actually causes oversubscription depends on your `numClients` values relative +to the host's core count; it's not automatic, but it's also not something the +auto-sizer will warn you about, because from either group's perspective alone +the sizing looks fine. See xref:pipes/cpu-sizing.adoc#_known_limitation_multiple_pipes_groups_in_one_process[Known limitation: multiple Pipes groups in one process] for the mechanics and mitigation (scope `endpoints` to what you actually use, or set diff --git a/tika-core/src/main/java/org/apache/tika/sax/BasicContentHandlerFactory.java b/tika-core/src/main/java/org/apache/tika/sax/BasicContentHandlerFactory.java index 935ea45aeb..cc25fecbdf 100644 --- a/tika-core/src/main/java/org/apache/tika/sax/BasicContentHandlerFactory.java +++ b/tika-core/src/main/java/org/apache/tika/sax/BasicContentHandlerFactory.java @@ -165,7 +165,7 @@ private ContentHandler createHandlerInner() { new WriteOutContentHandler(new ToTextContentHandler(), writeLimit, throwOnWriteLimitReached, parseContext)); } else if (type == HANDLER_TYPE.IGNORE) { - return new DefaultHandler(); + return new NoOpContentHandler(); } ContentHandler formatHandler = getFormatHandler(); if (writeLimit < 0) { @@ -201,7 +201,7 @@ public ContentHandler createHandler(OutputStream os, Charset charset) { private ContentHandler createHandlerInner(OutputStream os, Charset charset) { if (type == HANDLER_TYPE.IGNORE) { - return new DefaultHandler(); + return new NoOpContentHandler(); } try { if (writeLimit > -1) { @@ -332,4 +332,20 @@ public int hashCode() { result = 31 * result + (validateXHTML ? 1 : 0); return result; } + + /** + * DefaultHandler, but with toString() returning "" instead of the default + * Object identity string. Callers that want to know whether a parse + * actually produced content can blank-check toString() directly -- no + * need to special-case DefaultHandler's class identity, which breaks the + * moment this handler is wrapped by a decorator (e.g. StrictXHTMLValidator + * when validateXHTML is set): ContentHandlerDecorator.toString() delegates + * to the wrapped handler, so the empty string still propagates through. + */ + private static final class NoOpContentHandler extends DefaultHandler { + @Override + public String toString() { + return ""; + } + } } diff --git a/tika-core/src/main/java/org/apache/tika/sax/RecursiveParserWrapperHandler.java b/tika-core/src/main/java/org/apache/tika/sax/RecursiveParserWrapperHandler.java index 9294dcaf42..c4ac0df068 100644 --- a/tika-core/src/main/java/org/apache/tika/sax/RecursiveParserWrapperHandler.java +++ b/tika-core/src/main/java/org/apache/tika/sax/RecursiveParserWrapperHandler.java @@ -24,7 +24,6 @@ import org.xml.sax.ContentHandler; import org.xml.sax.SAXException; -import org.xml.sax.helpers.DefaultHandler; import org.apache.tika.metadata.Metadata; import org.apache.tika.metadata.TikaCoreProperties; @@ -147,20 +146,18 @@ public List getMetadataList() { } void addContent(ContentHandler handler, Metadata metadata) { - - if (handler.getClass().equals(DefaultHandler.class)) { - //no-op: we can't rely on just testing for - //empty content because DefaultHandler's toString() - //returns e.g. "org.xml.sax.helpers.DefaultHandler@6c8b1edd" - } else { - String content = handler.toString(); - if (content != null && !content.isBlank()) { - metadata.add(TikaCoreProperties.TIKA_CONTENT, content); - metadata.add(TikaCoreProperties.TIKA_CONTENT_HANDLER, - handler.getClass().getSimpleName()); - metadata.set(TikaCoreProperties.TIKA_CONTENT_HANDLER_TYPE, - getContentHandlerFactory().handlerTypeName()); - } + // BasicContentHandlerFactory's "ignore" handler's toString() returns "" (not + // Object's default identity string), so a plain blank check is enough here -- + // no need to special-case its class, which would break under decoration (e.g. + // StrictXHTMLValidator when validateXHTML is set): ContentHandlerDecorator + // delegates toString() to the wrapped handler, so "" still propagates through. + String content = handler.toString(); + if (content != null && !content.isBlank()) { + metadata.add(TikaCoreProperties.TIKA_CONTENT, content); + metadata.add(TikaCoreProperties.TIKA_CONTENT_HANDLER, + handler.getClass().getSimpleName()); + metadata.set(TikaCoreProperties.TIKA_CONTENT_HANDLER_TYPE, + getContentHandlerFactory().handlerTypeName()); } } } diff --git a/tika-core/src/test/java/org/apache/tika/sax/BasicContentHandlerFactoryTest.java b/tika-core/src/test/java/org/apache/tika/sax/BasicContentHandlerFactoryTest.java index bc6260d0a4..364ab0e414 100644 --- a/tika-core/src/test/java/org/apache/tika/sax/BasicContentHandlerFactoryTest.java +++ b/tika-core/src/test/java/org/apache/tika/sax/BasicContentHandlerFactoryTest.java @@ -76,8 +76,9 @@ public void testIgnore() throws Exception { .createHandler(); assertTrue(handler instanceof DefaultHandler); p.parse(null, handler, null, null); - //unfortunatley, the DefaultHandler does not return "", - assertContains("org.xml.sax.helpers.DefaultHandler", handler.toString()); + // toString() returns "" (not Object's default identity string) so callers can + // blank-check it directly instead of special-casing DefaultHandler's class identity. + assertEquals("", handler.toString()); //tests that no write limit exception is thrown p = new MockParser(100); @@ -85,7 +86,7 @@ public void testIgnore() throws Exception { .createHandler(); assertTrue(handler instanceof DefaultHandler); p.parse(null, handler, null, null); - assertContains("org.xml.sax.helpers.DefaultHandler", handler.toString()); + assertEquals("", handler.toString()); } @Test diff --git a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/EmitStrategyConfig.java b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/EmitStrategyConfig.java index 7c140fd18a..843949c943 100644 --- a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/EmitStrategyConfig.java +++ b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/EmitStrategyConfig.java @@ -18,6 +18,7 @@ import java.io.Serializable; +import org.apache.tika.annotation.TikaComponent; import org.apache.tika.exception.TikaConfigException; import org.apache.tika.exception.TikaException; @@ -45,7 +46,13 @@ * ParseContext context = new ParseContext(); * context.set(EmitStrategyConfig.class, new EmitStrategyConfig(EmitStrategy.PASSBACK_ALL)); * + *

+ * {@code @TikaComponent}-registered so that a per-request instance set on ParseContext + * survives serialization across the parent-child pipes IPC boundary (JsonPipesIpc / + * ParseContextSerializer require every context-map entry's class to have a registered + * friendly name) -- without this, setting it per-request throws during serialization. */ +@TikaComponent(name = "emit-strategy-config") public class EmitStrategyConfig implements Serializable { private static final long serialVersionUID = 1L; diff --git a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ParseHandler.java b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ParseHandler.java index c52008d0d0..69a3893346 100644 --- a/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ParseHandler.java +++ b/tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/ParseHandler.java @@ -259,7 +259,14 @@ public List parseConcatenated(FetchEmitTuple fetchEmitTuple, containerException = ExceptionUtils.getStackTrace(e); LOG.warn("parse exception: " + fetchEmitTuple.getId(), e); } finally { - metadata.add(TikaCoreProperties.TIKA_CONTENT, handler.toString()); + // BasicContentHandlerFactory's "ignore" handler's toString() returns "" (not + // Object's default identity string), so a plain blank check is enough here -- + // no need to special-case its class, which would break under decoration (e.g. + // StrictXHTMLValidator when validateXHTML is set). + String content = handler.toString(); + if (content != null && !content.isBlank()) { + metadata.add(TikaCoreProperties.TIKA_CONTENT, content); + } metadata.set(TikaCoreProperties.TIKA_CONTENT_HANDLER_TYPE, contentHandlerFactory.handlerTypeName()); if (containerException != null) { diff --git a/tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/PipesClientTest.java b/tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/PipesClientTest.java index f29a743739..09bfbfaccc 100644 --- a/tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/PipesClientTest.java +++ b/tika-pipes/tika-pipes-integration-tests/src/test/java/org/apache/tika/pipes/core/PipesClientTest.java @@ -40,6 +40,8 @@ import org.apache.tika.pipes.api.PipesResult; import org.apache.tika.pipes.api.emitter.EmitKey; import org.apache.tika.pipes.api.fetcher.FetchKey; +import org.apache.tika.sax.BasicContentHandlerFactory; +import org.apache.tika.sax.ContentHandlerFactory; public class PipesClientTest { @@ -854,4 +856,31 @@ public void testConcatenateMode(@TempDir Path tmp) throws Exception { assertNotNull(metadata.get(TikaCoreProperties.RESOURCE_NAME_KEY), "RESOURCE_NAME should be preserved in CONCATENATE mode"); } + + @Test + public void testConcatenateModeIgnoreHandlerDoesNotLeakContent(@TempDir Path tmp) throws Exception { + // CONCATENATE + handler type "ignore" must not add TIKA_CONTENT at all -- previously + // ParseHandler.parseConcatenated's finally block unconditionally called + // handler.toString(), which for the DefaultHandler behind "ignore" produces garbage + // like "org.xml.sax.helpers.DefaultHandler@6c8b1edd" instead of skipping, unlike + // RecursiveParserWrapperHandler.addContent (used by RMETA mode), which already guards + // against this. + String testFile = "mock-embedded.xml"; + Metadata metadata; + try (PipesClient pipesClient = init(tmp, testFile)) { + ParseContext parseContext = new ParseContext(); + parseContext.set(ParseMode.class, ParseMode.CONCATENATE); + parseContext.set(ContentHandlerFactory.class, + new BasicContentHandlerFactory(BasicContentHandlerFactory.HANDLER_TYPE.IGNORE, -1)); + PipesResult pipesResult = pipesClient.process( + new FetchEmitTuple(testFile, new FetchKey(fetcherName, testFile), + new EmitKey(), new Metadata(), parseContext, + FetchEmitTuple.ON_PARSE_EXCEPTION.SKIP)); + assertEquals(1, pipesResult.emitData().getMetadataList().size()); + metadata = pipesResult.emitData().getMetadataList().get(0); + } + + assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT), + "TIKA_CONTENT must not be set when the handler type is \"ignore\""); + } } diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java index ea569bceb1..93bde62770 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java @@ -180,11 +180,12 @@ private static ServerDetails initServer(TikaServerConfig tikaServerConfig) throw ServerStatus serverStatus = new ServerStatus(); - // Initialize pipes-based parsing only if /tika or /rmeta endpoints are enabled + // Initialize pipes-based parsing (and its shared PipesParser) only if any + // pipes-backed endpoint is enabled. PipesParsingHelper pipesParsingHelper = null; if (needsPipesParsingHelper(tikaServerConfig)) { pipesParsingHelper = initPipesParsingHelper(tikaServerConfig); - LOG.info("Pipes-based parsing enabled for /tika and /rmeta endpoints"); + LOG.info("Pipes-based parsing enabled for /tika, /rmeta, /unpack, /meta, and /pipes endpoints"); } TikaResource tikaResource = new TikaResource(tikaLoader, serverStatus, pipesParsingHelper, @@ -424,17 +425,13 @@ static List loadCoreProviders(TikaServerConfig tikaServerConfi resourceProviders.add(new SingletonResourceProvider(localAsyncResource)); } if (addPipesResource) { - final PipesResource localPipesResource = new PipesResource(tikaServerConfig.getConfigPath()); - Runtime - .getRuntime() - .addShutdownHook(new Thread(() -> { - try { - localPipesResource.close(); - } catch (Exception e) { - LOG.warn("exception closing local pipes resource", e); - } - })); - resourceProviders.add(new SingletonResourceProvider(localPipesResource)); + // /pipes shares its PipesParser with /tika+/rmeta+/unpack (see + // needsPipesParsingHelper) -- non-null here is guaranteed by that check. + // Lifecycle (shutdown/close) is owned by whoever built the shared parser, + // not by PipesResource. + PipesParsingHelper helper = tikaResource.getPipesParsingHelper(); + resourceProviders.add(new SingletonResourceProvider( + new PipesResource(helper.getPipesParser(), helper.isReturnStackTrace()))); } resourceProviders.addAll(loadResourceServices(serverStatus)); return resourceProviders; @@ -458,17 +455,23 @@ private static Collection loadWriterServices() { } /** - * Determines if PipesParsingHelper is needed based on configured endpoints. - * It's needed when /tika or /rmeta endpoints are enabled (either explicitly or by default). + * Determines if the shared PipesParser (wrapped in PipesParsingHelper) is needed + * based on configured endpoints. It's needed when /tika, /rmeta, /unpack, /meta, or + * /pipes are enabled (either explicitly or by default) -- all five now share one + * parser. (Note: unlike the others, /pipes also requires allowPipes to actually + * start; if it's listed without allowPipes, loadCoreProviders will refuse to start + * regardless of whether this method already triggered building the shared parser.) */ - private static boolean needsPipesParsingHelper(TikaServerConfig tikaServerConfig) { + static boolean needsPipesParsingHelper(TikaServerConfig tikaServerConfig) { List endpoints = tikaServerConfig.getEndpoints(); - // If no endpoints specified, all default endpoints are loaded (including tika and rmeta) + // If no endpoints specified, all default endpoints are loaded (including + // tika, rmeta, and unpack; pipes too when allowPipes is set) if (endpoints == null || endpoints.isEmpty()) { return true; } - // Check if tika or rmeta are in the configured endpoints - return endpoints.contains("tika") || endpoints.contains("rmeta"); + return endpoints.contains("tika") || endpoints.contains("rmeta") + || endpoints.contains("unpack") || endpoints.contains("pipes") + || endpoints.contains("meta"); } /** 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 ec0d9bc46f..1fbc0bffdb 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 @@ -18,7 +18,6 @@ import static org.apache.tika.server.core.resource.TikaResource.fillMetadata; -import java.io.IOException; import java.io.InputStream; import java.util.List; @@ -37,13 +36,16 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.tika.exception.TikaConfigException; -import org.apache.tika.extractor.DocumentSelector; +import org.apache.tika.config.EmbeddedLimits; +import org.apache.tika.exception.TikaException; import org.apache.tika.io.TikaInputStream; -import org.apache.tika.language.detect.LanguageHandler; import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.TikaCoreProperties; import org.apache.tika.parser.ParseContext; -import org.apache.tika.parser.Parser; +import org.apache.tika.pipes.api.ParseMode; +import org.apache.tika.sax.BasicContentHandlerFactory; +import org.apache.tika.sax.ContentHandlerFactory; +import org.apache.tika.server.core.TikaServerParseException; @Path("/meta") @@ -63,8 +65,9 @@ public MetadataResource(TikaResource tikaResource) { public Response getMetadataFromMultipart(Attachment att, @Context UriInfo info) throws Exception { ParseContext context = tikaResource.createParseContext(); try (TikaInputStream tis = TikaInputStream.get(att.getObject(InputStream.class))) { + tis.getPath(); // Spool to temp file for pipes-based parsing return Response - .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), info)) + .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), context)) .build(); } } @@ -79,25 +82,14 @@ public Response getMetadataFromMultipart(Attachment att, @Context UriInfo info) @Path("config") public Response getMetadataWithConfig( List attachments, - @Context HttpHeaders httpHeaders, - @Context UriInfo info) throws Exception { + @Context HttpHeaders httpHeaders) throws Exception { // Load default context from config, then overlay with request config ParseContext context = tikaResource.createParseContext(); Metadata metadata = Metadata.newInstance(context); try (TikaInputStream tis = tikaResource.setupMultipartConfig(attachments, metadata, context)) { - // No need to parse embedded docs for metadata-only extraction - context.set(DocumentSelector.class, metadata1 -> false); - - Parser parser = tikaResource.createParser(); TikaResource.logRequest(LOG, "/meta/config", metadata); - tikaResource.parse(parser, LOG, info.getPath(), tis, new LanguageHandler() { - public void endDocument() { - metadata.set("language", getLanguage().getLanguage()); - } - }, metadata, context); - - return Response.ok(metadata).build(); + return Response.ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), context)).build(); } } @@ -107,19 +99,19 @@ public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @C ParseContext context = tikaResource.createParseContext(); Metadata metadata = Metadata.newInstance(context); try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing return Response - .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), info)) + .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), context)) .build(); } } /** - * Get a specific metadata field. If the input stream cannot be parsed, but a - * value was found for the given metadata field, then the value of the field - * is returned as part of a 200 OK response; otherwise a - * {@link javax.ws.rs.core.Response.Status#BAD_REQUEST} is generated. If the stream - * was successfully parsed but the specific metadata field was not found, then a - * {@link javax.ws.rs.core.Response.Status#NOT_FOUND} is returned. + * Get a specific metadata field. If the document parses successfully but the + * specific metadata field was not found, a + * {@link javax.ws.rs.core.Response.Status#NOT_FOUND} is returned. Unlike the other + * /meta endpoints, a bare field value has no envelope to embed a container-level + * exception in, so that case is thrown (422) instead. *

* Note that this method handles multivalue fields and returns possibly more * metadata value than requested. @@ -131,35 +123,29 @@ public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @C * @param httpHeaders httpheaders * @param info info * @param field the tika metadata field name - * @return one of {@link javax.ws.rs.core.Response.Status#OK}, - * {@link javax.ws.rs.core.Response.Status#NOT_FOUND}, or - * {@link javax.ws.rs.core.Response.Status#BAD_REQUEST} + * @return one of {@link javax.ws.rs.core.Response.Status#OK} or + * {@link javax.ws.rs.core.Response.Status#NOT_FOUND} * @throws Exception */ @PUT @Path("{field}") @Produces({"text/csv", "application/json", "text/plain"}) public Response getMetadataField(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info, @PathParam("field") String field) throws Exception { - - // use BAD request to indicate that we may not have had enough data to - // process the request - Response.Status defaultErrorResponse = Response.Status.BAD_REQUEST; ParseContext context = tikaResource.createParseContext(); - Metadata metadata = Metadata.newInstance(context); - boolean success = false; + Metadata metadata; try (TikaInputStream tis = TikaInputStream.get(is)) { - parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), info); - // once we've parsed the document successfully, we should use NOT_FOUND - // if we did not see the field - defaultErrorResponse = Response.Status.NOT_FOUND; - success = true; - } catch (Exception e) { - LOG.warn("Failed to process field {}", field, e); + tis.getPath(); // Spool to temp file for pipes-based parsing + metadata = parseMetadata(tis, Metadata.newInstance(context), httpHeaders.getRequestHeaders(), context); + } + + String containerException = metadata.get(TikaCoreProperties.CONTAINER_EXCEPTION); + if (containerException != null && !containerException.isEmpty()) { + throw new TikaServerParseException(new TikaException(containerException)); } - if (success == false || metadata.get(field) == null) { + if (metadata.get(field) == null) { return Response - .status(defaultErrorResponse) + .status(Response.Status.NOT_FOUND) .entity("Failed to get metadata field " + field) .build(); } @@ -175,21 +161,26 @@ public Response getMetadataField(InputStream is, @Context HttpHeaders httpHeader .build(); } - protected Metadata parseMetadata(TikaInputStream tis, Metadata metadata, MultivaluedMap httpHeaders, UriInfo info) - throws IOException, TikaConfigException { - // Load default context from config (includes DigesterFactory from parse-context) - final ParseContext context = tikaResource.createParseContext(); - Parser parser = tikaResource.createParser(); - fillMetadata(parser, metadata, httpHeaders); - //no need to parse embedded docs - context.set(DocumentSelector.class, metadata1 -> false); + /** + * Parses via the shared pipes-backed PipesParser, stopping at the container document + * (EmbeddedLimits maxDepth=0) with content capture off ("ignore" handler) -- metadata + * only, matching /meta's contract. Set unconditionally so per-request config can't + * turn content capture back on. A container-level exception is embedded in + * CONTAINER_EXCEPTION here, not thrown; getMetadataField throws instead since it + * returns a bare scalar with nowhere to embed it. + */ + protected Metadata parseMetadata(TikaInputStream tis, Metadata metadata, MultivaluedMap httpHeaders, ParseContext context) + throws Exception { + fillMetadata(null, metadata, httpHeaders); + context.set(EmbeddedLimits.class, new EmbeddedLimits(0, false, EmbeddedLimits.UNLIMITED, false)); + context.set(ContentHandlerFactory.class, + new BasicContentHandlerFactory(BasicContentHandlerFactory.HANDLER_TYPE.IGNORE, -1)); TikaResource.logRequest(LOG, "/meta", metadata); - tikaResource.parse(parser, LOG, info.getPath(), tis, new LanguageHandler() { - public void endDocument() { - metadata.set("language", getLanguage().getLanguage()); - } - }, metadata, context); - return metadata; + List metadataList = tikaResource.parseWithPipes(tis, metadata, context, ParseMode.RMETA); + if (metadataList.isEmpty()) { + return Metadata.newInstance(context); + } + 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 7a0660d639..2d87de0605 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 @@ -34,6 +34,7 @@ import org.apache.tika.io.TikaInputStream; import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.Property; import org.apache.tika.metadata.TikaCoreProperties; import org.apache.tika.parser.ParseContext; import org.apache.tika.pipes.api.FetchEmitTuple; @@ -42,6 +43,8 @@ import org.apache.tika.pipes.api.emitter.EmitData; import org.apache.tika.pipes.api.emitter.EmitKey; import org.apache.tika.pipes.api.fetcher.FetchKey; +import org.apache.tika.pipes.core.EmitStrategy; +import org.apache.tika.pipes.core.EmitStrategyConfig; import org.apache.tika.pipes.core.PipesConfig; import org.apache.tika.pipes.core.PipesException; import org.apache.tika.pipes.core.PipesParser; @@ -145,6 +148,12 @@ public List parse(TikaInputStream tis, Metadata metadata, // Set parse mode in context parseContext.set(ParseMode.class, parseMode); + // This parser is shared with /pipes, whose own default is EMIT_ALL. No + // emitter is configured for /tika/rmeta/unpack requests (EmitKey.NO_EMIT + // below) -- results must come back over the socket, so set PASSBACK_ALL + // explicitly per-request rather than relying on the parser-level default. + parseContext.set(EmitStrategyConfig.class, new EmitStrategyConfig(EmitStrategy.PASSBACK_ALL)); + // Create FetchEmitTuple with relative filename (basePath is configured in fetcher) FetchKey fetchKey = new FetchKey(DEFAULT_FETCHER_ID, relativeName); @@ -160,7 +169,9 @@ public List parse(TikaInputStream tis, Metadata metadata, PipesResult result = pipesParser.parse(tuple); // Process result - return processResult(result); + List metadataList = processResult(result); + redactExceptionDetail(metadataList); + return metadataList; } catch (InterruptedException e) { Thread.currentThread().interrupt(); @@ -273,6 +284,40 @@ private List processResult(PipesResult result) { return Collections.emptyList(); } + /** + * Trims CONTAINER_EXCEPTION/EMBEDDED_EXCEPTION to one line unless returnStackTrace is + * on -- unlike buildProcessFailureResponse's family, a 200 response has no other way + * to signal a per-document exception, so we can't omit these fields entirely. + */ + private void redactExceptionDetail(List metadataList) { + if (returnStackTrace || metadataList == null) { + return; + } + for (Metadata m : metadataList) { + summarizeInPlace(m, TikaCoreProperties.CONTAINER_EXCEPTION); + summarizeInPlace(m, TikaCoreProperties.EMBEDDED_EXCEPTION); + } + } + + private static void summarizeInPlace(Metadata m, Property property) { + String full = m.get(property); + if (full != null) { + m.set(property, summarizeStackTrace(full, false)); + } + } + + /** + * First line of a stack trace (the caught exception's own class + message); no-op if + * returnStackTrace. + */ + public static String summarizeStackTrace(String fullTrace, boolean returnStackTrace) { + if (returnStackTrace || fullTrace == null || fullTrace.isBlank()) { + return fullTrace; + } + int newline = fullTrace.indexOf('\n'); + return newline < 0 ? fullTrace : fullTrace.substring(0, newline); + } + /** * Maps PipesResult status to HTTP response status. */ @@ -306,6 +351,14 @@ public PipesParser getPipesParser() { return pipesParser; } + /** + * Whether failure responses may include the (potentially stack-trace-bearing) + * {@code PipesResult} message. Mirrors {@code TikaServerConfig.isReturnStackTrace()}. + */ + public boolean isReturnStackTrace() { + return returnStackTrace; + } + /** * Gets the PipesConfig instance. */ @@ -358,6 +411,12 @@ public UnpackResult parseUnpack(TikaInputStream tis, Metadata metadata, // Set parse mode to UNPACK parseContext.set(ParseMode.class, ParseMode.UNPACK); + // Shared parser (see parse() above) -- PASSBACK_ALL is also required here + // for correctness: with UNPACK mode, EmitHandler.shouldEmit() only skips + // re-emitting metadata (already emitted as part of the zip) when the + // effective strategy is PASSBACK_ALL. + parseContext.set(EmitStrategyConfig.class, new EmitStrategyConfig(EmitStrategy.PASSBACK_ALL)); + // Configure UnpackConfig - use existing or create new UnpackConfig unpackConfig = parseContext.get(UnpackConfig.class); if (unpackConfig == null) { @@ -428,18 +487,10 @@ public UnpackResult parseUnpack(TikaInputStream tis, Metadata metadata, Metadata containerMetadata = metadataList.get(0); String containerException = containerMetadata.get(TikaCoreProperties.CONTAINER_EXCEPTION); if (containerException != null) { - // Map exception type to HTTP status - // 422 (Unprocessable Entity) for parse-related exceptions - int status = 422; // Default for parse exceptions - if (containerException.contains("EncryptedDocumentException") || - containerException.contains("TikaException") || - containerException.contains("NullPointerException") || - containerException.contains("IllegalStateException")) { - status = 422; - } - // Build response with exception string as body for stack trace support - Response response = Response.status(status) - .entity(containerException) + // 422 already signals failure, so (unlike redactExceptionDetail's + // 200 family) the body can be omitted entirely when off. + Response response = Response.status(422) + .entity(returnStackTrace ? containerException : "") .type("text/plain") .build(); throw new WebApplicationException(response); diff --git a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesResource.java b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesResource.java index 2c6316e081..6b7eb9fccd 100644 --- a/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesResource.java +++ b/tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/resource/PipesResource.java @@ -29,19 +29,17 @@ import jakarta.ws.rs.Produces; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.HttpHeaders; +import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriInfo; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.apache.tika.config.loader.TikaJsonConfig; -import org.apache.tika.exception.TikaConfigException; -import org.apache.tika.metadata.Metadata; import org.apache.tika.metadata.TikaCoreProperties; +import org.apache.tika.parser.ParseContext; import org.apache.tika.pipes.api.FetchEmitTuple; import org.apache.tika.pipes.api.PipesResult; import org.apache.tika.pipes.core.EmitStrategy; import org.apache.tika.pipes.core.EmitStrategyConfig; -import org.apache.tika.pipes.core.PipesConfig; import org.apache.tika.pipes.core.PipesException; import org.apache.tika.pipes.core.PipesParser; import org.apache.tika.pipes.core.serialization.JsonFetchEmitTuple; @@ -54,18 +52,18 @@ public class PipesResource { private static final Logger LOG = LoggerFactory.getLogger(PipesResource.class); private final PipesParser pipesParser; + private final boolean returnStackTrace; - public PipesResource(java.nio.file.Path tikaConfig) throws TikaConfigException, IOException { - TikaJsonConfig tikaJsonConfig = TikaJsonConfig.load(tikaConfig); - PipesConfig pipesConfig = PipesConfig.load(tikaJsonConfig); - // The /pipes endpoint always emits from the child process; force EMIT_ALL. - if (pipesConfig.getEmitStrategy().getType() != EmitStrategy.EMIT_ALL) { - if (pipesConfig.getEmitStrategy().getType() != EmitStrategyConfig.DEFAULT_EMIT_STRATEGY) { - LOG.warn("resetting emit strategy to EMIT_ALL for pipes endpoint"); - } - pipesConfig.setEmitStrategy(new EmitStrategyConfig(EmitStrategy.EMIT_ALL)); - } - this.pipesParser = PipesParser.load(tikaJsonConfig, pipesConfig, tikaConfig); + /** + * @param pipesParser shared parser, also used by /tika, /rmeta, and /unpack. + * Lifecycle (construction, shutdown) is owned by whoever + * built it, not by this class. + * @param returnStackTrace whether parse_exception may include the full stack trace + * vs. just the first line. + */ + public PipesResource(PipesParser pipesParser, boolean returnStackTrace) { + this.pipesParser = pipesParser; + this.returnStackTrace = returnStackTrace; } @@ -82,12 +80,14 @@ public PipesResource(java.nio.file.Path tikaConfig) throws TikaConfigException, * Must specify a fetcherString and an emitter in the posted json. * * @param info uri info - * @return InputStream that can be deserialized as a list of {@link Metadata} objects + * @return a JSON body describing the outcome (status/type, or a parse_exception), + * with HTTP status reflecting whether the process succeeded, crashed, or + * was unavailable within the configured wait * @throws Exception */ @POST @Produces("application/json") - public Map postRmeta(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { + public Response postRmeta(InputStream is, @Context HttpHeaders httpHeaders, @Context UriInfo info) throws Exception { FetchEmitTuple t = null; try (Reader reader = new InputStreamReader(is, StandardCharsets.UTF_8)) { t = JsonFetchEmitTuple.fromJson(reader); @@ -97,30 +97,45 @@ public Map postRmeta(InputStream is, @Context HttpHeaders httpHe return processTuple(t); } - private Map processTuple(FetchEmitTuple fetchEmitTuple) throws InterruptedException, PipesException, IOException { - + private Response processTuple(FetchEmitTuple fetchEmitTuple) throws InterruptedException, PipesException, IOException { + // This parser is shared with /tika+/rmeta+/unpack, whose own default is + // PASSBACK_ALL. /pipes needs the child to emit via the client's configured + // emitter by default -- set EMIT_ALL explicitly per-request rather than + // relying on the parser-level default, but don't clobber a caller's own + // explicit override if they set one. + ParseContext parseContext = fetchEmitTuple.getParseContext(); + if (parseContext.get(EmitStrategyConfig.class) == null) { + parseContext.set(EmitStrategyConfig.class, new EmitStrategyConfig(EmitStrategy.EMIT_ALL)); + } PipesResult pipesResult = pipesParser.parse(fetchEmitTuple); + Map body; if (pipesResult.isProcessCrash()) { - return returnProcessCrash(pipesResult.status().toString()); + body = returnProcessCrash(pipesResult.status().toString()); } else if (!pipesResult.isSuccess()) { // Handle fatal errors, initialization failures, and task exceptions - return returnApplicationError(pipesResult + body = returnApplicationError(pipesResult .status() .toString()); + } else { + body = switch (pipesResult.status()) { + case EMIT_SUCCESS_PARSE_EXCEPTION -> parseException(pipesResult.message(), true); + case PARSE_EXCEPTION_NO_EMIT -> parseException(pipesResult.message(), false); + default -> returnSuccess(); + }; } - switch (pipesResult.status()) { - case EMIT_SUCCESS_PARSE_EXCEPTION: - return parseException(pipesResult.message(), true); - case PARSE_EXCEPTION_NO_EMIT: - return parseException(pipesResult.message(), false); - } - return returnSuccess(); + // Same status mapping /tika+/rmeta+/unpack use (PipesParsingHelper) -- e.g. 429 for + // CLIENT_UNAVAILABLE_WITHIN_MS, 503 for TIMEOUT/OOM/UNSPECIFIED_CRASH -- rather than + // always 200 with the failure only visible in the body. + return Response.status(PipesParsingHelper.mapStatusToHttpResponse(pipesResult.status())) + .entity(body) + .build(); } private Map parseException(String msg, boolean emitted) { Map statusMap = new HashMap<>(); statusMap.put("status", "ok"); - statusMap.put("parse_exception", msg); + // 200 response, so trim rather than omit -- same reasoning as redactExceptionDetail. + statusMap.put("parse_exception", PipesParsingHelper.summarizeStackTrace(msg, returnStackTrace)); statusMap.put("emitted", Boolean.toString(emitted)); return statusMap; } @@ -144,8 +159,4 @@ private Map returnApplicationError(String type) { statusMap.put("type", type); return statusMap; } - - public void close() throws IOException { - pipesParser.close(); - } } 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 fd8cd651b4..5ac7fe1b4b 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 @@ -515,7 +515,7 @@ public String getMessage() { @PUT @Consumes("*/*") @Produces("text/xml") - public StreamingOutput getXhtml(final InputStream is, @Context HttpHeaders httpHeaders) + 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 @@ -530,7 +530,7 @@ public StreamingOutput getXhtml(final InputStream is, @Context HttpHeaders httpH @Consumes("*/*") @Produces("text/plain") @Path("text") - public StreamingOutput getText(final InputStream is, @Context HttpHeaders httpHeaders) + 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 @@ -545,7 +545,7 @@ public StreamingOutput getText(final InputStream is, @Context HttpHeaders httpHe @Consumes("*/*") @Produces("text/html") @Path("html") - public StreamingOutput getHtml(final InputStream is, @Context HttpHeaders httpHeaders) + 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 @@ -560,7 +560,7 @@ public StreamingOutput getHtml(final InputStream is, @Context HttpHeaders httpHe @Consumes("*/*") @Produces("text/xml") @Path("xml") - public StreamingOutput getXml(final InputStream is, @Context HttpHeaders httpHeaders) + 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 @@ -575,7 +575,7 @@ public StreamingOutput getXml(final InputStream is, @Context HttpHeaders httpHea @Consumes("*/*") @Produces("text/plain") @Path("md") - public StreamingOutput getMarkdown(final InputStream is, @Context HttpHeaders httpHeaders) + 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 @@ -634,7 +634,7 @@ public Metadata getJson(final InputStream is, @Context HttpHeaders httpHeaders, @Consumes("multipart/form-data") @Produces("text/xml") @Path("config") - public StreamingOutput postRaw(List attachments, @Context HttpHeaders httpHeaders) + public Response postRaw(List attachments, @Context HttpHeaders httpHeaders) throws IOException, TikaConfigException { ParseContext context = createParseContext(); Metadata metadata = Metadata.newInstance(context); @@ -657,7 +657,7 @@ public StreamingOutput postRaw(List attachments, @Context HttpHeader @Consumes("multipart/form-data") @Produces("text/plain") @Path("config/text") - public StreamingOutput postText(List attachments, @Context HttpHeaders httpHeaders) + public Response postText(List attachments, @Context HttpHeaders httpHeaders) throws IOException, TikaConfigException { ParseContext context = createParseContext(); Metadata metadata = Metadata.newInstance(context); @@ -679,7 +679,7 @@ public StreamingOutput postText(List attachments, @Context HttpHeade @Consumes("multipart/form-data") @Produces("text/html") @Path("config/html") - public StreamingOutput postHtml(List attachments, @Context HttpHeaders httpHeaders) + public Response postHtml(List attachments, @Context HttpHeaders httpHeaders) throws IOException, TikaConfigException { ParseContext context = createParseContext(); Metadata metadata = Metadata.newInstance(context); @@ -701,7 +701,7 @@ public StreamingOutput postHtml(List attachments, @Context HttpHeade @Consumes("multipart/form-data") @Produces("text/xml") @Path("config/xml") - public StreamingOutput postXml(List attachments, @Context HttpHeaders httpHeaders) + public Response postXml(List attachments, @Context HttpHeaders httpHeaders) throws IOException, TikaConfigException { ParseContext context = createParseContext(); Metadata metadata = Metadata.newInstance(context); @@ -723,7 +723,7 @@ public StreamingOutput postXml(List attachments, @Context HttpHeader @Consumes("multipart/form-data") @Produces("text/plain") @Path("config/md") - public StreamingOutput postMarkdown(List attachments, @Context HttpHeaders httpHeaders) + public Response postMarkdown(List attachments, @Context HttpHeaders httpHeaders) throws IOException, TikaConfigException { ParseContext context = createParseContext(); Metadata metadata = Metadata.newInstance(context); @@ -760,7 +760,7 @@ public Metadata postJson(List attachments, @Context HttpHeaders http /** * Produces raw streaming output (text, html, xml, md) using pipes-based parsing. */ - private StreamingOutput produceRawOutput(TikaInputStream tis, Metadata metadata, + private Response produceRawOutput(TikaInputStream tis, Metadata metadata, MultivaluedMap httpHeaders, String handlerTypeName) throws IOException { fillMetadata(null, metadata, httpHeaders); @@ -771,8 +771,11 @@ private StreamingOutput produceRawOutput(TikaInputStream tis, Metadata metadata, /** * Produces raw streaming output with a pre-configured ParseContext (for PUT endpoints). + * A container-level parse exception doesn't discard content already captured -- status + * is 422 (no field to embed the exception in, unlike the JSON endpoints), but the body + * still carries whatever content was actually extracted. */ - private StreamingOutput produceRawOutputWithContext(TikaInputStream tis, Metadata metadata, + private Response produceRawOutputWithContext(TikaInputStream tis, Metadata metadata, ParseContext context, String handlerTypeName) throws IOException { logRequest(LOG, "/tika", metadata); @@ -794,42 +797,45 @@ private StreamingOutput produceRawOutputWithContext(TikaInputStream tis, Metadat LOG.debug("produceRawOutput: parseWithPipes returned {} metadata objects", metadataList.size()); - // For raw streaming endpoints, throw exception if there was a parse error - // (JSON endpoints return exceptions in metadata) - // Note: CONTAINER_EXCEPTION is extracted before the metadata filter runs, - // so it's available in the passback even though the filter strips it - if (!metadataList.isEmpty()) { - String exception = metadataList.get(0).get(TikaCoreProperties.CONTAINER_EXCEPTION); - if (exception != null && !exception.isEmpty()) { - LOG.debug("produceRawOutput: parse exception: {}", exception); - // Wrap in TikaException so TikaServerParseExceptionMapper returns 422 - throw new TikaServerParseException(new TikaException(exception)); - } - } - - // Extract content from result + // Extract content before checking for an exception -- content must not be + // discarded just because a container-level exception also occurred. String content = ""; + boolean hasException = false; + String exceptionMessage = null; if (!metadataList.isEmpty()) { String extracted = metadataList.get(0).get(TikaCoreProperties.TIKA_CONTENT); LOG.debug("produceRawOutput: TIKA_CONTENT length={}", extracted != null ? extracted.length() : 0); if (extracted != null) { content = extracted; } + exceptionMessage = metadataList.get(0).get(TikaCoreProperties.CONTAINER_EXCEPTION); + hasException = exceptionMessage != null && !exceptionMessage.isEmpty(); + if (hasException) { + LOG.debug("produceRawOutput: parse exception: {}", exceptionMessage); + } + } + // No separate field for the exception here, unlike JSON bodies -- append it, + // gated by returnStackTrace like TikaServerParseExceptionMapper. + if (hasException && pipesParsingHelper != null && pipesParsingHelper.isReturnStackTrace()) { + content = content.isEmpty() ? exceptionMessage : content + "\n" + exceptionMessage; } final String finalContent = content; - return outputStream -> { + StreamingOutput streamingOutput = outputStream -> { try (Writer writer = new OutputStreamWriter(outputStream, UTF_8)) { writer.write(finalContent); writer.flush(); } }; + return Response.status(hasException ? 422 : Response.Status.OK.getStatusCode()) + .entity(streamingOutput) + .build(); } /** * Produces raw streaming output with a pre-configured ParseContext (for POST endpoints). */ - private StreamingOutput produceRawOutput(TikaInputStream tis, Metadata metadata, + private Response produceRawOutput(TikaInputStream tis, Metadata metadata, ParseContext context, String handlerTypeName) throws IOException { return produceRawOutputWithContext(tis, metadata, context, handlerTypeName); 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 da625df6dd..ed557ad8cb 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 @@ -215,7 +215,7 @@ public void setUp() throws Exception { pipesConfig.setEmitStrategy(new EmitStrategyConfig(EmitStrategy.PASSBACK_ALL)); this.pipesParser = PipesParser.load(tikaJsonConfig, pipesConfig, this.pipesConfigPath); PipesParsingHelper pipesParsingHelper = new PipesParsingHelper(this.pipesParser, pipesConfig, - inputTempDirectory, getUnpackEmitterBasePath(), false); + inputTempDirectory, getUnpackEmitterBasePath(), isReturnStackTrace()); tikaResource = new TikaResource(tika, new ServerStatus(), pipesParsingHelper, isAllowPerRequestConfig()); } finally { @@ -377,6 +377,14 @@ protected boolean isAllowPerRequestConfig() { return false; } + /** + * Mirrors TikaServerConfig.isReturnStackTrace(); defaults to false (production + * default). Override in tests that exercise exception-detail visibility. + */ + protected boolean isReturnStackTrace() { + return false; + } + protected InputStream getPipesConfigInputStream() throws IOException { if (getPipesInputPath() == null) { return null; diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/StackTraceTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/StackTraceTest.java index c1aa7b7248..752a8af690 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/StackTraceTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/StackTraceTest.java @@ -67,6 +67,12 @@ public class StackTraceTest extends CXFTestBase { @TempDir private static Path unpackTempDir; + @Override + protected boolean isReturnStackTrace() { + // Matches this class's own TikaServerParseExceptionMapper(true) below. + return true; + } + @Override protected void setUpResources(JAXRSServerFactoryBean sf) { List rCoreProviders = new ArrayList<>(); @@ -113,7 +119,8 @@ protected Path getUnpackEmitterBasePath() { @Test public void testEncrypted() throws Exception { for (String path : PATHS) { - if ("/rmeta".equals(path)) { + // /rmeta and /meta embed a container exception at 200 instead of throwing 422. + if ("/rmeta".equals(path) || "/meta".equals(path)) { continue; } // Use path-based routing for /tika @@ -132,7 +139,8 @@ public void testEncrypted() throws Exception { @Test public void testNullPointerOnTika() throws Exception { for (String path : PATHS) { - if ("/rmeta".equals(path)) { + // Same as testEncrypted. + if ("/rmeta".equals(path) || "/meta".equals(path)) { continue; } // Use path-based routing for /tika @@ -170,10 +178,7 @@ public void testEmptyParser() throws Exception { } - //For now, make sure that non-complete document - //still returns BAD_REQUEST. We may want to - //make MetadataResource return the same types of parse - //exceptions as the others... + // A truncated document isn't a process failure -- NOT_FOUND, not BAD_REQUEST. @Test public void testMeta() throws Exception { InputStream stream = ClassLoader.getSystemResourceAsStream(TEST_HELLO_WORLD); @@ -183,7 +188,7 @@ public void testMeta() throws Exception { .type("application/mock+xml") .accept(MediaType.TEXT_PLAIN) .put(copy(stream, 100)); - assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), response.getStatus()); String msg = getStringFromInputStream((InputStream) response.getEntity()); assertEquals("Failed to get metadata field Author", msg); } diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaPipesTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaPipesTest.java index 38b1ec47a7..d43abc4c43 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaPipesTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaPipesTest.java @@ -49,6 +49,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; +import org.apache.tika.config.loader.TikaJsonConfig; import org.apache.tika.exception.TikaConfigException; import org.apache.tika.metadata.Metadata; import org.apache.tika.metadata.TikaCoreProperties; @@ -57,6 +58,10 @@ import org.apache.tika.pipes.api.ParseMode; import org.apache.tika.pipes.api.emitter.EmitKey; import org.apache.tika.pipes.api.fetcher.FetchKey; +import org.apache.tika.pipes.core.EmitStrategy; +import org.apache.tika.pipes.core.EmitStrategyConfig; +import org.apache.tika.pipes.core.PipesConfig; +import org.apache.tika.pipes.core.PipesParser; import org.apache.tika.pipes.core.serialization.JsonFetchEmitTuple; import org.apache.tika.sax.BasicContentHandlerFactory; import org.apache.tika.sax.ContentHandlerFactory; @@ -85,6 +90,7 @@ public class TikaPipesTest extends CXFTestBase { private static final String[] VALUE_ARRAY = new String[]{"my-value-1", "my-value-2", "my-value-3"}; private PipesResource pipesResource; + private PipesParser pipesParser; @Override @BeforeAll @@ -115,10 +121,11 @@ public void setUp() throws Exception { @Override @AfterAll public void tearDown() throws Exception { - if (pipesResource != null) { - pipesResource.close(); - pipesResource = null; + if (pipesParser != null) { + pipesParser.close(); + pipesParser = null; } + pipesResource = null; super.tearDown(); if (tmpDir != null) { FileUtils.deleteDirectory(tmpDir.toFile()); @@ -140,7 +147,13 @@ public void setUpEachTest() throws Exception { protected void setUpResources(JAXRSServerFactoryBean sf) { List rCoreProviders = new ArrayList<>(); try { - pipesResource = new PipesResource(tikaConfigPath); + // Mirrors what PipesResource used to build internally, back when it + // constructed its own parser instead of sharing one. + TikaJsonConfig tikaJsonConfig = TikaJsonConfig.load(tikaConfigPath); + PipesConfig pipesConfig = PipesConfig.load(tikaJsonConfig); + pipesConfig.setEmitStrategy(new EmitStrategyConfig(EmitStrategy.EMIT_ALL)); + pipesParser = PipesParser.load(tikaJsonConfig, pipesConfig, tikaConfigPath); + pipesResource = new PipesResource(pipesParser, false); rCoreProviders.add(new SingletonResourceProvider(pipesResource)); } catch (IOException | TikaConfigException e) { throw new RuntimeException(e); diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaResourceTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaResourceTest.java index c5d5ce6da7..c0f01e0863 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaResourceTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaResourceTest.java @@ -109,7 +109,11 @@ public void testJsonNPE() throws Exception { assertEquals("Nikolai Lobachevsky", metadata.get("author")); assertEquals("application/mock+xml", metadata.get(Metadata.CONTENT_TYPE)); assertContains("some content", metadata.get(TikaCoreProperties.TIKA_CONTENT)); - assertContains("null pointer message", metadata.get(TikaCoreProperties.CONTAINER_EXCEPTION)); + // returnStackTrace defaults to false here, so CONTAINER_EXCEPTION is trimmed to + // the caught exception's own class + message -- the NPE detail underneath it is + // intentionally not exposed by default. + assertContains("TikaException", metadata.get(TikaCoreProperties.CONTAINER_EXCEPTION)); + assertNotFound("null pointer message", metadata.get(TikaCoreProperties.CONTAINER_EXCEPTION)); } @Test diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerPipesIntegrationTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerPipesIntegrationTest.java index 01637dd308..675518069d 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerPipesIntegrationTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerPipesIntegrationTest.java @@ -151,7 +151,7 @@ public void testSystemExit() throws Exception { "-config", ProcessUtils.escapeCommandLine(TIKA_CONFIG .toAbsolutePath() .toString())}); - JsonNode node = testOne("system_exit.xml", false); + JsonNode node = testOne("system_exit.xml", false, FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT, 503); assertEquals("process_crash", node .get("status") .asText()); @@ -168,7 +168,7 @@ public void testOOM() throws Exception { "-config", ProcessUtils.escapeCommandLine(TIKA_CONFIG .toAbsolutePath() .toString())}); - JsonNode node = testOne("fake_oom.xml", false); + JsonNode node = testOne("fake_oom.xml", false, FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT, 503); assertEquals("process_crash", node .get("status") .asText()); @@ -188,7 +188,7 @@ public void testTimeout() throws Exception { "-config", ProcessUtils.escapeCommandLine(TIKA_CONFIG_TIMEOUT .toAbsolutePath() .toString())}); - JsonNode node = testOne("heavy_hang_30000.xml", false); + JsonNode node = testOne("heavy_hang_30000.xml", false, FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT, 503); assertEquals("process_crash", node .get("status") .asText()); @@ -206,7 +206,7 @@ public void testPerRequestTimeout() throws Exception { "-config", ProcessUtils.escapeCommandLine(TIKA_CONFIG .toAbsolutePath() .toString())}); - JsonNode node = testOneWithPerRequestTimeout("heavy_hang_30000.xml", 100); + JsonNode node = testOneWithPerRequestTimeout("heavy_hang_30000.xml", 100, 503); assertEquals("process_crash", node .get("status") .asText()); @@ -215,17 +215,15 @@ public void testPerRequestTimeout() throws Exception { .asText()); } - private JsonNode testOneWithPerRequestTimeout(String fileName, long timeoutMillis) throws Exception { + private JsonNode testOneWithPerRequestTimeout(String fileName, long timeoutMillis, int expectedStatus) throws Exception { awaitServerStartup(); Response response = WebClient .create(endPoint + "/pipes") .accept("application/json") .post(getJsonStringWithTimeout(fileName, timeoutMillis)); - if (response.getStatus() == 200) { - Reader reader = new InputStreamReader((InputStream) response.getEntity(), UTF_8); - return new ObjectMapper().readTree(reader); - } - return null; + assertEquals(expectedStatus, response.getStatus()); + Reader reader = new InputStreamReader((InputStream) response.getEntity(), UTF_8); + return new ObjectMapper().readTree(reader); } private String getJsonStringWithTimeout(String fileName, long timeoutMillis) throws IOException { @@ -245,27 +243,30 @@ private String getJsonStringWithTimeout(String fileName, long timeoutMillis) thr } private JsonNode testOne(String fileName, boolean shouldFileExist) throws Exception { - return testOne(fileName, shouldFileExist, FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT); + return testOne(fileName, shouldFileExist, FetchEmitTuple.ON_PARSE_EXCEPTION.EMIT, 200); } private JsonNode testOne(String fileName, boolean shouldFileExist, FetchEmitTuple.ON_PARSE_EXCEPTION onParseException) throws Exception { + return testOne(fileName, shouldFileExist, onParseException, 200); + } + + private JsonNode testOne(String fileName, boolean shouldFileExist, + FetchEmitTuple.ON_PARSE_EXCEPTION onParseException, int expectedStatus) throws Exception { awaitServerStartup(); Response response = WebClient .create(endPoint + "/pipes") .accept("application/json") .post(getJsonString(fileName, onParseException)); - if (response.getStatus() == 200) { - Path targFile = TEMP_OUTPUT_DIR.resolve(fileName + ".json"); - if (shouldFileExist) { - assertTrue(Files.size(targFile) > 1); - } else { - assertFalse(Files.isRegularFile(targFile)); - } - Reader reader = new InputStreamReader((InputStream) response.getEntity(), UTF_8); - return new ObjectMapper().readTree(reader); + assertEquals(expectedStatus, response.getStatus()); + Path targFile = TEMP_OUTPUT_DIR.resolve(fileName + ".json"); + if (shouldFileExist) { + assertTrue(Files.size(targFile) > 1); + } else { + assertFalse(Files.isRegularFile(targFile)); } - return null; + Reader reader = new InputStreamReader((InputStream) response.getEntity(), UTF_8); + return new ObjectMapper().readTree(reader); } @Test diff --git a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTest.java b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTest.java index 351daa770b..d7acbf6173 100644 --- a/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTest.java +++ b/tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerProcessTest.java @@ -17,7 +17,9 @@ package org.apache.tika.server.core; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; import java.util.List; @@ -51,4 +53,13 @@ public void ordinaryEndpointIsAllowedWithoutAllowPipes() { assertDoesNotThrow( () -> TikaServerProcess.loadCoreProviders(config(false, "meta"), null, null)); } + + @Test + public void metaAloneNeedsPipesParsingHelper() { + // /meta is now pipes-backed too; a config listing only "meta" (no tika/rmeta/ + // unpack/pipes) must still build the shared PipesParser, or every /meta request + // hits IllegalStateException("Pipes-based parsing is not enabled"). + assertTrue(TikaServerProcess.needsPipesParsingHelper(config(false, "meta"))); + assertFalse(TikaServerProcess.needsPipesParsingHelper(config(false, "status"))); + } } 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 3fc7b4501b..6bf07562ef 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 @@ -58,8 +58,9 @@ public Response getMetadataField(InputStream is, @Context HttpHeaders httpHeader public Response getMetadataFromMultipart(Attachment att, @Context UriInfo info) throws Exception { ParseContext context = new ParseContext(); try (TikaInputStream tis = TikaInputStream.get(att.getObject(InputStream.class))) { + tis.getPath(); // Spool to temp file for pipes-based parsing return Response - .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), info)) + .ok(parseMetadata(tis, Metadata.newInstance(context), att.getHeaders(), context)) .build(); } } @@ -70,8 +71,9 @@ public Response getMetadata(InputStream is, @Context HttpHeaders httpHeaders, @C ParseContext context = new ParseContext(); Metadata metadata = Metadata.newInstance(context); try (TikaInputStream tis = TikaInputStream.get(is)) { + tis.getPath(); // Spool to temp file for pipes-based parsing return Response - .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), info)) + .ok(parseMetadata(tis, metadata, httpHeaders.getRequestHeaders(), context)) .build(); } } diff --git a/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/MetadataResourceTest.java b/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/MetadataResourceTest.java index 6e205486ee..e548f9d5c6 100644 --- a/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/MetadataResourceTest.java +++ b/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/MetadataResourceTest.java @@ -47,6 +47,7 @@ import org.apache.tika.metadata.TikaCoreProperties; import org.apache.tika.serialization.JsonMetadata; import org.apache.tika.server.core.CXFTestBase; +import org.apache.tika.server.core.TikaServerParseExceptionMapper; import org.apache.tika.server.core.resource.MetadataResource; import org.apache.tika.server.core.writer.CSVMessageBodyWriter; import org.apache.tika.server.core.writer.JSONMessageBodyWriter; @@ -73,6 +74,8 @@ protected void setUpResources(JAXRSServerFactoryBean sf) { @Override protected void setUpProviders(JAXRSServerFactoryBean sf) { List providers = new ArrayList<>(); + // Needed by getMetadataField's TikaServerParseException throw. + providers.add(new TikaServerParseExceptionMapper(false)); providers.add(new JSONMessageBodyWriter()); providers.add(new CSVMessageBodyWriter()); providers.add(new XMPMessageBodyWriter()); @@ -118,10 +121,13 @@ public void testPasswordProtected() throws Exception { .accept("application/json") .post(new MultipartBody(Arrays.asList(fileAtt))); - // Won't work, no password given - EncryptedDocumentException returns 422 - assertEquals(500, response.getStatus()); + // A failed decrypt isn't a process failure -- 200, exception on the metadata. + assertEquals(200, response.getStatus()); + Metadata noPasswordMetadata = JsonMetadata.fromJson(new InputStreamReader((InputStream) response.getEntity(), UTF_8)); + assertContains("org.apache.tika.exception.EncryptedDocumentException", + noPasswordMetadata.get(TikaCoreProperties.CONTAINER_EXCEPTION)); - // Test 2: Wrong password - should fail + // Test 2: Wrong password - should fail the same way fileCd = new ContentDisposition("form-data; name=\"file\"; filename=\"test.xls\""); fileAtt = new Attachment("file", ClassLoader.getSystemResourceAsStream(TikaResourceTest.TEST_PASSWORD_PROTECTED), fileCd); @@ -142,7 +148,10 @@ public void testPasswordProtected() throws Exception { .accept("application/json") .post(new MultipartBody(Arrays.asList(fileAtt, wrongConfigAtt))); - assertEquals(500, response.getStatus()); + assertEquals(200, response.getStatus()); + Metadata wrongPasswordMetadata = JsonMetadata.fromJson(new InputStreamReader((InputStream) response.getEntity(), UTF_8)); + assertContains("org.apache.tika.exception.EncryptedDocumentException", + wrongPasswordMetadata.get(TikaCoreProperties.CONTAINER_EXCEPTION)); // Test 3: Correct password - should work fileCd = new ContentDisposition("form-data; name=\"file\"; filename=\"test.xls\""); @@ -213,8 +222,9 @@ public void testGetField_XXX_NotFound() throws Exception { } @Test - public void testGetField_Author_TEXT_Partial_BAD_REQUEST() throws Exception { - + public void testGetField_Author_TEXT_Partial_UNPROCESSABLE() throws Exception { + // Truncating at 8000 bytes corrupts the OLE2 structure enough that OfficeParser + // throws -- a real container exception, not just a missing field. InputStream stream = ClassLoader.getSystemResourceAsStream(TikaResourceTest.TEST_DOC); Response response = WebClient @@ -222,7 +232,7 @@ public void testGetField_Author_TEXT_Partial_BAD_REQUEST() throws Exception { .type("application/msword") .accept(MediaType.TEXT_PLAIN) .put(copy(stream, 8000)); - assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals(422, response.getStatus()); } @Test diff --git a/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/TikaPipesTest.java b/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/TikaPipesTest.java index 544382a2bd..08844f84df 100644 --- a/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/TikaPipesTest.java +++ b/tika-server/tika-server-standard/src/test/java/org/apache/tika/server/standard/TikaPipesTest.java @@ -59,6 +59,10 @@ import org.apache.tika.pipes.api.ParseMode; import org.apache.tika.pipes.api.emitter.EmitKey; import org.apache.tika.pipes.api.fetcher.FetchKey; +import org.apache.tika.pipes.core.EmitStrategy; +import org.apache.tika.pipes.core.EmitStrategyConfig; +import org.apache.tika.pipes.core.PipesConfig; +import org.apache.tika.pipes.core.PipesParser; import org.apache.tika.pipes.core.extractor.UnpackConfig; import org.apache.tika.pipes.core.fetcher.FetcherManager; import org.apache.tika.pipes.core.serialization.JsonFetchEmitTuple; @@ -93,6 +97,7 @@ public class TikaPipesTest extends CXFTestBase { private FetcherManager fetcherManager; private PipesResource pipesResource; + private PipesParser pipesParser; @Override @BeforeAll @@ -134,7 +139,13 @@ public void setUpEachTest() throws Exception { protected void setUpResources(JAXRSServerFactoryBean sf) { List rCoreProviders = new ArrayList<>(); try { - pipesResource = new PipesResource(tikaConfigPath); + // Mirrors what PipesResource used to build internally, back when it + // constructed its own parser instead of sharing one. + TikaJsonConfig tikaJsonConfig = TikaJsonConfig.load(tikaConfigPath); + PipesConfig pipesConfig = PipesConfig.load(tikaJsonConfig); + pipesConfig.setEmitStrategy(new EmitStrategyConfig(EmitStrategy.EMIT_ALL)); + pipesParser = PipesParser.load(tikaJsonConfig, pipesConfig, tikaConfigPath); + pipesResource = new PipesResource(pipesParser, false); rCoreProviders.add(new SingletonResourceProvider(pipesResource)); } catch (IOException | TikaConfigException e) { throw new RuntimeException(e); @@ -145,10 +156,11 @@ protected void setUpResources(JAXRSServerFactoryBean sf) { @Override @AfterAll public void tearDown() throws Exception { - if (pipesResource != null) { - pipesResource.close(); - pipesResource = null; + if (pipesParser != null) { + pipesParser.close(); + pipesParser = null; } + pipesResource = null; super.tearDown(); if (tmpWorkingDir != null) { FileUtils.deleteDirectory(tmpWorkingDir.toFile());