Repository navigation
Conversation
The EST test server carried its own copy of the chunk-size and framing parser, and wolfSSL#26 hardened only the client copy. The server copy rejected a zero-padded chunk size longer than eight digits and whitespace before a chunk-ext, both legal per RFC 9112 section 7.1.1, and accepted any trailer section without looking at it. Its framing scan also reported malformed input as complete and left the rejection to the decode pass. http.c now exports its framing scan and decoder, and the new wolfcert_server_read_chunked() receives and decodes a chunked request body for both servers. The EST server drops its own parser. The SCEP server, which only read Content-Length bodies, now accepts a chunked PKIOperation POST as well. The receive loop clamps its last read to the raw cap instead of refusing a body that still fits, and grows its buffer only when it is full. The framing scan now resumes where the previous call stopped instead of starting over at offset 0 after every read. A body sent in many small pieces used to cost time quadratic in its size, about 12 s for 240 KB fed one byte at a time; it is now linear, so a 1 MiB body takes a few milliseconds. The client's chunked response reader shares the scan and gets the same fix. Both servers now take a Transfer-Encoding of exactly "chunked" and answer any other coding, or a second Transfer-Encoding header, with 400; the EST server used to accept any value starting with "chunked". RFC 9112 section 6.3 requires the 400 when chunked is not the final coding, and section 6.1 would allow 501 for an unknown coding before it. A request framed by both Transfer-Encoding and Content-Length, or an HTTP/1.0 request carrying Transfer-Encoding, is decoded as chunked and the connection closes after the response (RFC 9112 section 6.1). The SCEP server also answers whitespace before a header's colon with 400 (RFC 9112 section 5.1), as the EST server does, so it no longer ignores "Transfer-Encoding : chunked". test_est_chunked_robustness checks a table of framings and one of Transfer-Encoding values; against the old server parser the padded size, both chunk-ext whitespace forms and both bad trailers fail. test_scep_roundtrip posts a pkiMessage chunked, checks a chunked request on a kept-alive connection, bad framing, other codings and the conflicting-framing and HTTP/1.0 closes, and test_http drives the reader's decoded and raw caps and feeds the framing scan one byte at a time.
There was a problem hiding this comment.
🟢 Approval recommended
The shared implementation is bounded, consistently integrated, and covered by focused unit and integration tests.
0 open findings
What changed in this PR
Shares hardened, resumable chunked-body parsing across the HTTP client and EST/SCEP test servers.
Changes:
- Extracts shared chunk framing, decoding, and server-reading helpers.
- Adds strict Transfer-Encoding handling and connection-closing behavior.
- Expands unit and integration coverage for framing, limits, and keep-alive.
| File | Description |
|---|---|
src/http.c |
Implements resumable shared chunk parsing and decoding. |
src/internal.h |
Declares shared internal chunk helpers and scan state. |
src/server.c |
Adds shared server-side chunk reading and coding validation. |
src/est/est_server.c |
Replaces duplicated parser and hardens framing handling. |
src/scep/scep_server.c |
Adds chunked request support and header validation. |
tests/unit/test_http.c |
Tests resumable scanning and body limits. |
tests/integration/test_est_chunked_robustness.c |
Tests EST framing and coding behavior. |
tests/integration/test_scep_roundtrip.c |
Tests SCEP chunked POSTs, errors, and keep-alive. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
Author
|
@wolfSSL-Fenrir-bot review balanced |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #53
Scan targets checked: wolfcert-src, wolfcert-bugs
Coverage: 3 of 5 in-scope changed file(s) opened by the reviewer; not opened: tests/integration/test_est_chunked_robustness.c, tests/unit/test_http.c
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The EST test server kept its own copy of the chunked parser, which #26 never hardened. It rejected legal input (zero-padded sizes over eight digits, whitespace before a chunk-ext), accepted any trailer section, and reported malformed framing as complete.
http.cexports its framing scan and decoder, andwolfcert_server_read_chunked()serves both test servers. The SCEP server now accepts a chunked PKIOperation POST.chunkedcoding and answer any other coding, or a repeated Transfer-Encoding header, with 400.Transfer-Encoding : chunkedis no longer ignored.Tests:
test_est_chunked_robustness(framing and coding tables; the old server parser fails the padded size, both chunk-ext whitespace forms and both bad trailers),test_scep_roundtrip(chunked POST, keep-alive, bad framing, the closing cases) andtest_http(reader caps, byte-at-a-time scan).Outside the test servers, only the client changes: its chunked response reader now uses the resumable scan.