MAJOR: http/compression: fix chunked-encoded response processing
Now we have valid buffer offsets, we can use them to safely parse the
input and only forward when needed. Thus we can get rid of the
consumed_data accumulator, and the code now works both for chunked and
content-length, even with a server feeding one byte at a time (which
systematically broke the previous one).
It's worth noting that 0<CRLF> must always be sent after end of data
(ie: chunk_len==0), and that the trailing CRLF is sent only content
length mode, because in chunked we'll have to pass trailers.
diff --git a/src/compression.c b/src/compression.c
index 0fd6846..17a81e8 100644
--- a/src/compression.c
+++ b/src/compression.c
@@ -128,25 +128,21 @@
*/
int http_compression_buffer_init(struct session *s, struct buffer *in, struct buffer *out)
{
- struct http_msg *msg = &s->txn.rsp;
int left;
/* not enough space */
if (in->size - buffer_len(in) < 40)
return -1;
- /*
- * Skip data, we don't need them in the new buffer. They are results
- * of CHUNK_CRLF and CHUNK_SIZE parsing.
+ /* We start by copying the current buffer's pending outgoing data into
+ * a new temporary buffer that we initialize with a new empty chunk.
*/
- b_adv(in, msg->next);
- msg->next = 0;
out->size = global.tune.bufsize;
out->i = 0;
out->o = 0;
out->p = out->data;
- /* copy output data */
+
if (in->o > 0) {
left = in->o - bo_contig_data(in);
memcpy(out->data, bo_ptr(in), bo_contig_data(in));
@@ -170,41 +166,42 @@
struct http_msg *msg = &s->txn.rsp;
int consumed_data = 0;
int data_process_len;
- int left;
- int ret;
+ int block1, block2;
/*
- * Skip data, we don't need them in the new buffer. They are results
- * of CHUNK_CRLF and CHUNK_SIZE parsing.
+ * Temporarily skip already parsed data and chunks to jump to the
+ * actual data block. It is fixed before leaving.
*/
b_adv(in, msg->next);
- msg->next = 0;
/*
* select the smallest size between the announced chunk size, the input
- * data, and the available output buffer size
+ * data, and the available output buffer size. The compressors are
+ * assumed to be able to process all the bytes we pass to them at once.
*/
data_process_len = MIN(in->i, msg->chunk_len);
data_process_len = MIN(out->size - buffer_len(out), data_process_len);
- left = data_process_len - bi_contig_data(in);
- if (left <= 0) {
- consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), data_process_len, out);
- if (ret < 0)
- return -1;
+ block1 = data_process_len;
+ if (block1 > bi_contig_data(in))
+ block1 = bi_contig_data(in);
+ block2 = data_process_len - block1;
- } else {
- consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), bi_contig_data(in), out);
- if (ret < 0)
- return -1;
- consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, in->data, left, out);
- if (ret < 0)
- return -1;
+ /* compressors return < 0 upon error or the amount of bytes read */
+ consumed_data = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), block1, out);
+ if (consumed_data >= 0 && block2 > 0) {
+ consumed_data = s->comp_algo->add_data(s->comp_ctx, in->data, block2, out);
+ if (consumed_data >= 0)
+ consumed_data += block1;
}
- b_adv(in, data_process_len);
- msg->chunk_len -= data_process_len;
+ /* restore original buffer pointer */
+ b_rew(in, msg->next);
+ if (consumed_data > 0) {
+ msg->next += consumed_data;
+ msg->chunk_len -= consumed_data;
+ }
return consumed_data;
}
@@ -245,12 +242,20 @@
*tail++ = '\r';
*tail++ = '\n';
- if (!(msg->flags & HTTP_MSGF_TE_CHNK) && msg->chunk_len == 0) {
- /* End of data, 0<CRLF><CRLF> is needed but we're not
- * in chunked mode on input so we must add it ourselves.
- */
- memcpy(tail, "0\r\n\r\n", 5);
- tail += 5;
+ /* At the end of data, we must write the empty chunk 0<CRLF>,
+ * and terminate the trailers section with a last <CRLF>. If
+ * we're forwarding a chunked-encoded response, we'll have a
+ * trailers section after the empty chunk which needs to be
+ * forwarded and which will provide the last CRLF. Otherwise
+ * we write it ourselves.
+ */
+ if (msg->msg_state >= HTTP_MSG_TRAILERS) {
+ memcpy(tail, "0\r\n", 3);
+ tail += 3;
+ if (msg->msg_state >= HTTP_MSG_DONE) {
+ memcpy(tail, "\r\n", 2);
+ tail += 2;
+ }
}
ob->i = tail - ob->p;
} else {
@@ -272,6 +277,9 @@
}
/* copy the remaining data in the tmp buffer. */
+ b_adv(ib, msg->next);
+ msg->next = 0;
+
if (ib->i > 0) {
left = ib->i - bi_contig_data(ib);
memcpy(bi_end(ob), bi_ptr(ib), bi_contig_data(ib));
diff --git a/src/proto_http.c b/src/proto_http.c
index 6dc9fb5..5242826 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -6160,7 +6160,6 @@
struct http_msg *msg = &s->txn.rsp;
static struct buffer *tmpbuf = NULL;
int compressing = 0;
- int consumed_data = 0;
int ret;
if (unlikely(msg->msg_state < HTTP_MSG_BODY))
@@ -6233,7 +6232,7 @@
switch (msg->msg_state - HTTP_MSG_DATA) {
case HTTP_MSG_DATA - HTTP_MSG_DATA: /* must still forward */
if (compressing) {
- consumed_data += ret = http_compression_buffer_add_data(s, res->buf, tmpbuf);
+ ret = http_compression_buffer_add_data(s, res->buf, tmpbuf);
if (ret < 0)
goto aborted_xfer;
}
@@ -6248,7 +6247,7 @@
msg->msg_state = HTTP_MSG_CHUNK_CRLF;
} else {
msg->msg_state = HTTP_MSG_DONE;
- if (compressing && consumed_data) {
+ if (compressing) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
compressing = 0;
}
@@ -6267,11 +6266,6 @@
http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_CRLF, s->fe);
goto return_bad_res;
}
- /* skipping data in buffer for compression */
- if (compressing) {
- b_adv(res->buf, msg->next);
- msg->next = 0;
- }
/* we're in MSG_CHUNK_SIZE now, fall through */
case HTTP_MSG_CHUNK_SIZE - HTTP_MSG_DATA:
@@ -6288,17 +6282,9 @@
http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_SIZE, s->fe);
goto return_bad_res;
}
- if (compressing) {
- if (likely(msg->chunk_len > 0)) {
- /* skipping data if we are in compression mode */
- b_adv(res->buf, msg->next);
- msg->next = 0;
- } else {
- if (consumed_data) {
- http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
- compressing = 0;
- }
- }
+ if (compressing && msg->msg_state == HTTP_MSG_TRAILERS) {
+ http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
+ compressing = 0;
}
/* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
break;
@@ -6354,7 +6340,7 @@
}
missing_data:
- if (compressing && consumed_data) {
+ if (compressing) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 0);
compressing = 0;
}