BUG/MEDIUM: proto-http: Always start the parsing if there is no outgoing data
When we are waiting for a request or a response, if the channel's buffer is not
rewritable (the reservce is not fully free), nothing is done and we wait to have
a rewritable buffer. It was an old implicit assumption of HTTP analyzers. On old
versions, at this stage, if a buffer was not rewritable, it meant some outgoing
data were pending to be sent.
On recent versions, it should not happen because all outgoing data are sent
before starting the analysis of the next transaction. But the applets may be
lead to use the reserve. For instance, the cache applet adds the header "Age" to
cached responses. It may use the reserve to do so if the size of the response
headers is huge. So, in such case, the implicit assumption of a no rewritable
buffer because of output data is wrong. But the message analysis remains
blocked, sometime infinitely depending on circumstances.
To fix the bug and to avoid any ambiguity, we now also check if there are some
outgoing data when the buffer is not rewritable to postpone the message
analysis. In fact, this code may probably be removed because it should never
happen. But I prefer to be conservative here and don't introduce a bug because of
an unknown/unexpected hidden corner case. Anyway, it is not a big deal because
all legacy HTTP code is removed in the 2.1.
This is a direct commit to the 2.0 branch, as the problem doesn't exist in
master. It must be backported at least to 1.9 and 1.8 because of the cache. But
it may be also backported to all stable versions.
This patch should partly fix the github issue #233.
diff --git a/src/proto_http.c b/src/proto_http.c
index f77d7bc..d29f13c 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -598,7 +598,7 @@
*/
if (c_data(req) && msg->msg_state < HTTP_MSG_ERROR) {
if (txn->flags & TX_NOT_FIRST) {
- if (unlikely(!channel_is_rewritable(req))) {
+ if (unlikely(!channel_is_rewritable(req) && co_data(req))) {
if (req->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_WRITE_ERROR|CF_WRITE_TIMEOUT))
goto failed_keep_alive;
/* some data has still not left the buffer, wake us once that's done */
@@ -4230,7 +4230,7 @@
* data later, which is much more complicated.
*/
if (c_data(rep) && msg->msg_state < HTTP_MSG_ERROR) {
- if (unlikely(!channel_is_rewritable(rep))) {
+ if (unlikely(!channel_is_rewritable(rep) && co_data(rep))) {
/* some data has still not left the buffer, wake us once that's done */
if (rep->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_WRITE_ERROR|CF_WRITE_TIMEOUT))
goto abort_response;