Escape CR and LF in WARC metadata records and resource Content-Type

[abhinav-phi]

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; since the keys are fixed configuration, this check runs once in the constructor rather than for every record (following review feedback).
• 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 — invalid keys are dropped once in the constructor; format() sanitises every value 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 #1998 WARC writer: WARC-Protocol header to follow WARC field proposals #2034, which reworks WARC-Protocol writing. (I checked #1998 WARC writer: WARC-Protocol header to follow WARC field proposals #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.)

[dpol1]

Went looking for other header sinks with the same pattern in the three record formats and came up empty, so this seems to cover them all. Space over dropping the value feels right for a format with no escaping.

Small thought, non-blocking: the invalid-key warning fires per record, and keys are fixed config - would validating once in the constructor be enough?

[abhinav-phi]

Thanks for the review and for double-checking the other header sinks — good to have that confirmed.

Fair point on the per-record warning: the configured keys are indeed fixed topology configuration. Done in 311f3ab — MetadataRecordFormat now filters invalid keys once in the constructor (with a single warning listing... well, warning per invalid key) and keeps an immutable list of the valid keys, so format() no longer re-validates per record.

[dpol1]

👍

[jnioche]

thanks @abhinav-phi