net.mbedtls: track Content-Length/chunked framing in vschannel's one-shot HTTP/1.1 client#27714
Open
quaesitor-scientiam wants to merge 3 commits into
Open
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>
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
1 participant
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.
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