Repository navigation
Fix the redirect-with-body test endpoint announcing a length it does not send - #16
Merged
Merged
Conversation
…not send /redirect/302-with-body announces a Content-Length of `size`, but the test server wrapped every body into the JSON description of the request, which is longer. The bytes after the announced length were taken by the client for the start of the next response, failed to parse, and closed the connection, so the next request of a redirect chain failed with `I/O on closed channel` in 15 to 20 percent of the tries. It was not a bug of the client: the same sequence of requests against the fixed endpoint did not fail in 2400 tries. Add a way to send a body as it is, use it for this endpoint, and add a test that follows redirects with bodies that are not read, which fails when the endpoint announces a length that it does not send. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.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.
What
Fixes the test endpoint
/redirect/302-with-body, which announced a length it did not send, and adds a test that would have caught it.This is test-only. No production code changes.
Why
The endpoint announces
Content-Length: <size>, butHTTPBinwraps every response body into the JSON description of the request (RequestInfo), which is longer thansize. The bytes after the announced length are taken by the client for the start of the next response. The parser fails (invalid constant string), the channel is closed, and the next request of a redirect chain gets that connection and fails withI/O on closed channel.With the async API and
.follow, 300 sequential requests to/redirect/302-with-body?size=4096failed 41, 65, 44 and 42 times onrelease. With the endpoint fixed, 8 rounds of 300 requests had no failure.I first took this for a race in the client, and I wrote that down in #15. That was wrong: it was the test server. The failures also showed up on upstream
mainand onfeature/redirect-custom-handlerbecause I had copied the same endpoint into the test server there to compare.Changes
HTTPResponseBuilder.sendsBodyVerbatimmakes the test server send the body as it is. The endpoint uses it.testFollowingRedirectsWithBodiesThatAreNotReadDoesNotFailTheNextRequestfollows 150 redirects whose body is not read. It passes 5 out of 5 runs with the fix and fails 5 out of 5 without it.Testing
testConnectTimeoutinHTTPClientTestsandAsyncAwaitEndToEndTests, which fail the same on a cleanreleasehere.-Xswiftc -warnings-as-errors --explicit-target-dependency-import-check error, and the redirect tests.swift format lint --strictis clean.