Hi Nenad, On Sun, Oct 05, 2014 at 07:33:33PM +0200, Nenad Merdanovic wrote: > Hello, > > I accidentally noticed that HAproxy doesn't follow the HTTP standard > when it comes to HEAD requests. Using an 'errofile' directive will > return whatever is inside, which often means a HTTP header + a body > (used to display a nice error page to the end user). > > The standard of course forbids this (https://tools.ietf.org/html/rfc7231): > 4.3.2. HEAD > The HEAD method is identical to GET except that the server MUST NOT > send a message body in the response (i.e., the response terminates at > the end of the header section). > > > The easiest fix would be to just ignore errorfile directive on HEAD > requests, but that might cause problems for people who, like me, abuse > the errorfile directive to rewrite the return error code (and remove the > body in the process). If this is the approach we are willing to take, > the patch is attached. > > The other option I see is to add something like 'force-send' to the > errorfile directive for people who are sure they want to send the > content on HEAD requests.
Sorry, I didn't notice your mail previously. You brought a good point. The thing is that the errorfiles are intentionally full HTTP so that haproxy sends all the bytes on the wire without parsing anything. But I think that could be improved. In practice, there are rarely huge headers in such responses so we can imagine that the code used to send them looks for the empty header on the fly while sending. Another option would be to change the error message structs to store the header length and that this is checked when the file is loaded. I don't know what option is best, as the sending function is http_server_error() and it's the same which is used to send redirects, so as you guess I'd rather avoid to risk degrading it. But I still think it can reasonably be done. However we still have some direct calls to stream_int_retnclose() with the error message in arguments. This is generally on the hard error path (eg: failed to parse a request), so that's not critical. For example we have this also in the reqdeny rules or the tarpit. We also have it on the 502/504, which is more annoying to process. I think that the current state is not critical, in that the only way a client could be affected is that if it uses pipelining, sends multiple requests at once, and the error message does not contain "Connection: close" so that the client considers the body as the start of response to the second request, and unfortunately this body parses as a valid HTTP response. There could be other corner cases as well, but I think that at least for the sake of protocol compliance, we should add it to the TODO list and try to address it as well as we can. And anyway for HTTP/2 we won't be able to proceed this way anymore! Best regards, Willy
