feat: implement MIME type whitelist filtering in ParserBolt for Tika-… - #2116
feat: implement MIME type whitelist filtering in ParserBolt for Tika-…#2116Riddhish1 wants to merge 3 commits into
Conversation
|
You missed this: https://github.com/apache/stormcrawler/blame/e1d54a6572438a5518d772cb88ac51e8dada57fb/README.md#L43 - Format the code pls |
|
Fixed the formatting. Thanks for catching that!
|
dpol1
left a comment
There was a problem hiding this comment.
This closes the scenario from #2104 — server claims a whitelisted type, bytes say otherwise, doc rejected — and the new test pins exactly that. Rejection path and ack behaviour unchanged, and detection only runs when a whitelist is configured, so no cost for everyone else.
Two questions inline. One observation: undetectable content (application/octet-stream) is now rejected when a whitelist is set, where a whitelisted header used to let it through — stricter is right for this fix, but maybe worth a line in the PR description so the behaviour change is on record.
| detectionMd.set(org.apache.tika.metadata.Metadata.CONTENT_TYPE, httpCTHint); | ||
| } | ||
| try { | ||
| mimeType = tika.detect(new ByteArrayInputStream(content), detectionMd); |
There was a problem hiding this comment.
The parse-time detection further down also gets the filename via RESOURCE_NAME_KEY, this one doesn't — so the two can still disagree (I could reproduce it with plain-text bytes and a .html URL: text/plain here, text/html at dispatch). Passing the same filename hint into detectionMd would make the check genuinely match what the parser dispatches on.
There was a problem hiding this comment.
updated detectionMd to pass TikaCoreProperties.RESOURCE_NAME_KEY from the URL as well, and added a test case verifying that ambiguous content (like plain text with a .html URL) resolves consistently
| boolean mt_match = false; | ||
| // see if a mimetype was guessed in JSOUPBolt | ||
| // see if a mimetype was detected already (e.g. by JSoupParserBolt) | ||
| String mimeType = metadata.getFirstValue("parse.Content-Type"); |
There was a problem hiding this comment.
Genuine question: is a pre-existing parse.Content-Type trusted by design? Coming from JSoupParserBolt's own byte detection that seems fine, but any other upstream writing a server-influenced value there skips the new check entirely. If it's intentional, worth a comment saying so.
There was a problem hiding this comment.
JSoupParserBolt.guessMimeType() does byte detection too (header is just a hint) so basically the value is trustworthy in the standard topology, a custom upstream writing a server copied value there would bypass it worth a comment will add it
…gic and corresponding unit test
Fix
parser.mimetype.whitelistevaluating against the HTTP response header instead of the detected content typeThe problem
ParserBolt.execute()checksparser.mimetype.whitelistbefore parsing. Whenparse.Content-Typeis present in the metadata (written by
JSoupParserBoltwhendetect.mimetypeis true), it is usedfor the whitelist check and also drives Tika's
AutoDetectParser— the two are in agreement.When
parse.Content-Typeis absent — becausedetect.mimetypeis false, or because the topologyfeeds
ParserBoltdirectly withoutJSoupParserBoltupstream — the code fell back to theContent-Typeresponse header supplied by the fetched server:The whitelist was then evaluated against this server-declared value, while Tika's
AutoDetectParserdispatched on the raw content bytes. A server can claim any MIME type in itsresponse header, so the two sources can disagree. In the worst case a server reports a whitelisted
type (e.g.
application/vnd.openxmlformats-officedocument.wordprocessingml.document) whileserving an entirely different payload (e.g. HTML). The whitelist gate opened, and Tika parsed
whatever the bytes actually were.
The practical impact is limited in the common archetype setup because
JSoupParserBoltruns aheadof the Tika bolt with detection enabled, so
parse.Content-Typeis almost always present.The gap opens in two real scenarios:
detect.mimetype: falsein the crawler configuration.FetcherBoltdirectly toParserBoltwithout a JSoup stage.In both cases the whitelist was not doing the job its name and the archetype documentation imply:
controlling which document types this bolt parses.
What this PR changes
ParserBolt.execute()— detect from bytes whenparse.Content-Typeis absentWhen the metadata key
parse.Content-Typeis missing, the bolt now callstika.detect()on thecontent bytes before evaluating the whitelist, rather than trusting the server header:
The HTTP response header is still passed to Tika as a hint. Content bytes take precedence when
the two disagree. The filename extracted from the URL is also passed as a hint — without it,
ambiguous bytes (e.g. plain text at a
.htmlURL) can resolve differently in the whitelist checkvs. at parse dispatch time. The result is written back into
parse.Content-Typeso both thewhitelist gate and Tika's
AutoDetectParserare bound to the same type.Behaviour in the common case is unchanged. When
parse.Content-Typeis already present (thenormal path with
JSoupParserBoltupstream), the new block is not entered.Behaviour changes worth noting in release notes
Type mismatch rejection. Documents whose server-declared
Content-Typematched the whitelistbut whose bytes are detected as a different type will now be rejected where they were
previously parsed. This is the correct outcome — the whitelist was not enforcing what operators
expected — but operators who relied on the previous behaviour should be aware.
Undetectable content (
application/octet-stream) rejection. Previously, truly undetectablebinary content (no magic bytes Tika can match) would pass the whitelist if the server header
claimed a whitelisted type, because the whitelist checked that header. Now the whitelist checks
the byte-detected type, which for undetectable content is
application/octet-stream. Unless thewhitelist explicitly includes
application/octet-stream, such documents will be rejected. This isthe stricter and more correct behaviour, but operators should be aware the rejection boundary has
changed.
Restricting the parser set in
tika-config.xmlis a complementary defence in depth: it boundswhich parsers can be selected at all, regardless of what the whitelist or detection step resolves.
Tests
ParserBoltWhitelistDetectionTest(new,external/tika)whitelistAppliesToTheDetectedType— reproduces the original bug:application/.+word.*(the pattern shipped by the archetypes).Content-Typeheader:application/vnd.openxmlformats-officedocument.wordprocessingml.document.<html><body><p>not a word document</p></body></html>.parse.Content-Typein metadata (simulates a topology without JSoupParserBolt upstream).Before this fix the bolt parsed the HTML and emitted a document. After this fix the bolt detects
text/htmlfrom the bytes, the whitelist does not match, and the tuple is emitted on the statusstream with
Status.ERROR.whitelistUsesPreexistingParsedContentType— sanity check for the unchanged common path:parse.Content-Typeis set totext/html; charset=UTF-8(as JSoupParserBolt would write it).text/html.*.filenameHintInfluencesDetection— pins theRESOURCE_NAME_KEYfix:https://example.org/page.html(.htmlextension).parse.Content-Type, noContent-Typeheader.text/html.*.Without the filename hint the bytes alone resolve to
text/plainand the whitelist rejects thedocument. With the hint, Tika resolves
text/html, the whitelist accepts it, and the detectionmatches what the parser would dispatch on.
Verification
Detected types printed during the test run:
The bolt rejects the mismatched document and the filename hint correctly resolves ambiguous content.