From c24715e5f76d96d848ca66a93d80951a276c4cb8 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 17 Apr 2014 15:21:20 +0200 Subject: [PATCH] MAJOR: http: don't update msg->sov anymore while processing the body We used to have msg->sov updated for every chunk that was parsed. The issue is that we want to be able to rewind after chunks were parsed in case we need to redispatch a request and perform a new hash on the request or insert a different server header name. Currently, msg->sov and msg->next make parallel progress. We reached a point where they're always equal because msg->next is initialized from msg->sov, and is subtracted msg->sov's value each time msg->sov bytes are forwarded. So we can now ensure that msg->sov can always be replaced by msg->next for every state after HTTP_MSG_BODY where it is used as a position counter. This allows us to keep msg->sov untouched whatever the number of chunks that are parsed, as is needed to extract data from POST request (eg: url_param). However, we still need to know the starting position of the data relative to the body, which differs by the chunk size length. We use msg->sol for this since it's now always zero and unused in the body. So with this patch, we have the following situation : - msg->sov = msg->eoh + msg->eol = size of the headers including last CRLF - msg->sol = length of the chunk size if any. So msg->sov + msg->sol = DATA. - msg->next corresponds to the byte being inspected based on the current state and is always >= msg->sov before starting to forward anything. Since sov and next are updated in case of header rewriting, a rewind will fix them both when needed. Of course, ->sol has no reason for changing in such conditions, so it's fine to keep it relative to msg->sov. In theory, even if a redispatch has to be performed, a transformation occurring on the request would still work because the data moved would still appear at the same place relative to bug->p. --- src/backend.c | 6 ++-- src/compression.c | 2 -- src/proto_http.c | 78 ++++++++++++++++++++--------------------------- 3 files changed, 36 insertions(+), 50 deletions(-) diff --git a/src/backend.c b/src/backend.c index 72c12d1c6e..17e4b7fcbf 100644 --- a/src/backend.c +++ b/src/backend.c @@ -299,12 +299,12 @@ struct server *get_server_ph_post(struct session *s) struct proxy *px = s->be; unsigned int plen = px->url_param_len; unsigned long len = msg->body_len; - const char *params = b_ptr(req->buf, (int)(msg->sov - req->buf->o)); + const char *params = b_ptr(req->buf, (int)(msg->sov + msg->sol - req->buf->o)); const char *p = params; const char *start, *end; - if (len > buffer_len(req->buf) - msg->sov) - len = buffer_len(req->buf) - msg->sov; + if (len > buffer_len(req->buf) - msg->sov - msg->sol) + len = buffer_len(req->buf) - msg->sov - msg->sol; if (len == 0) return NULL; diff --git a/src/compression.c b/src/compression.c index 868083263c..0fd684692a 100644 --- a/src/compression.c +++ b/src/compression.c @@ -141,7 +141,6 @@ int http_compression_buffer_init(struct session *s, struct buffer *in, struct bu */ b_adv(in, msg->next); msg->next = 0; - msg->sov = 0; out->size = global.tune.bufsize; out->i = 0; @@ -180,7 +179,6 @@ int http_compression_buffer_add_data(struct session *s, struct buffer *in, struc */ b_adv(in, msg->next); msg->next = 0; - msg->sov = 0; /* * select the smallest size between the announced chunk size, the input diff --git a/src/proto_http.c b/src/proto_http.c index 943439cbe4..1334579cb5 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -1923,9 +1923,11 @@ void http_change_connection_header(struct http_txn *txn, struct http_msg *msg, i return; } -/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to the - * first byte of body, and increments msg->sov by the number of bytes parsed. - * so that we know we can forward ->sov bytes. +/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to + * the first byte of data after the chunk size, so that we know we can forward + * exactly msg->next bytes. msg->sol contains the exact number of bytes forming + * the chunk size. That way it is always possible to differentiate between the + * start of the body and the start of the data. * Return >0 on success, 0 when some data is missing, <0 on error. * Note: this function is designed to parse wrapped CRLF at the end of the buffer. */ @@ -2008,12 +2010,12 @@ static inline int http_parse_chunk_size(struct http_msg *msg) } /* OK we found our CRLF and now points to the next byte, - * which may or may not be present. We save that into ->next and - * ->sov. + * which may or may not be present. We save that into ->next, + * and the number of bytes parsed into msg->sol. */ + msg->sol = ptr - ptr_old; if (unlikely(ptr < ptr_old)) - msg->sov += buf->size; - msg->sov += ptr - ptr_old; + msg->sol += buf->size; msg->next = buffer_count(buf, buf->p, ptr); msg->chunk_len = chunk; msg->body_len += chunk; @@ -2029,14 +2031,14 @@ static inline int http_parse_chunk_size(struct http_msg *msg) * the trailers is found, it is automatically scheduled to be forwarded, * msg->msg_state switches to HTTP_MSG_DONE, and the function returns >0. * If not enough data are available, the function does not change anything - * except maybe msg->next and msg->sov if it could parse some lines, and returns - * zero. If a parse error is encountered, the function returns < 0 and does not - * change anything except maybe msg->next and msg->sov. Note that the message - * must already be in HTTP_MSG_TRAILERS state before calling this function, + * except maybe msg->next if it could parse some lines, and returns zero. + * If a parse error is encountered, the function returns < 0 and does not + * change anything except maybe msg->next. Note that the message must + * already be in HTTP_MSG_TRAILERS state before calling this function, * which implies that all non-trailers data have already been scheduled for - * forwarding, and that msg->sov exactly matches the length of trailers already - * parsed and not forwarded. It is also important to note that this function is - * designed to be able to parse wrapped headers at end of buffer. + * forwarding, and that msg->next exactly matches the length of trailers + * already parsed and not forwarded. It is also important to note that this + * function is designed to be able to parse wrapped headers at end of buffer. */ static int http_forward_trailers(struct http_msg *msg) { @@ -2083,11 +2085,6 @@ static int http_forward_trailers(struct http_msg *msg) if (bytes < 0) bytes += buf->size; - /* schedule this line for forwarding */ - msg->sov += bytes; - if (msg->sov >= buf->size) - msg->sov -= buf->size; - if (p1 == b_ptr(buf, msg->next)) { /* LF/CRLF at beginning of line => end of trailers at p2. * Everything was scheduled for forwarding, there's nothing @@ -2102,9 +2099,9 @@ static int http_forward_trailers(struct http_msg *msg) } } -/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF or - * a possible LF alone at the end of a chunk. It automatically adjusts msg->sov, - * and ->next in order to include this part into the next forwarding phase. +/* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF + * or a possible LF alone at the end of a chunk. It automatically adjusts + * msg->next in order to include this part into the next forwarding phase. * Note that the caller must ensure that ->p points to the first byte to parse. * It also sets msg_state to HTTP_MSG_CHUNK_SIZE and returns >0 on success. If * not enough data are available, the function does not change anything and @@ -2142,8 +2139,7 @@ static inline int http_skip_chunk_crlf(struct http_msg *msg) ptr++; if (unlikely(ptr >= buf->data + buf->size)) ptr = buf->data; - /* prepare the CRLF to be forwarded (->sov) */ - msg->sov += bytes; + /* Advance ->next to allow the CRLF to be forwarded */ msg->next += bytes; msg->msg_state = HTTP_MSG_CHUNK_SIZE; return 1; @@ -5005,8 +5001,7 @@ int http_resync_states(struct session *s) * be between MSG_BODY and MSG_DONE (inclusive). It returns zero if it needs to * read more data, or 1 once we can go on with next request or end the session. * When in MSG_DATA or MSG_TRAILERS, it will automatically forward chunk_len - * bytes of pending data + the headers if not already done (between sol and sov). - * It eventually adjusts sol to match sov after the data in between have been sent. + * bytes of pending data + the headers if not already done. */ int http_request_forward_body(struct session *s, struct channel *req, int an_bit) { @@ -5065,11 +5060,10 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit http_silent_debug(__LINE__, s); /* we may have some pending data starting at req->buf->p */ - if (msg->chunk_len || msg->sov) { - msg->chunk_len += msg->sov; + if (msg->chunk_len || msg->next) { + msg->chunk_len += msg->next; msg->chunk_len -= channel_forward(req, msg->chunk_len); - msg->next -= msg->sov; - msg->sov = 0; + msg->next = 0; } if (msg->msg_state == HTTP_MSG_DATA) { @@ -5087,7 +5081,7 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit } else if (msg->msg_state == HTTP_MSG_CHUNK_SIZE) { /* read the chunk size and assign it to ->chunk_len, then - * set ->sov and ->next to point to the body and switch to DATA or + * set ->next to point to the body and switch to DATA or * TRAILERS state. */ int ret = http_parse_chunk_size(msg); @@ -6153,8 +6147,7 @@ int http_process_res_common(struct session *t, struct channel *rep, int an_bit, * be between MSG_BODY and MSG_DONE (inclusive). It returns zero if it needs to * read more data, or 1 once we can go on with next request or end the session. * When in MSG_DATA or MSG_TRAILERS, it will automatically forward chunk_len - * bytes of pending data + the headers if not already done (between sol and sov). - * It eventually adjusts sol to match sov after the data in between have been sent. + * bytes of pending data + the headers if not already done. */ int http_response_forward_body(struct session *s, struct channel *res, int an_bit) { @@ -6195,7 +6188,6 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi */ channel_forward(res, msg->sov); msg->next = 0; - msg->sov = 0; if (msg->flags & HTTP_MSGF_TE_CHNK) msg->msg_state = HTTP_MSG_CHUNK_SIZE; @@ -6217,11 +6209,10 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi /* we may have some pending data starting at res->buf->p */ if (s->comp_algo == NULL) { - if (msg->chunk_len || msg->sov) { - msg->chunk_len += msg->sov; + if (msg->chunk_len || msg->next) { + msg->chunk_len += msg->next; msg->chunk_len -= channel_forward(res, msg->chunk_len); - msg->next -= msg->sov; - msg->sov = 0; + msg->next = 0; } } @@ -6266,13 +6257,12 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi if (compressing) { b_adv(res->buf, msg->next); msg->next = 0; - msg->sov = 0; } /* we're in MSG_CHUNK_SIZE now, fall through */ case HTTP_MSG_CHUNK_SIZE - HTTP_MSG_DATA: /* read the chunk size and assign it to ->chunk_len, then - * set ->sov and ->next to point to the body and switch to DATA or + * set ->next to point to the body and switch to DATA or * TRAILERS state. */ @@ -6289,7 +6279,6 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi /* skipping data if we are in compression mode */ b_adv(res->buf, msg->next); msg->next = 0; - msg->sov = 0; } else { if (consumed_data) { http_compression_buffer_end(s, &res->buf, &tmpbuf, 1); @@ -6380,11 +6369,10 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi /* forward any pending data starting at res->buf->p */ if (s->comp_algo == NULL) { - if (msg->chunk_len || msg->sov) { - msg->chunk_len += msg->sov; + if (msg->chunk_len || msg->next) { + msg->chunk_len += msg->next; msg->chunk_len -= channel_forward(res, msg->chunk_len); - msg->next -= msg->sov; - msg->sov = 0; + msg->next = 0; } } -- 2.47.2