Okhttp: 100 (Continue) HTTP response should be handled gracefully regardless of whether it was requested/expected

Created on 25 Sep 2017  Â·  12Comments  Â·  Source: square/okhttp

I'm using OkHttp 3.8.1 on Android to POST to a poorly-behaved server.

Even though the request does not include an "Expect: 100-continue" header, the web server returns a 100 Continue response. Rather than continuing to send the request body (or just ignoring the 100 response since the body had already been sent), then getting the actual (200) response, OkHttp stops there and sends back the 100 Continue response in my okhttp.Callback.

The w3c definition of the 100 status code states:

The client SHOULD continue by sending the remainder of the request or, if the request has already been completed, ignore this response.

and:

A client MUST be prepared to accept one or more 1xx status responses prior to a regular response, even if the client does not expect a 100 Continue status message.

I have verified the correct behavior on three other HTTP clients (iOS, Windows, and curl(1)), who correctly ignore/continue when receiving an unexpected 100 Continue response.

In contrast, it looks like OkHttp only handles the 100 response iff it was explicitly requested via an "Expect: 100-continue" header. From CallServerInterceptor.java:

// If there's a "Expect: 100-continue" header on the request, wait for a "HTTP/1.1 100
// Continue" response before transmitting the request body. If we don't get that, return
// what we did get (such as a 4xx response) without ever transmitting the request body.
if ("100-continue".equalsIgnoreCase(request.header("Expect"))) {
    httpCodec.flushRequest();
    realChain.eventListener().responseHeadersStart(realChain.call());
    responseBuilder = httpCodec.readResponseHeaders(true);
}

Incidentally, the obvious workaround (including "Expect: 100-continue" in the request to trigger OkHttp's logic) didn't work for me because this particular server is further broken in that it complains about a mangled Expect: header. :expressionless: It doesn't appear any other workaround is possible, though I'd love to be proven wrong.

See: https://stackoverflow.com/a/46382361/920849

bug

All 12 comments

The key takeaway is that a server can choose to return 100 when the client doesn't request it, or return a non-100 response when even when the client does request it.

@stephentalley Thanks for the detailed writeup. It sounds like a legitimate bug. Given the rarity of it and it being triggered by a badly behaving server, it may not be at the top of the pile to work on. So if it's really impacting you we would love a PR and I'll happily review quickly.

@stephentalley I think the appropriate spec is now https://tools.ietf.org/html/rfc7231#section-6.2.1.

The client ought to continue sending the request and discard the 100 response.

Since when is "ought" a rfc2119 keyword :(

I have also encountered this problem, and I don't want to add the Expect header because we send requests to a lot of different servers and I don't want to risk breaking working things.

What do you mean by "badly behaving server"? As I understand it, any server may send 100 Continue, and it should be ignored.

Do you think it's possible to write a patch/test without much okhttp experience?

The server is arguably doing the wrong thing by the spec. The spec says the client should be tolerant even if it didn't request this (Postel's law).

I say arguably, because as the spec mentions, ther are clients out there that don't support this and servers will break by implementing.

Note: The Expect header field was added after the original
publication of HTTP/1.1 [RFC2068] as both the means to request an
interim 100 (Continue) response and the general mechanism for
indicating must-understand extensions. However, the extension
mechanism has not been used by clients and the must-understand
requirements have not been implemented by many servers, rendering
the extension mechanism useless. This specification has removed
the extension mechanism in order to simplify the definition and
processing of 100-continue.

But it's now a bug based on the latest spec, so OkHttp should support this even when not sending Expect.

If you need this fixed ASAP, then definitely read through the code, try making a failing test and PR, ask questions here and I'll (and others) help if you get stuck.

We are also encountering this issue in our Android app with one specific server (see opacapp/opacclient#497) since we switched from Apache HttpClient to OkHttp. I don't have any experience with the OkHttp codebase either, but maybe I can try to look into it.

I have sent a pull request (#3766) that includes a failing test and a commit that seems to fix the problem. However, there is still one issue where I am stuck -> see the description of the PR.

This is fixed with PR #3766 , so it can be closed with the next release.

What release did this make it into? 3.10 or 3.11? I'm using Retrofit and it pulls in 3.10, but I think I'm hitting this bug.

Edit: I forced gradle to grab 3.11, but it's still failing with a 401 when I try to POST to a customer's website with okhttp (via retrofit). If I use curl, it succeeds. Most customers are fine, these guys are doing something odd, but since it works with curl there must be something I should be configuring to make it work? Or is it something to do with the certificates? Here's the log from curl:

* SSL connection using TLS1.2 / ECDHE_RSA_AES_128_GCM_SHA256
*    server certificate verification SKIPPED
*    server certificate status verification SKIPPED
*    common name: *.tourist-mobile.com (matched)
*    server certificate expiration date OK
*    server certificate activation date OK
*    certificate public key: RSA
*    certificate version: #3
*    subject: OU=Domain Control Validated,CN=*.mycustomer.com
*    start date: Mon, 04 Jun 2018 10:28:08 GMT
*    expire date: Thu, 13 Jun 2019 10:27:08 GMT
*    issuer: C=US,ST=Arizona,L=Scottsdale,O=GoDaddy.com\, Inc.,OU=http://certs.godaddy.com/repository/,CN=Go Daddy Secure Certificate Authority - G2
*    compression: NULL
* ALPN, server did not agree to a protocol
> POST /EventComMaster/callback/flightstats HTTP/1.1
> Host: mycustomer.com
> User-Agent: curl/7.47.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 15887
> Expect: 100-continue
> 
< HTTP/1.1 100 Continue
* We are completely uploaded and fine
< HTTP/1.1 200 OK
< Date: Mon, 12 Nov 2018 21:47:46 GMT
< Server: Apache/2.2.15 (CentOS)
< Pragma: no-cache
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Cache-Control: no-cache
< Cache-Control: no-store
< Content-Length: 0
< Connection: close
< Content-Type: text/plain; charset=UTF-8
< 
* Closing connection 0

You can always specify an explicit dependency on a newer version. Retrofit
only uses public APIs.

On Mon, Nov 12, 2018, 7:03 PM Chris Kessel <[email protected] wrote:

What release did this make it into? 3.10 or 3.11? I'm using Retrofit and
it pulls in 3.10, but I think I'm hitting this bug.

—
You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
https://github.com/square/okhttp/issues/3628#issuecomment-438038735, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEEEcmamFi-8dx_Y2XusUagYintIJtvks5uueuKgaJpZM4Pi9Ux
.

Yea, I forced it to 3.11 and verified it with gradle dependencies, but it still fails. I'm going to try a POST with the javax.ws.rs.Client and see if that works. We were using Apache http and it worked with the default configuration. It started failing when we converted over to Retrofit/OkHttp.

I'm guessing it's something in OkHttp, but I can't figure out what exactly. Everything looks to be configured as far as ciphers, TLS versions, etc compared to what the curl command shows should happen.

I know this is through Retrofit, so maybe not useful for you, but for what it's worth, the call works (200 response) with javax.ws.rs.Client. Default client, just ClientBuilder.newClient().

          MediaType contentType = MediaType.valueOf(deliveryFormat.getMediaType());
            Response response = httpClient.target(uri)
                    .request(contentType)
                    .post(Entity.entity(serializedAlert, contentType), Response.class);

This fails with Retrofit with a 401:

            RequestBody requestBody = RequestBody.create(MediaType.parse(deliveryFormat.getMediaType()), serializedAlert);
            Response<ResponseBody> response = retrofitCustomerCallback.postAlert(uri.toString(), requestBody).execute();

Where that postAlert is a Retrofit interface with this method:

public interface RetrofitCustomerCallback{
    @POST
    Call<ResponseBody> postAlert(@Url String customerEndpoint, @Body RequestBody alert);
}

And the OkHttp configuration is mostly basic aside from some timeout settings (which I'll be adding to the Client code above):

        OkHttpClient client = new OkHttpClient.Builder()
                .connectTimeout(myConfig.getPostConnectionTimeoutSeconds(), TimeUnit.SECONDS)
                .readTimeout(myConfig.getPostResponseTimeoutSeconds(), TimeUnit.SECONDS)
                .writeTimeout(myConfig.getPostResponseTimeoutSeconds(), TimeUnit.SECONDS)
                .build();

Unfortunately, this is posting to a customer's endpoint, so I don't have any details on how that's configured. Just the CURL trace above.

Was this page helpful?
0 / 5 - 0 ratings