bryancall opened a new issue, #13624:
URL: https://github.com/apache/trafficserver/issues/13624
Follow-up to #12195 and #13595.
#13595 stops the raw input line from being preserved when the byte
immediately before the terminating CRLF is a carriage return. That covers the
line reported in #12195, but not the general case.
`mime_parser_parse` trims the field value with
`field_value.rtrim_if(&ParseRules::is_wslfcr)`, which strips SP and HT as well
as CR and LF. So a bare CR only has to be separated from the CRLF by one space
or tab to end up back in the raw-print pad and be forwarded unchanged.
Built on master with #13595 applied, output captured from `HTTPHdr::print`:
| request header line | serialized output |
|---|---|
| `Extra-CRs: \r\r\r\r\n` | `Extra-CRs: \r\n` (fixed by #13595) |
| `Extra-CRs: \r \r\n` | `Extra-CRs: \r \r\n` |
| `Extra-CRs: \r\r \r\n` | `Extra-CRs: \r\r \r\n` |
| `Extra-CRs: bar\r \r\n` | `Extra-CRs: bar\r \r\n` |
| `Extra-CRs: \r\t\r\n` | `Extra-CRs: \r\t\r\n` |
RFC 9112 section 2.2 says a recipient of a bare CR must either consider the
element invalid or replace each bare CR with SP before processing or forwarding
the message. The last four rows do neither.
A positional check will keep missing spellings of this. Making it structural
works and keeps the fast path: once the value is trimmed, require that
everything between the end of the value and the end of the line is optional
whitespace followed by exactly one CRLF.
```cpp
// find value first
field_value.ltrim_if(&ParseRules::is_ws);
field_value.rtrim_if(&ParseRules::is_wslfcr);
if (raw_print_field) {
// Raw printing copies the original input bytes instead of
re-serializing, so it would
// replay this line ending verbatim. Everything between the end of the
trimmed value and
// the end of the line must be optional whitespace followed by exactly
one CRLF; a bare
// CR in there is malformed (RFC 9112 section 2.2) and must not be
forwarded.
TextView tail{field_value.data() + field_value.size(), parsed.data() +
parsed.size()};
if (tail.suffix(2) != "\r\n" ||
tail.remove_suffix(2).find_first_of("\r\n") != TextView::npos) {
raw_print_field = false;
}
}
```
With that in place all five rows normalize to a single CRLF and
`test_proxy_hdrs` stays green. Well-formed fields keep the raw fast path: `X: a
\r\n`, `X:\ta\r\n` and a folded line are all still emitted byte for byte. The
`else if` on the line ending goes away, as does the need for the `size() > 2`
guard, since `suffix()` clamps.
Three smaller items for the same change:
1. The unit test fixture added in #13595 produces a raw-print pad of exactly
7, and `mime_field_name_value_set` only honors raw printing when the pad is 7
or less. One more CR and the pre-existing pad gate suppresses raw printing on
its own, so the test would pass without any fix at all. It needs a comment
saying so, plus a case that is not on the boundary.
2. Only the request direction is covered. The same function serves
responses, so a `parse_resp` case is cheap.
3. `tests/gold_tests/headers/field_name_space.test.py` covers the sibling
whitespace-before-colon case end to end. Because
`MIMEFieldBlockImpl::move_strings` clears the raw flag on any string-heap
relocation, whether these bytes reach the origin depends on heap state a unit
test does not exercise, so a gold test with a raw-socket client would pin the
actual wire contract.
This reproduces on 10.2.x and 10.1.x as well.
--
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]
