abhinav-phi opened a new pull request, #2109: URL: https://github.com/apache/stormcrawler/pull/2109
Fixes #2105. ## What happens `MetadataRecordFormat.format()` writes one line per metadata value as `key: value` into the `application/warc-fields` payload of the WARC metadata record (`external/warc/src/main/java/org/apache/stormcrawler/warc/MetadataRecordFormat.java`, config key `warc.metadata.keys`). Neither the key nor the value was checked for CR or LF, so a value containing CR LF became two or more field lines in the payload: ``` feed.description: some text hopsFromSeed: 1 ``` The extra lines look exactly like fields written by the crawler. Framing stayed valid because `Content-Length` is computed from the finished payload, so no record was split — and WARC readers had no way to tell that `hopsFromSeed: 1` (or a fabricated `via`, etc.) was contributed by the captured content rather than written by StormCrawler. Values that reach this sink often originate from the crawled content: `FeedParserBolt` sets `feed.description` from the feed item without trimming, and `parse.*` values produced by the XPath, LDJson and Tika filters keep their newlines. As soon as an operator lists such a key in `warc.metadata.keys`, a crawled page or feed can add field lines to its own metadata record. The effect is limited — nothing in this repository reads `warc-fields` back, and metadata records are opt-in — but a downstream tool that ranks or filters captures by those fields can be misled. The same lack of escaping applied to the resource record `Content-Type` in `WARCRecordFormat.format()` (`external/warc/src/main/java/org/apache/stormcrawler/warc/WARCRecordFormat.java`), where the server-supplied content type was appended verbatim into the WARC header block itself — worse in principle, because it could forge arbitrary WARC header lines (`WARC-Truncated`, `WARC-Protocol`, …), even though the sibling formats happen not to feed such values into it today. ## The fix `WARCRecordFormat` gains two shared helpers (used by its subclass `MetadataRecordFormat` as well): - `isValidWarcFieldName(String)` — a WARC field name must consist of printable ASCII characters without a colon (RFC 5322 § 2.2, which the WARC specification references for named fields). `MetadataRecordFormat` drops a configured key with a warning if it is not a valid field name, instead of writing a malformed line. - `sanitizeWarcFieldValue(String)` — replaces CR and LF characters by spaces so that a value cannot end its field line early and forge additional lines. Folding long values via continuation lines (as the WARC spec allows) was considered, but replacement keeps each value on a single line and avoids re-introducing interpretation questions for values that already contain line breaks. Concretely: 1. `MetadataRecordFormat.format()` — the key is validated and the value sanitised before the field line is appended. 2. `WARCRecordFormat.format()` — the server-controlled `Content-Type` of resource records is sanitised before it is appended to the WARC header block. In addition, `MetadataRecordFormat` logged under `WARCRequestRecordFormat.class` (copy/paste); the logger now uses the correct class. ### Deliberately out of scope - The WARC header values written from protocol metadata (`WARC-Protocol`, `WARC-Truncated`) are produced internally by the protocol implementations, not from captured content; touching them would also collide with the pending #2034, which reworks `WARC-Protocol` writing. (I checked #2034 for duplication as suggested — it is about WARC-Protocol/WARC-Cipher-Suite conformance, not escaping; the only overlap is that both PRs touch `WARCRecordFormat.format()`.) - `generateWARCInfo()` writes operator-configured fields (`WARCHdfsBolt.withHeader()`), which are topology settings rather than crawled content. ## Tests New `MetadataRecordFormatCRLFTest` (the reproduction from the issue, extended): - a metadata value containing CR LF no longer introduces a new `warc-fields` line; the record parsed with jwarc contains only the field the crawler wrote, and the sanitised value stays on its own field line; - bare CR and bare LF are neutralised the same way; - keys that are not valid field names (colon, space) are dropped. `WARCRecordFormatTest#testWarcResourceRecordContentTypeCRLFInjection`: a server-supplied `Content-Type` containing CR LF cannot forge WARC header lines; jwarc sees exactly one `Content-Type` header and no forged `WARC-Truncated`. All 17 tests of the warc module pass, together with the `editorconfig`, `git-code-format:validate-code-format` and `-Prat` checks. (`WARCHdfsBoltTest` cannot run locally on Windows — pre-existing `HADOOP_HOME`/winutils environment limitation, unrelated to this change; it runs on Linux CI as before.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
