Committed. Thanks for your bug report. /Benno
Sebastian Benoit([email protected]) on 2018.10.01 14:38:08 +0200: > Reyk Floeter([email protected]) on 2018.10.01 14:06:42 +0200: > > On Wed, Sep 26, 2018 at 12:36:50PM +0200, Sebastian Benoit wrote: > > > (sorry about my last mail without subject, this is the same content) > > > > > > Sebastian Benoit([email protected]) on 2018.09.26 11:42:40 +0200: > > > > Alexander Bluhm([email protected]) on 2018.09.26 01:00:17 +0200: > > > > > On Mon, Mar 12, 2018 at 06:42:28PM +0100, Nikola Kolev wrote: > > > > > > [12/Mar/2018:19:16:56 +0100] "<UNKNOWN> " 408 0 server default, > > > > > > client 1 (1 > > > > > > active), 10.0.2.2:56670 -> 10.0.2.15, timeout (408 Request Timeout) > > > > > > ... > > > > > > > > > > According to a private mail from Nikola, this error message is > > > > > caused by the httpd request timeout configuration. Our default is > > > > > 60 seconds while Firefox uses 115 seconds. > > > > > > > > > > about:config network.http.keep-alive.timeout 115 > > > > > > > > > > So I think we should increase httpd's default to 120 seconds. Then > > > > > Firefox closes the persistent connection first and we don't get > > > > > this ugly error messages. > > > > > > > > > > ok? > > > > > > > > Sorry, but i think sending a 408 is wrong: > > > > > > > > The request timeout is the time the server waits for the client to send > > > > an > > > > HTTP-Request. > > > > > > > > If the server reaches that timeout, it should close the connection, not > > > > generate a 408. > > > > > > > > https://tools.ietf.org/html/rfc7230#page-55 > > > > 6.5. Failures and Timeouts > > > > > > > > Indeed, it seems that only httpd sends a 408 in this case, all other > > > > servers > > > > i tested just close the connection. > > > > > > > > Same if its waiting for more requests in the keep-alive case. > > > > > > Here is a diff that should make httpd close the connection without any > > > response if it did not receive anything yet (also in the keep-alive case, > > > if > > > it is waiting for the next request). > > > > > > > Thanks for the analysis. > > > > > Instead of using a new variable clt_seendata this could probably be just a > > > new value for clt_toread? > > > > > > > Why not? TOREAD already became some some kind of a state machine, so > > why no adding TOREAD_HTTP_INIT? > > ok, lets try this then, it seems to work for me. > > please check that i have not missed a path where we end up with the wrong > value in clt_toread. > > ok? > > (benno_httpd_408_3.diff) > diff --git usr.sbin/httpd/httpd.h usr.sbin/httpd/httpd.h > index 4b1d9d72237..f2517c25dd4 100644 > --- usr.sbin/httpd/httpd.h > +++ usr.sbin/httpd/httpd.h > @@ -100,10 +100,11 @@ > > enum httpchunk { > TOREAD_UNLIMITED = -1, > - TOREAD_HTTP_HEADER = -2, > - TOREAD_HTTP_CHUNK_LENGTH = -3, > - TOREAD_HTTP_CHUNK_TRAILER = -4, > - TOREAD_HTTP_NONE = -5, > + TOREAD_HTTP_INIT = -2, > + TOREAD_HTTP_HEADER = -3, > + TOREAD_HTTP_CHUNK_LENGTH = -4, > + TOREAD_HTTP_CHUNK_TRAILER = -5, > + TOREAD_HTTP_NONE = -6, > TOREAD_HTTP_RANGE = TOREAD_HTTP_CHUNK_LENGTH > }; > > diff --git usr.sbin/httpd/server.c usr.sbin/httpd/server.c > index 5f4304705d8..ec4097414fe 100644 > --- usr.sbin/httpd/server.c > +++ usr.sbin/httpd/server.c > @@ -901,7 +901,6 @@ server_input(struct client *clt) > return; > } > > - clt->clt_toread = TOREAD_HTTP_HEADER; > inrd = server_read_http; > > slen = sizeof(clt->clt_sndbufsiz); > @@ -1019,7 +1018,10 @@ server_error(struct bufferevent *bev, short error, > void *arg) > struct evbuffer *dst; > > if (error & EVBUFFER_TIMEOUT) { > - server_abort_http(clt, 408, "timeout"); > + if (clt->clt_toread != TOREAD_HTTP_INIT) > + server_abort_http(clt, 408, "timeout"); > + else > + server_abort_http(clt, 0, "timeout"); > return; > } > if (error & EVBUFFER_ERROR) { > diff --git usr.sbin/httpd/server_http.c usr.sbin/httpd/server_http.c > index 9306082edaf..c3ded934187 100644 > --- usr.sbin/httpd/server_http.c > +++ usr.sbin/httpd/server_http.c > @@ -88,6 +88,7 @@ server_httpdesc_init(struct client *clt) > } > RB_INIT(&desc->http_headers); > clt->clt_descresp = desc; > + clt->clt_toread = TOREAD_HTTP_INIT; > > return (0); > } > @@ -211,6 +212,10 @@ server_read_http(struct bufferevent *bev, void *arg) > size = EVBUFFER_LENGTH(src); > DPRINTF("%s: session %d: size %lu, to read %lld", > __func__, clt->clt_id, size, clt->clt_toread); > + > + if (clt->clt_toread == TOREAD_HTTP_INIT) > + clt->clt_toread = TOREAD_HTTP_HEADER; > + > if (!size) { > clt->clt_toread = TOREAD_HTTP_HEADER; > goto done; > @@ -734,6 +739,7 @@ server_reset_http(struct client *clt) > server_httpdesc_free(clt->clt_descresp); > clt->clt_headerlen = 0; > clt->clt_headersdone = 0; > + clt->clt_toread = TOREAD_HTTP_INIT; > clt->clt_done = 0; > clt->clt_line = 0; > clt->clt_chunk = 0; >
