net.mbedtls: track Content-Length/chunked framing in vschannel's one-shot HTTP/1.1 client#27714
Conversation
…shot HTTP/1.1 client https_make_request (thirdparty/vschannel/vschannel.c), the core of the one-shot HTTP/1.1 client used both when HTTP/2 is disabled and as the ALPN-negotiation-failed fallback, previously read the response until the peer closed the connection or sent a TLS close_notify -- it never parsed Content-Length or chunked Transfer-Encoding to know the response was already complete. HTTP/1.1 keep-alive is the default, so a compliant server that leaves the connection open after responding stalled the client for as long as the peer chose to keep the socket open (observed ~41s against a realistic test server, tracking its own idle-read timeout). Adds a small set of static helpers that inspect the response bytes accumulated so far for a complete Content-Length-framed or fully-received chunked body, and stops the read loop as soon as one is found -- deliberately conservative throughout: anything it cannot confidently parse (headers not yet complete, no length-framing header, a malformed length) reports incomplete and falls back to the existing read-until-close behavior, which remains correct in every case. Guards the length accumulators against overflow so a malformed/absurd length can never wrap into a small value and cause a premature (truncating) stop. Regression test reproduces the exact reported symptom (a keep-alive loopback peer) and was verified to fail on the pre-fix code (~3.1s, waiting for the peer's own close) before confirming it passes fast on the fix. Fixes vlang#27705. Co-Authored-By: WOZCODE <contact@withwoz.com>
…ake_request Implementation half of the previous commit -- vschannel.c was left unstaged by a staging mistake and only the regression test landed. This is the actual fix: https_make_request's read loop now recognizes a complete Content-Length-framed or fully-received chunked response and stops reading without waiting for the peer to close the connection (see the previous commit's message for the full rationale). No content differs from what /vreview already covered for this diff. Fixes vlang#27705. Co-Authored-By: WOZCODE <contact@withwoz.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1b14edda6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Content-Length. Only "chunked" is understood here; anything else | ||
| // falls back to the existing (always-correct) read-until-close | ||
| // behavior. | ||
| if(http_ci_contains(te, te_len, "chunked", 7)) { |
There was a problem hiding this comment.
When a response has a Transfer-Encoding value that merely contains the substring chunked (for example an extension token like xchunked or a parameter containing that word), this branch treats the body as chunked and can stop reading before the peer closes. That contradicts the fallback intended for unknown transfer codings and can hand parse_response a truncated raw body, since the V parser only recognizes real comma-separated chunked tokens. Parse the header as transfer-coding tokens and only use chunk framing for an actual chunked coding.
Useful? React with 👍 / 👎.
| return FALSE; // no length-framing header: must read until the peer closes | ||
| } | ||
| long content_length = 0; | ||
| if(!http_parse_decimal(cl, cl_len, &content_length)) { |
There was a problem hiding this comment.
For a keep-alive response with a valid header such as Content-Length: 27 or Content-Length:\t27\t, http_find_header leaves the trailing spaces/tabs in cl_len, so this strict decimal parse fails and the read loop falls back to waiting for the server to close. That preserves the vschannel hang for length-framed responses emitted with trailing optional whitespace; trim trailing OWS before calling http_parse_decimal.
Useful? React with 👍 / 👎.
2 participants
Summary
https_make_request(thirdparty/vschannel/vschannel.c), the core ofvschannel's one-shot HTTP/1.1 client (used both when HTTP/2 is disabled and
as the ALPN-negotiation-failed fallback), previously read the response
until the peer closed the connection or sent a TLS close_notify -- it never
parsed
Content-Lengthor chunkedTransfer-Encodingto know the responsewas already complete.
HTTP/1.1 keep-alive is the default: a compliant server is free to leave the
connection open after responding. Against such a server, the client
blocked in
recv()until the peer's own idle-read timeout eventuallyforced it to give up and close the socket -- there was no bound on the
client's own side at all. This was found while adding HTTP/2 connection
pooling for the Windows SChannel backend (#27702): a test exercising the
ALPN-negotiation-failed fallback against a realistic keep-alive loopback
server took ~41 seconds instead of being near-instant.
https_make_requestthat inspectthe response bytes accumulated so far for a complete Content-Length-framed
or fully-received chunked body, and stop the read loop as soon as one is
found.
(headers not yet complete, no length-framing header present, a malformed
length value) reports "incomplete" and falls back to the existing
read-until-close/read-until-context-expired behavior, which remains
correct in every case -- these helpers only ever add an earlier stopping
point, never a different one.
longis 32-biteven in 64-bit Windows builds) so a malformed/absurd length can never wrap
into a small value and cause a premature, truncating stop -- caught and
fixed during self-review before this was pushed.
Fixes #27705.
Test plan
vschannel_h1_windows_test.v) reproduces theexact reported symptom against a loopback keep-alive server, and was
verified with a Phase-R-style stash/reproduce cycle to fail on the
pre-fix code (~3.1s, waiting for the peer's own close) before
confirming it passes fast (well under the 1.5s bound) on the fix
./vnew test vlib/net/http/-- 25 passed, 2 skipped (unrelated,network-dependent), on this standalone branch
/vreview(full A-G angles + mechanical detectors) -- one finding(the integer-overflow guard above), fixed before push
Co-Authored-By: WOZCODE contact@withwoz.com