The GitHub Actions job "Scalafmt" on pekko-http.git/parse-error-line-escaping has failed. Run started by GitHub user pjfanning (triggered by pjfanning).
Head commit for run: 1e23bbc9dd23dc11c3cb8b2272946be5b20129ec / PJ Fanning <[email protected]> Escape control characters in parse-error details and document the raw request target Motivation: `UriParser.fail` and `HeaderParser.parseError` build the `detail` of their `ErrorInfo` from parboiled2's `formatErrorLine`, which reproduces the failing input line verbatim under a caret. That input is the raw request target or header value as the peer sent it, and the shipped `error-logging-verbosity = full` writes the detail to the log at warning level, so a client could put a CR, ESC or NUL into a request target and have it land raw in the log: a forged log line, or a terminal control sequence for whoever tails it. The summary was already safe, because parboiled2 escapes the offending character there; only the line was not. #1246 handed the same raw request target to custom `ParsingErrorHandler`s through `IllegalRequestContext`. Its scaladoc said the value must be escaped, but `toString` did not, the `error-handler` comment in `reference.conf` still said the handler had no access to the request, and no documentation page mentioned the handler at all. Modification: Add `ParseErrorLine.render`, which renders the failing line the way `formatErrorLine` does but escapes control characters as `\t`, `\r`, `\n` and `\uXXXX`, keeping the caret aligned under the escaped text. The EOI sentinel U+FFFF is deliberately left alone, since `HeaderParser` appends it to every value and strips it from the error afterwards. Use it in both parsers. Escape the raw request target in `IllegalRequestContext.toString` so that logging the context is safe, and say so in the scaladoc; add the warning to the Java accessor. Bring the `error-handler` comment up to date and note at `error-logging-verbosity` what `full` logs. Add a "Requests that fail to parse" section to the low-level server docs describing the handler, the context and the escaping requirement. Result: A parse error's detail carries no raw control characters, so a client can no longer inject into the log through a request target or header value. The raw request target is documented as attacker-controlled everywhere a user meets it, and the one rendering the library provides is escaped. Tests: - New `ParseErrorLineSpec` covers caret placement, end-of-input, escaping and caret alignment past an escaped character. - `UriSpec`, `HeaderSpec` and `RequestParserSpec` each gain a case sending an ESC or NUL and asserting the escaped detail, and the request-parser case asserts the escaped `toString`. All three fail with the change to the parsers and `toString` reverted. - sbt "http-core/testOnly ...ParseErrorLineSpec ...UriSpec ...HeaderSpec ...RequestParserCRLFSpec ...RequestParserLFSpec" - 197 tests pass. - sbt "http-core/mimaReportBinaryIssues" - pass. - sbt "docs/paradox" - pass; the new apidoc links resolve. References: Refs #1246, #1245 Report URL: https://github.com/apache/pekko-http/actions/runs/34590598377 With regards, GitHub Actions via GitBox --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
