This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4809-stage-2 in repository https://gitbox.apache.org/repos/asf/tika.git
commit 391c548203c22e4d7f5f5e0cb469c43122c2be32 Author: tallison <[email protected]> AuthorDate: Fri Aug 7 12:17:51 2026 -0400 TIKA-4809: Fix content-handler ignore type leaking garbage into TIKA_CONTENT --- .../tika/sax/BasicContentHandlerFactory.java | 20 +++++++++++++-- .../tika/sax/RecursiveParserWrapperHandler.java | 27 +++++++++----------- .../tika/sax/BasicContentHandlerFactoryTest.java | 7 +++--- .../tika/pipes/core/server/ParseHandler.java | 9 ++++++- .../apache/tika/pipes/core/PipesClientTest.java | 29 ++++++++++++++++++++++ 5 files changed, 71 insertions(+), 21 deletions(-) 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 @@ public class BasicContentHandlerFactory implements StreamingContentHandlerFactor 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 class BasicContentHandlerFactory implements StreamingContentHandlerFactor 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 class BasicContentHandlerFactory implements StreamingContentHandlerFactor 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 java.util.concurrent.atomic.AtomicInteger; 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 class RecursiveParserWrapperHandler extends AbstractRecursiveParserWrappe } 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 class BasicContentHandlerFactoryTest { .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 class BasicContentHandlerFactoryTest { .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/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 @@ class ParseHandler { 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.ParseMode; 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 class PipesClientTest { 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\""); + } }
