The twisted.web server should not ignore "Expect: 100-continue"

[itamar]

From the RFC: "The purpose of the 100 (Continue) status ... is to allow a client that is sending a request message with a request body to determine if the origin server is willing to accept the request (based on the request headers) before the client sends the request body."

See  http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html#sec8.2.3 for details.

Supporting the "Expect: 100-continue" header on an API level is currently difficult, since twisted.web always reads the full request body before handing the request to business logic code. Until we can allow custom Resources to handle requests before the body is read, we should at the very least return the appropriate "100 (Continue)" response to the HTTP client so it can continue.

[itamarst]

(In [32482]) Branching to '100-continue-4673'

[itamar]

This is an implementation of the simple, just respond with 100 continue, suggestion. A user-exposed API depends on #288. I tested with curl, and without this POSTs take an extra 1 second as curl waits for the 100 continue, decides it's not going to happen and then switches to normal processing.

[itamar]

 http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/100-continue-4673 is the buildbot run, just started, hopefully they all pass :).

[exarkun]

1. I think that writing out a 100-Continue response as soon as the "Expect: 100-Continue" request header is received has a minor problem. A later request header may be unparseable and result in a 400 response; for example, a multipart/form-data request without a content-disposition field results in a 400 response. Another case which may result in a 400 response is the case where the maximum number of request headers is exceeded. It's not the end of the world if we send a 100 and then a 400, probably, but it would probably be better to avoid doing so.
2. It's not immediately obvious that this works correctly with pipelined requests. I think it does, but it would be nice to see a unit test demonstrating that the response to an earlier request isn't corrupted by a later request which includes an "Expect: 100-Continue".
3. test_HTTP10 docstring typos Except for Expect

[itamar]

All 3 comments were addressed. As a result of the change, the Expect header will now (as in trunk) be exposed to user code; this can be changed if necessary.

Test run, in progress:  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/100-continue-4673

[exarkun]

Thanks, I like the look of this. One thing though, for the behavior exercised by the new test_expect100ContinueWithPipelining, I was thinking of the case where requests arrive in the opposite order as they do in the current implementation, and where the first request has an asynchronous response which lasts past the time when the second request is received. If you feel like adding that one, cool, otherwise I don't know if the new one that's there now offers much - I suppose it's a another case but it doesn't seem like one that's very likely to different.

Also, the news fragment says Twisted.web's server code but it should probably be twisted.web's server code or even twisted.web.server.

Address to your satisfaction and merge. Thanks.

[itamar]

I'm just going to merge as is (with updated news file).

[itamarst]

(In [32596]) Merge 100-continue-4673.

Fixes: #4673 Author: itamarst Review: exarkun

twisted.web server code now responds to 100-continue headers, although this is not controllable by user code yet.