Skip to content
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

BUG: expect: 100-continue is not properly supported #8

Open
yaronyg opened this issue Oct 30, 2013 · 3 comments
Open

BUG: expect: 100-continue is not properly supported #8

yaronyg opened this issue Oct 30, 2013 · 3 comments

Comments

@yaronyg
Copy link
Contributor

yaronyg commented Oct 30, 2013

If a request contains expect: 100-continue then per section 8.2.3 of RFC 2616 the server MUST either respond with a 100 Continue response OR it MUST return a final status code. The listener does neither as far as I can tell. This breaks Ektorp unless you turn off 100-continue support on the client side but how the heck is a client supposed to know to do that? I can see two choices. One bad but legal and one good but hard.

Bad But Legal - The Listener could look for the expect header and immediately return a 417 Expectation Failed. This would be legal. But how many clients would then fall back to not using the expect header?

Good But Hard - The Listener could properly support 100-Continue. This is hard because near as I can tell there is no support in the Servlet API for this. So we would probably need a TJWS specific hack to make this work. I sent mail to the TJWS help mailing list asking for suggestions.

@tleyden
Copy link
Contributor

tleyden commented Oct 30, 2013

Thanks for posting this. Just curious - can you give more of an idea on how to reproduce this? Eg, does it only happen on certain requests?

It seems like this should be handled by TJWS. Hopefully the TJWS maintainer will come back with something.

@tleyden
Copy link
Contributor

tleyden commented Oct 30, 2013

To put this issue in a larger context, here is a post from @yaronyg on the google group:

TLDR

Couchbase-Lite-Android-Listener has a number of bugs that make it not HTTP/1.1 compliant and as a result a 'naive' use of Ektorp with Couchbase-Lite-Android-Listener will fail. To fix this add the useExpectContinue(false) option to your StdHttpClient.Builder()/AndroidHttpClient.Builder().

Details

Couchbase-Lite-Android-Listener violates HTTP/1.1 in a number of ways, including:
#5 - HEAD response that include a response body (banned by RFC 2616)
#7 - Responses with no response body send transfer-encoding: chunked and nothing in the response body (also banned by RFC 2616)
#8 - Expect:100-Continue is not properly supported (mandated by RFC 2616)

Originally I tried to work around these problems by using hacks built right into the Apache Client library that is underneath Ektorp.

        StdHttpClient.Builder httpClientBuilder = new StdHttpClient.Builder().host(hostName).port(port);



        org.apache.http.client.HttpClient apacheHttpClient = httpClientBuilder.configureClient();

        apacheHttpClient.getParams().setIntParameter("http.protocol.head-body-timeout",500);

        apacheHttpClient.getParams().setIntParameter("http.protocol.wait-for-continue",3000);

        return new StdHttpClient(apacheHttpClient);

This made no difference. Whatever is making Ektorp unhappy it isn't the body in the HEAD response nor is it the fact that a 100 Continue or a final response doesn't show up after 3 seconds. Something else is irritating it.

        StdHttpClient.Builder httpClientBuilder = new StdHttpClient.Builder().host(hostName).port(port).useExpectContinue(false);



        return httpClientBuilder.build();

The previous fixed everything. The difference is that I turned off the use of ExpectContinue. Now it works just fine on Android (where I use AndroidHttpClient.Builder()) and Java (where I use StdHttpClient.Builder()).

This is not a great solution though. The reason is that if one is sending a large document/file to the phone running Couchbase-Lite-Android-Listener and the listener is going to reject the request for whatever reason (say authentication/authorization) then the large document/file has to be fully sent before the error will be returned. This is bad for battery life. This is bad for storage. It's just bad.

So what we need are the three bugs, especially the last one, fixed. Then we can turn expect support back on and get good perf!

But until then, if someone is trying to talk to your Listener and complaining that they are getting bizarre failures then tell them to turn off Expect Continue.

@yaronyg
Copy link
Contributor Author

yaronyg commented Oct 30, 2013

The bug should show up on any request from Ektorp to Android Listener that has a request body if you don't turn off expect: continue.

@hideki hideki added the backlog label Jun 23, 2016
@hideki hideki added icebox and removed backlog labels Oct 8, 2016
@hideki hideki removed this from the Future milestone Oct 8, 2016
@djpongh djpongh added this to the 1.4.2 milestone Nov 28, 2017
@djpongh djpongh modified the milestones: 1.4.2, 1.4.x Dec 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants