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?
Reyk
> diff --git usr.sbin/httpd/httpd.h usr.sbin/httpd/httpd.h
> index 4b1d9d72237..75a62fb3ada 100644
> --- usr.sbin/httpd/httpd.h
> +++ usr.sbin/httpd/httpd.h
> @@ -359,6 +359,7 @@ struct client {
> int clt_done;
> int clt_chunk;
> int clt_inflight;
> + int clt_seendata;
> struct range_data clt_ranges;
> struct fcgi_data clt_fcgi;
> char *clt_remote_user;
> diff --git usr.sbin/httpd/server.c usr.sbin/httpd/server.c
> index 5f4304705d8..67712cc8462 100644
> --- usr.sbin/httpd/server.c
> +++ usr.sbin/httpd/server.c
> @@ -1019,7 +1019,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_seendata)
> + 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..742e1b2c207 100644
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -216,6 +216,8 @@ server_read_http(struct bufferevent *bev, void *arg)
> goto done;
> }
>
> + clt->clt_seendata = 1;
> +
> while (!clt->clt_headersdone) {
> if (!clt->clt_line) {
> /* Peek into the buffer to see if it looks like HTTP */
> @@ -498,6 +500,7 @@ server_read_httpcontent(struct bufferevent *bev, void
> *arg)
> if (clt->clt_toread == 0) {
> fcgi_add_stdin(clt, NULL);
> clt->clt_toread = TOREAD_HTTP_HEADER;
> + clt->clt_seendata = 1;
> bufferevent_disable(bev, EV_READ);
> bev->readcb = server_read_http;
> return;
>
--
