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;
