*From: *Willy Tarreau <w...@1wt.eu> *Sent: * 2014-05-02 12:56:16 E *To: *Patrick Hemmer <hapr...@stormcloud9.net> *CC: *Rachel Chavez <rachel.chave...@gmail.com>, haproxy@formilux.org *Subject: *Re: please check
> On Fri, May 02, 2014 at 12:18:43PM -0400, Patrick Hemmer wrote: >>>> At any moment the server is free to respond yes, but the server cannot >>>> respond *properly* until it gets the complete request. >>> Yes it can, redirects are the most common anticipated response, as the >>> result of a POST to a page with an expired cookie. And the 302 is a >>> clean response, it's not even an error. >> I should have clarified what I meant by "properly" more. I didn't mean >> that the server can't respond at all, as there are many cases it can, >> some of which you point out. I meant that if the server is expecting a >> request body, it can't respond with a 200 until it verifies that request >> body. > OK, but from a reverse-proxy point of view, all of them are equally valid, > and there's even no way to know if the server is interested in receiving > these data at all. The only differences are that some of them are considered > "precious" (ie those returning 200) and other ones less since they're > possibly ephemeral. > >>>> If the response depends on the request payload, the server doesn't know >>>> whether to respond with 200 or with a 400. >>> With WAFs deployed massively on server infrastructures, 403 are quite >>> common long before the whole data. 413 request entity too large appears >>> quite commonly as well. 401 and 407 can also happen when authentication >>> is needed. >>> >>>> RFC2616 covers this behavior in depth. See "8.2.3 Use of the 100 >>>> (Continue) Status". This section indicates that it should not be >>>> expected for the server to respond without a request body unless the >>>> client explicitly sends a "Expect: 100-continue" >>> Well, 2616 is 15-years old now and pretty obsolete, which is why the >>> HTTP-bis WG is working on refreshing this. New wording is clearer about >>> how a request body is used : >>> >>> o A server MAY omit sending a 100 (Continue) response if it has >>> already received some or all of the message body for the >>> corresponding request, or if the framing indicates that there is >>> no message body. >>> >>> Note the "some or all". >> I'm assuming you're quoting from: >> http://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-26#section-5.1.1 > Yes indeed. Ah in fact I found the exact part I was looking for, it's in > the same block, two points below : > > o A server that responds with a final status code before reading the > entire message body SHOULD indicate in that response whether it > intends to close the connection or continue reading and discarding > the request message (see Section 6.6 of [Part1]). > >> This only applies if the "Expect: 100-continue" was sent. "Expect: >> 100-continue" was meant to solve the issue where the client has a large >> body, and wants to make sure that the server will accept the body before >> sending it (and wasting bandwidth). Meaning that without sending >> "Expect: 100-continue", it is expected that the server will not send a >> response until the body has been sent. > No, it is expected that it will need to consume all the data before the > connection may be reused for sending another request. That is the point > of 100. And the problem is that if the server closes the connection when > responding early (typically a 302) and doesn't drain the client's data, > there's a high risk that the TCP stack will send an RST that can arrive > before the actual response, making the client unaware of the response. > That's why the server must consume the data even if it responds before > the end. " A 100-continue expectation informs recipients that the client is about to send a (presumably large) message body in this request and wishes to receive a 100 (Continue) interim response if the request- line and header fields are not sufficient to cause an immediate success, redirect, or error response. This allows the client to wait for an indication that it is worthwhile to send the message body before actually doing so, which can improve efficiency when the message body is huge or when the client anticipates that an error is likely > > (...) >>> In general (unless there's something wrong with the way client timeouts >>> are reported in http_request_forward_body), client timeouts should be >>> reported as such, and same for server timeouts. It's possible that there >>> are corner cases, but we need to be extremely careful about them and not >>> try to generalize. >> I agree, a client timeout should be reported as such, and that's what >> this is all about. If the client sends half the body (or no body), and >> then freezes, the client timeout should kick in and send back a 408, not >> the server timeout resulting in a 504. > Yes, I agree with this description. > >> I think in this regards it is very clear. >> * The server may respond with the HTTP response status code any time it >> feels like it. > OK > >> * Enable the server timeout and disable the client timeout upon any of >> the following: >> * The client sent "Expect: 100-continue" and has completed all headers > No, this one is wrong as well, as the client is expected to start sending > if it does not see the 100-continue, for compatibility with 1.0 and pre-2616 > servers, because this header was invented very late. So both sides are > responsible for acting here, and the client timeout must not be cleared. > >> * The complete client request has been sent, including body if >> "Content-Length" > 0 > Yes or chunked encoding is used and all the request, body and trailers > have been received. This is already done exactly this way (unless there's > a but of course). > >> * Writing to the server socket would result in a blocking write >> (indicating that the remote end is not processing). > In fact we don't block, we disable client-side timeout when the request > buffer is full. And this buffer is full because the server stops consuming > data. > >> * Enable the client timeout and disable the server timeout upon any of >> the following: >> * New connection. >> * The server has responded to the "Expect: 100-continue". > This one would be tricky as it will likely make it possible for a server > to block timeouts forever if sent at the right time. Also, HTTP has a bug > with 100-continue and pipelining : since you may receive as many 100-continue > as you want, as a client you can miss some of them if you sent a pipelined > request and accidentely discarded too many 100 in the response. Also, > "Expect: 100-continue" with "content-length: 0" is a fun one that nobody > knows how to deal with :-) > > So I'd rather stay on the safe side and maintain the server timeout even > after 100 was sent. Anyway, do not forget that the server is expected to > timeout as well if it's fed up. So better keep in mind that its own timeout > will strike before ours. If our timeout strikes in such a place, it's in > order to cover network issues for example. > >> * The complete response has been sent, and "Connection: close" was >> not set. > The connection goes back to the "idle" state there in fact and is > handled by the idle session manager. > >> * Writing to the client socket would result in a blocking write. >> The only ambiguity is the "Connection: upgrade" "Upgrade: websocket", as >> at that point the stream is interactive. I'm sure whatever haproxy does >> here is the right behavior (whether it is to disable both timers, or >> enable both timers). > It switches to the tunnel mode which is 100% equivalent to TCP, in which > both sides are responsible for sending and consuming data. In fact, for > keeping the flow alive. > >> Does this not cover all scenarios? I can't think of anything where this >> would not result in the desired behavior. > I think that there are something like 10 or 20 other cases that are dealt > with in the code and that I don't have right now in mind. Most of the > problematic cases are due to simultaneous events (eg: client closes while > server times out, etc...). We report in the logs what we identify as the > *first* culprit for aborting a transfer, not as the *single* culprit. And > with equal timeouts on both sides, you sometimes get a pseudo-random > generator :-) > > But in general what you described is already what is being done. As I > said, the point where I have a doubt is about how the client timeout > is handled during request body transfer because I didn't find where in > the code the status code is set for this specific case. It is possible > that we fall into the default case and that the code is assigned at a > later point once part of the cleanup has started. > > Willy While I strongly disagree with your interpretation of "Expect: 100-continue", I also don't much care about 100-continue. Hardly anyone uses it. I was just using it as documentation that the server should not be expected to respond before the entire request has been sent. The main thing I care about is not responding with 504 if the client freezes while sending the body. This has been a thorn in our side for quite some time now, and why I am interested in this patch. I've set up a test scenario, and the only time haproxy responds with 408 is if the client times out in the middle of request headers. If the client has sent all headers, but no body, or partial body, it times out after the configured 'timeout server' value, and responds with 504. Applying the patch solves this behavior. But my test scenario is very simple, and I'm not sure if it has any other consequences. -Patrick
