Skip to content

100 Continue is not handled correctly in all cases #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MFranceschi6 opened this issue Oct 22, 2021 · 5 comments · Fixed by apple/swift-nio#1984 or #469
Closed

100 Continue is not handled correctly in all cases #461

MFranceschi6 opened this issue Oct 22, 2021 · 5 comments · Fixed by apple/swift-nio#1984 or #469

Comments

@MFranceschi6
Copy link

Issue related to:
AsyncHttpClient Version 1.6.0 onward

Context:
We interact with a server that always responds first with a 100 continue status code. Before version 1.6 we were able to handle this case with something like this:

extension EventLoopFuture where Value == HTTPClient.Response {
func handleContinueStatusCode(client: HTTPClient, request: HTTPClient.Request) -> EventLoopFuture<HTTPClient.Response> {

        let resp = self.eventLoop.makePromise(of: HTTPClient.Response.self)
        
        self.whenSuccess { response in
            if response.status == .continue {
                let next = client.execute(request.method, url: "\(request.url)")
                        .handleContinuationStatusCode(client: client, request: request)
                resp.completeWith(next)
            }
            else {
                resp.succeed(response)
            }
        }
...
}

After version 1.6 the client started to throw the new httpEndReceivedAfterHeadWith1xx error thown here

case .running(_, .waitingForHead):
// If we receive a http response header with a status code of 1xx, we ignore the header
// except for 101, which we consume.
// If the remote closes the connection after sending a 1xx (not 101) response head, we
// will receive a response end from the parser. We need to protect against this case.
let error = HTTPClientError.httpEndReceivedAfterHeadWith1xx
self.state = .failed(error)
return .failRequest(error, .close)

Now we are exactly in this situation and we changed the code above with:

extension EventLoopFuture where Value == HTTPClient.Response {
func handleContinueStatusCode(client: HTTPClient, request: HTTPClient.Request) -> EventLoopFuture<HTTPClient.Response> {

        let resp = self.eventLoop.makePromise(of: HTTPClient.Response.self)
        
        self.whenFailure { error in
            if let error =  error as? HTTPClientError, error == .httpEndReceivedAfterHeadWith1xx {
                let next = client.execute(request.method, url: "\(request.url)")
                        .handleContinuationStatusCode(client: client, request: request)
                resp.completeWith(next)
            }
            else {
                resp.fail(error)
            }
        }
...
}

But this doesn't work anymore.
As a quirk fix for our case, we found that modifying the handling of the error at line 538 with
return .failRequest(error, .none)

The handleContinueStatusCode start to work again. But we don't know what other impacts this change has.
For an additional Insight I add here the curl of the request that fails:

curl --location --request POST 'https://sandbox.cedacri.it/psd2/v1.2/bg/03105/v1/consents' \
--header 'x-request-id: 3c528418-c8dc-4627-95e2-3e05e93411d8' \
--header 'psu-ip-address: 127.0.0.1' \
--header 'tpp-redirect-uri: http://localhost' \
--header 'Content-Type: application/json' \
--data-raw '{"validUntil":"1970-01-01","access":{"availableAccounts":"allAccounts"},"frequencyPerDay":1,"recurringIndicator":false,"combinedServiceIndicator":false}' -v 

Swift version:
Swift version 5.5 (swift-5.5-RELEASE)
Target: x86_64-unknown-linux-gnu

@fabianfett
Copy link
Member

From what I can see this is not actually an issue of AHC, but of SwiftNIO. We should keep the issue open here though.
apple/swift-nio#1330
apple/swift-nio#1422

@fabianfett
Copy link
Member

This issue hasn't been fully fixed yet.

@fabianfett
Copy link
Member

@MFranceschi6 We will release a new version of AHC shortly. Once you have updated, you will not need any special handling for your request anymore. It should just work, without any retries whatsoever.

Thanks for reporting the issue. If you run into any further issues please reach out!

@fabianfett
Copy link
Member

Released in 1.6.4

@MFranceschi6
Copy link
Author

Thank you very much I will let you know ASAP if the release has fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants