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.
This commit is contained in:
Willy Tarreau 2014-04-18 00:20:14 +02:00
parent 5fb0abd9a1
commit 7f2f8d5cc3
2 changed files with 47 additions and 53 deletions

View File

@ -128,25 +128,21 @@ int http_emit_chunk_size(char *out, unsigned int chksz, int add_crlf)
*/ */
int http_compression_buffer_init(struct session *s, struct buffer *in, struct buffer *out) int http_compression_buffer_init(struct session *s, struct buffer *in, struct buffer *out)
{ {
struct http_msg *msg = &s->txn.rsp;
int left; int left;
/* not enough space */ /* not enough space */
if (in->size - buffer_len(in) < 40) if (in->size - buffer_len(in) < 40)
return -1; return -1;
/* /* We start by copying the current buffer's pending outgoing data into
* Skip data, we don't need them in the new buffer. They are results * a new temporary buffer that we initialize with a new empty chunk.
* of CHUNK_CRLF and CHUNK_SIZE parsing.
*/ */
b_adv(in, msg->next);
msg->next = 0;
out->size = global.tune.bufsize; out->size = global.tune.bufsize;
out->i = 0; out->i = 0;
out->o = 0; out->o = 0;
out->p = out->data; out->p = out->data;
/* copy output data */
if (in->o > 0) { if (in->o > 0) {
left = in->o - bo_contig_data(in); left = in->o - bo_contig_data(in);
memcpy(out->data, bo_ptr(in), bo_contig_data(in)); memcpy(out->data, bo_ptr(in), bo_contig_data(in));
@ -170,41 +166,42 @@ int http_compression_buffer_add_data(struct session *s, struct buffer *in, struc
struct http_msg *msg = &s->txn.rsp; struct http_msg *msg = &s->txn.rsp;
int consumed_data = 0; int consumed_data = 0;
int data_process_len; int data_process_len;
int left; int block1, block2;
int ret;
/* /*
* Skip data, we don't need them in the new buffer. They are results * Temporarily skip already parsed data and chunks to jump to the
* of CHUNK_CRLF and CHUNK_SIZE parsing. * actual data block. It is fixed before leaving.
*/ */
b_adv(in, msg->next); b_adv(in, msg->next);
msg->next = 0;
/* /*
* select the smallest size between the announced chunk size, the input * 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(in->i, msg->chunk_len);
data_process_len = MIN(out->size - buffer_len(out), data_process_len); data_process_len = MIN(out->size - buffer_len(out), data_process_len);
left = data_process_len - bi_contig_data(in); block1 = data_process_len;
if (left <= 0) { if (block1 > bi_contig_data(in))
consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), data_process_len, out); block1 = bi_contig_data(in);
if (ret < 0) block2 = data_process_len - block1;
return -1;
} else { /* compressors return < 0 upon error or the amount of bytes read */
consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), bi_contig_data(in), out); consumed_data = s->comp_algo->add_data(s->comp_ctx, bi_ptr(in), block1, out);
if (ret < 0) if (consumed_data >= 0 && block2 > 0) {
return -1; consumed_data = s->comp_algo->add_data(s->comp_ctx, in->data, block2, out);
consumed_data += ret = s->comp_algo->add_data(s->comp_ctx, in->data, left, out); if (consumed_data >= 0)
if (ret < 0) consumed_data += block1;
return -1;
} }
b_adv(in, data_process_len); /* restore original buffer pointer */
msg->chunk_len -= data_process_len; b_rew(in, msg->next);
if (consumed_data > 0) {
msg->next += consumed_data;
msg->chunk_len -= consumed_data;
}
return consumed_data; return consumed_data;
} }
@ -245,12 +242,20 @@ int http_compression_buffer_end(struct session *s, struct buffer **in, struct bu
*tail++ = '\r'; *tail++ = '\r';
*tail++ = '\n'; *tail++ = '\n';
if (!(msg->flags & HTTP_MSGF_TE_CHNK) && msg->chunk_len == 0) { /* At the end of data, we must write the empty chunk 0<CRLF>,
/* End of data, 0<CRLF><CRLF> is needed but we're not * and terminate the trailers section with a last <CRLF>. If
* in chunked mode on input so we must add it ourselves. * we're forwarding a chunked-encoded response, we'll have a
*/ * trailers section after the empty chunk which needs to be
memcpy(tail, "0\r\n\r\n", 5); * forwarded and which will provide the last CRLF. Otherwise
tail += 5; * 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; ob->i = tail - ob->p;
} else { } else {
@ -272,6 +277,9 @@ int http_compression_buffer_end(struct session *s, struct buffer **in, struct bu
} }
/* copy the remaining data in the tmp buffer. */ /* copy the remaining data in the tmp buffer. */
b_adv(ib, msg->next);
msg->next = 0;
if (ib->i > 0) { if (ib->i > 0) {
left = ib->i - bi_contig_data(ib); left = ib->i - bi_contig_data(ib);
memcpy(bi_end(ob), bi_ptr(ib), bi_contig_data(ib)); memcpy(bi_end(ob), bi_ptr(ib), bi_contig_data(ib));

View File

@ -6160,7 +6160,6 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
struct http_msg *msg = &s->txn.rsp; struct http_msg *msg = &s->txn.rsp;
static struct buffer *tmpbuf = NULL; static struct buffer *tmpbuf = NULL;
int compressing = 0; int compressing = 0;
int consumed_data = 0;
int ret; int ret;
if (unlikely(msg->msg_state < HTTP_MSG_BODY)) if (unlikely(msg->msg_state < HTTP_MSG_BODY))
@ -6233,7 +6232,7 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
switch (msg->msg_state - HTTP_MSG_DATA) { switch (msg->msg_state - HTTP_MSG_DATA) {
case HTTP_MSG_DATA - HTTP_MSG_DATA: /* must still forward */ case HTTP_MSG_DATA - HTTP_MSG_DATA: /* must still forward */
if (compressing) { 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) if (ret < 0)
goto aborted_xfer; goto aborted_xfer;
} }
@ -6248,7 +6247,7 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
msg->msg_state = HTTP_MSG_CHUNK_CRLF; msg->msg_state = HTTP_MSG_CHUNK_CRLF;
} else { } else {
msg->msg_state = HTTP_MSG_DONE; msg->msg_state = HTTP_MSG_DONE;
if (compressing && consumed_data) { if (compressing) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 1); http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
compressing = 0; compressing = 0;
} }
@ -6267,11 +6266,6 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_CRLF, s->fe); http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_CRLF, s->fe);
goto return_bad_res; 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 */ /* we're in MSG_CHUNK_SIZE now, fall through */
case HTTP_MSG_CHUNK_SIZE - HTTP_MSG_DATA: case HTTP_MSG_CHUNK_SIZE - HTTP_MSG_DATA:
@ -6288,17 +6282,9 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_SIZE, s->fe); http_capture_bad_message(&s->be->invalid_rep, s, msg, HTTP_MSG_CHUNK_SIZE, s->fe);
goto return_bad_res; goto return_bad_res;
} }
if (compressing) { if (compressing && msg->msg_state == HTTP_MSG_TRAILERS) {
if (likely(msg->chunk_len > 0)) { http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
/* skipping data if we are in compression mode */ compressing = 0;
b_adv(res->buf, msg->next);
msg->next = 0;
} else {
if (consumed_data) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 1);
compressing = 0;
}
}
} }
/* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */ /* otherwise we're in HTTP_MSG_DATA or HTTP_MSG_TRAILERS state */
break; break;
@ -6354,7 +6340,7 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
} }
missing_data: missing_data:
if (compressing && consumed_data) { if (compressing) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 0); http_compression_buffer_end(s, &res->buf, &tmpbuf, 0);
compressing = 0; compressing = 0;
} }