[PR] Stop parsing chunks once the chunk count limit is reached [pekko-http]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning opened a new pull request, #1220:
URL: https://github.com/apache/pekko-http/pull/1220

   ### Motivation
   The `max-chunk-count` check in `HttpMessageParser.parseChunkBody` is missing its `else`:
   
   ```scala
   if (chunkSize > 0) {
     if (chunkCount >= settings.maxChunkCount)
       failEntityStream(s"HTTP chunk count exceeds the configured limit of ${settings.maxChunkCount} chunks")
     val chunkBodyEnd = cursor + chunkSize
     ...  // emits the chunk and trampolines into the next one regardless
   ```
   
   `failEntityStream` does not throw — it emits an `EntityStreamError`, sets `terminated = true` and returns a `StateResult` (`done()` is `null`). Its result is dropped here, so after the error the parser emits the current chunk, continues with the next one, and emits a further error for every remaining chunk in the buffer, i.e. it keeps producing output after it has terminated itself. Every other limit in this parser is written as `if (cond) failEntityStream(...) else <continue>`.
   
   This is not a bypass: the error is emitted before the chunk, so a consumer that stops at the first error still fails the entity stream, which is presumably why it went unnoticed — there is no test for `max-chunk-count` in the repo. But the parser does work it should not, and it emits the whole rest of the body behind the error.
   
   ### Modification
   Move the rest of `parseChunkBody` into the `else` branch of the check.
   
   ### Result
   Nothing is emitted after the chunk count error and the parser stops parsing the remainder of the body.
   
   ### Tests
   - `sbt "http-core/testOnly org.apache.pekko.http.impl.engine.parsing.RequestParserCRLFSpec org.apache.pekko.http.impl.engine.parsing.RequestParserLFSpec"` - pass, with a new test that collects the raw parser output for a body of three chunks with `max-chunk-count = 2`. It asserts that only the chunks `"a"` and `"b"` are emitted and that the error is the last output. Without the change the parser emits `"a", "b", "c", ""`, so the test fails. It deliberately reads the parser output directly, because the existing harness cancels the rest of the stream at the first error and therefore cannot see the difference.
   - `sbt http-core/test` - pass (1216 tests)
   - `sbt http-core/mimaReportBinaryIssues` - pass
   - `sbt http-core/scalafmt http-core/Test/scalafmt` - clean
   
   ### References
   None - makes the max-chunk-count limit stop parsing
   


-- 
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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org

Re: [PR] Stop parsing chunks once the chunk count limit is reached [pekko-http]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning merged PR #1220:
URL: https://github.com/apache/pekko-http/pull/1220


-- 
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: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org