Package: lighttpd Version: 1.4.19-5 Severity: normal Tags: patch Hi,
I found a bug in lighttpd tonight relating to pipelining requests. If you make more requests on a single connection than the max-keep-alive-requests (default 16), and your client is pipelining, *and* you're unlucky, it'll sometimes RST your connection and give you bad data. This is probably related to (and may be the cause of) the lighttpd bug reported here: http://redmine.lighttpd.net/issues/657 Attached below is a patch which fixes the problem, as well as giving a more thorough explanation of what's going on. Have fun, Avery -- System Information: Debian Release: 4.0 APT prefers stable APT policy: (500, 'stable') Architecture: i386 (i686) Kernel: Linux 2.6.18-6-686 (SMP w/2 CPU cores) Locale: LANG=en_CA.UTF-8, LC_CTYPE=en_CA.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/bash Versions of packages lighttpd depends on: ii libattr1 1:2.4.43-2 Extended attribute shared library ii libbz2-1.0 1.0.5-1 high-quality block-sorting file co ii libc6 2.7-18 GNU C Library: Shared libraries ii libfam0 2.7.0-13.3 Client library to control the FAM ii libldap-2.4-2 2.4.11-1 OpenLDAP libraries ii libpcre3 7.6-2.1 Perl 5 Compatible Regular Expressi ii libssl0.9.8 0.9.8g-15+lenny1 SSL shared libraries ii libterm-readline-perl- 1.0302-1 Perl implementation of Readline li ii lsb-base 3.2-20 Linux Standard Base 3.2 init scrip ii mime-support 3.39-1 MIME files 'mime.types' & 'mailcap ii zlib1g 1:1.2.3.3.dfsg-12 compression library - runtime lighttpd recommends no packages. Versions of packages lighttpd suggests: ii apache2-utils 2.2.3-4+etch6 utility programs for webservers ii openssl 0.9.8c-4etch3 Secure Socket Layer (SSL) binary a pn rrdtool <none> (no description available) -- no debconf information commit 9ecb4c72816acd71f810c8d6db88cf829027fc10 Author: Avery Pennarun <[email protected]> Date: Thu Aug 13 22:06:31 2009 -0400 Fix linger-on-close behaviour to avoid rare failure conditions. - Don't assume that when FIONREAD returns 0, that it's safe to close the socket. There may still be data that's about to arrive, and we'll still send an RST if the socket is confused, potentially confusing the client. - Don't close the connection immediately after sending a successful response; linger-on-close was only happening in the case of errors, but it has to happen in case of success too, because the client doesn't necessarily know we're about to close after this request, and may have sent additional ones. (eg. if server.max-keep-alive-requests is small.) - Don't close the connection immediately even if keep_alive is 0; there are several reasons keep_alive can be 0. If the client requested Connection: close, then it would be okay to close right away, since we can assume he didn't send anything else. But it's harmless (and more resilient) to do the lingering regardless. - Increase the lingering timeout from 1s to 30s. In the vast majority of cases, the timeout never kicks in anyway. The only times when it might be needed are a) in race conditions, in which case timing out too early defeats the purpose of lingering at all; b) if there's a lot of data, which is basically the same as (a); or c) if the remote end disappears, in which case we now suffer through a longer timeout... but we would anyway, if we were waiting for them to receive our transmission. diff --git a/src/connections.c b/src/connections.c index d547b65..fbbdf40 100644 --- a/src/connections.c +++ b/src/connections.c @@ -1255,9 +1255,11 @@ handler_t connection_handle_fdevent(void *s, void *context, int revents) { /* */ read(con->fd, buf, sizeof(buf)); } else { - /* nothing to read */ - - con->close_timeout_ts = 0; + /* nothing to read - yet. But that doesn't + * mean something won't show up in our buffers + * sometime soon, so we can't quite until + * poll() gives us the HUP notification. + */ } } @@ -1569,7 +1571,12 @@ int connection_state_machine(server *srv, connection *con) { } } #endif - connection_close(srv, con); + if ((0 == shutdown(con->fd, SHUT_WR))) { + con->close_timeout_ts = srv->cur_ts; + connection_set_state(srv, con, CON_STATE_CLOSE); + } else { + connection_close(srv, con); + } srv->con_closed++; } @@ -1594,28 +1601,31 @@ int connection_state_machine(server *srv, connection *con) { "state for fd", con->fd, connection_get_state(con->state)); } - if (con->keep_alive) { - if (ioctl(con->fd, FIONREAD, &b)) { - log_error_write(srv, __FILE__, __LINE__, "ss", - "ioctl() failed", strerror(errno)); - } - if (b > 0) { - char buf[1024]; - log_error_write(srv, __FILE__, __LINE__, "sdd", - "CLOSE-read()", con->fd, b); - - /* */ - read(con->fd, buf, sizeof(buf)); - } else { - /* nothing to read */ + /* we have to do the linger_on_close stuff regardless + * of con->keep_alive; even non-keepalive sockets may + * still have unread data, and closing before reading + * it will make the client not see all our output. + */ + if (ioctl(con->fd, FIONREAD, &b)) { + log_error_write(srv, __FILE__, __LINE__, "ss", + "ioctl() failed", strerror(errno)); + } + if (b > 0) { + char buf[1024]; + log_error_write(srv, __FILE__, __LINE__, "sdd", + "CLOSE-read()", con->fd, b); - con->close_timeout_ts = 0; - } + /* */ + read(con->fd, buf, sizeof(buf)); } else { - con->close_timeout_ts = 0; + /* nothing to read - yet. But that doesn't + * mean something won't show up in our buffers + * sometime soon, so we can't quite until + * poll() gives us the HUP notification. + */ } - if (srv->cur_ts - con->close_timeout_ts > 1) { + if (srv->cur_ts - con->close_timeout_ts > 30) { connection_close(srv, con); if (srv->srvconf.log_state_handling) { @@ -1739,8 +1749,7 @@ int connection_state_machine(server *srv, connection *con) { connection_reset(srv, con); /* close the connection */ - if ((con->keep_alive == 1) && - (0 == shutdown(con->fd, SHUT_WR))) { + if ((0 == shutdown(con->fd, SHUT_WR))) { con->close_timeout_ts = srv->cur_ts; connection_set_state(srv, con, CON_STATE_CLOSE); -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected]
