[MINOR] http: partially revert the chunking optimization for now
Commit 57f5c1 used to provide a nice improvement on chunked encoding since
it ensured that we did not set a PUSH flag for every chunk or buffer data
part of a chunked transfer.
Some applications appear to erroneously abuse HTTP chunking in order to
get interactive exchanges between a user agent and an origin server with
very small chunks. While it happens to work through haproxy, it's terribly
slow due to the latency added after passing each chunk to the system, who
could wait up to 200ms before pushing them onto the wire.
So we need an interactive mode for such usages. In the mean time, step back
on the optim, but not completely, so that we still keep the flag as long as
we know we're not finished with the current chunk.
This change should be backported to 1.4 too as the issue was discovered
with it.
diff --git a/src/proto_http.c b/src/proto_http.c
index bdfc113..07faf1f 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4445,9 +4445,6 @@
goto return_bad_req;
}
/* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
- /* Don't set a PUSH at the end of that chunk if it's not the last one */
- if (msg->msg_state == HTTP_MSG_DATA)
- req->flags |= BF_EXPECT_MORE;
}
else if (msg->msg_state == HTTP_MSG_DATA_CRLF) {
/* we want the CRLF after the data */
@@ -4561,6 +4558,17 @@
if (txn->flags & TX_REQ_TE_CHNK)
buffer_dont_close(req);
+ /* We know that more data are expected, but we couldn't send more that
+ * what we did. In theory we could always set the BF_EXPECT_MORE so that
+ * the system knows it must not set a PUSH on this first part. However
+ * there exists some applications which incorrectly rely on chunks being
+ * interactively exchanged. So we set the flag only if the current chunk
+ * is not finished, or we're not DONE and interactive mode is not set.
+ */
+ if (txn->req.msg_state >= HTTP_MSG_DATA &&
+ txn->req.msg_state <= HTTP_MSG_TRAILERS)
+ req->flags |= BF_EXPECT_MORE;
+
http_silent_debug(__LINE__, s);
return 0;
@@ -5491,9 +5499,6 @@
goto return_bad_res;
}
/* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
- /* Don't set a PUSH at the end of that chunk if it's not the last one */
- if (msg->msg_state == HTTP_MSG_DATA)
- res->flags |= BF_EXPECT_MORE;
}
else if (msg->msg_state == HTTP_MSG_DATA_CRLF) {
/* we want the CRLF after the data */
@@ -5594,6 +5599,17 @@
(txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
buffer_dont_close(res);
+ /* We know that more data are expected, but we couldn't send more that
+ * what we did. In theory we could always set the BF_EXPECT_MORE so that
+ * the system knows it must not set a PUSH on this first part. However
+ * there exists some applications which incorrectly rely on chunks being
+ * interactively exchanged. So we set the flag only if the current chunk
+ * is not finished, or we're not DONE and interactive mode is not set.
+ */
+ if (txn->rsp.msg_state >= HTTP_MSG_DATA &&
+ txn->rsp.msg_state <= HTTP_MSG_TRAILERS)
+ res->flags |= BF_EXPECT_MORE;
+
/* the session handler will take care of timeouts and errors */
http_silent_debug(__LINE__, s);
return 0;