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.
This commit is contained in:
Willy Tarreau 2014-04-17 15:21:20 +02:00
parent 0669d7dcf3
commit c24715e5f7
3 changed files with 36 additions and 50 deletions

View File

@ -299,12 +299,12 @@ struct server *get_server_ph_post(struct session *s)
struct proxy *px = s->be; struct proxy *px = s->be;
unsigned int plen = px->url_param_len; unsigned int plen = px->url_param_len;
unsigned long len = msg->body_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 *p = params;
const char *start, *end; const char *start, *end;
if (len > buffer_len(req->buf) - msg->sov) if (len > buffer_len(req->buf) - msg->sov - msg->sol)
len = buffer_len(req->buf) - msg->sov; len = buffer_len(req->buf) - msg->sov - msg->sol;
if (len == 0) if (len == 0)
return NULL; return NULL;

View File

@ -141,7 +141,6 @@ int http_compression_buffer_init(struct session *s, struct buffer *in, struct bu
*/ */
b_adv(in, msg->next); b_adv(in, msg->next);
msg->next = 0; msg->next = 0;
msg->sov = 0;
out->size = global.tune.bufsize; out->size = global.tune.bufsize;
out->i = 0; 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); b_adv(in, msg->next);
msg->next = 0; msg->next = 0;
msg->sov = 0;
/* /*
* select the smallest size between the announced chunk size, the input * select the smallest size between the announced chunk size, the input

View File

@ -1923,9 +1923,11 @@ void http_change_connection_header(struct http_txn *txn, struct http_msg *msg, i
return; return;
} }
/* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to the /* Parse the chunk size at msg->next. Once done, it adjusts ->next to point to
* first byte of body, and increments msg->sov by the number of bytes parsed. * the first byte of data after the chunk size, so that we know we can forward
* so that we know we can forward ->sov bytes. * 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. * 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. * 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 <ptr> points to the next byte, /* OK we found our CRLF and now <ptr> points to the next byte,
* which may or may not be present. We save that into ->next and * which may or may not be present. We save that into ->next,
* ->sov. * and the number of bytes parsed into msg->sol.
*/ */
msg->sol = ptr - ptr_old;
if (unlikely(ptr < ptr_old)) if (unlikely(ptr < ptr_old))
msg->sov += buf->size; msg->sol += buf->size;
msg->sov += ptr - ptr_old;
msg->next = buffer_count(buf, buf->p, ptr); msg->next = buffer_count(buf, buf->p, ptr);
msg->chunk_len = chunk; msg->chunk_len = chunk;
msg->body_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, * the trailers is found, it is automatically scheduled to be forwarded,
* msg->msg_state switches to HTTP_MSG_DONE, and the function returns >0. * 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 * 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 * except maybe msg->next if it could parse some lines, and returns zero.
* zero. If a parse error is encountered, the function returns < 0 and does not * 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 * change anything except maybe msg->next. Note that the message must
* must already be in HTTP_MSG_TRAILERS state before calling this function, * already be in HTTP_MSG_TRAILERS state before calling this function,
* which implies that all non-trailers data have already been scheduled for * which implies that all non-trailers data have already been scheduled for
* forwarding, and that msg->sov exactly matches the length of trailers already * forwarding, and that msg->next exactly matches the length of trailers
* parsed and not forwarded. It is also important to note that this function is * already parsed and not forwarded. It is also important to note that this
* designed to be able to parse wrapped headers at end of buffer. * function is designed to be able to parse wrapped headers at end of buffer.
*/ */
static int http_forward_trailers(struct http_msg *msg) 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) if (bytes < 0)
bytes += buf->size; 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)) { if (p1 == b_ptr(buf, msg->next)) {
/* LF/CRLF at beginning of line => end of trailers at p2. /* LF/CRLF at beginning of line => end of trailers at p2.
* Everything was scheduled for forwarding, there's nothing * 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 /* This function may be called only in HTTP_MSG_CHUNK_CRLF. It reads the CRLF
* a possible LF alone at the end of a chunk. It automatically adjusts msg->sov, * or a possible LF alone at the end of a chunk. It automatically adjusts
* and ->next in order to include this part into the next forwarding phase. * 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. * 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 * 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 * 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++; ptr++;
if (unlikely(ptr >= buf->data + buf->size)) if (unlikely(ptr >= buf->data + buf->size))
ptr = buf->data; ptr = buf->data;
/* prepare the CRLF to be forwarded (->sov) */ /* Advance ->next to allow the CRLF to be forwarded */
msg->sov += bytes;
msg->next += bytes; msg->next += bytes;
msg->msg_state = HTTP_MSG_CHUNK_SIZE; msg->msg_state = HTTP_MSG_CHUNK_SIZE;
return 1; 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 * 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. * 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 * 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). * bytes of pending data + the headers if not already done.
* It eventually adjusts sol to match sov after the data in between have been sent.
*/ */
int http_request_forward_body(struct session *s, struct channel *req, int an_bit) 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); http_silent_debug(__LINE__, s);
/* we may have some pending data starting at req->buf->p */ /* we may have some pending data starting at req->buf->p */
if (msg->chunk_len || msg->sov) { if (msg->chunk_len || msg->next) {
msg->chunk_len += msg->sov; msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(req, msg->chunk_len); msg->chunk_len -= channel_forward(req, msg->chunk_len);
msg->next -= msg->sov; msg->next = 0;
msg->sov = 0;
} }
if (msg->msg_state == HTTP_MSG_DATA) { 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) { else if (msg->msg_state == HTTP_MSG_CHUNK_SIZE) {
/* read the chunk size and assign it to ->chunk_len, then /* 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. * TRAILERS state.
*/ */
int ret = http_parse_chunk_size(msg); 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 * 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. * 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 * 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). * bytes of pending data + the headers if not already done.
* It eventually adjusts sol to match sov after the data in between have been sent.
*/ */
int http_response_forward_body(struct session *s, struct channel *res, int an_bit) 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); channel_forward(res, msg->sov);
msg->next = 0; msg->next = 0;
msg->sov = 0;
if (msg->flags & HTTP_MSGF_TE_CHNK) if (msg->flags & HTTP_MSGF_TE_CHNK)
msg->msg_state = HTTP_MSG_CHUNK_SIZE; 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 */ /* we may have some pending data starting at res->buf->p */
if (s->comp_algo == NULL) { if (s->comp_algo == NULL) {
if (msg->chunk_len || msg->sov) { if (msg->chunk_len || msg->next) {
msg->chunk_len += msg->sov; msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(res, msg->chunk_len); msg->chunk_len -= channel_forward(res, msg->chunk_len);
msg->next -= msg->sov; msg->next = 0;
msg->sov = 0;
} }
} }
@ -6266,13 +6257,12 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
if (compressing) { if (compressing) {
b_adv(res->buf, msg->next); b_adv(res->buf, msg->next);
msg->next = 0; msg->next = 0;
msg->sov = 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:
/* read the chunk size and assign it to ->chunk_len, then /* 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. * 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 */ /* skipping data if we are in compression mode */
b_adv(res->buf, msg->next); b_adv(res->buf, msg->next);
msg->next = 0; msg->next = 0;
msg->sov = 0;
} else { } else {
if (consumed_data) { if (consumed_data) {
http_compression_buffer_end(s, &res->buf, &tmpbuf, 1); 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 */ /* forward any pending data starting at res->buf->p */
if (s->comp_algo == NULL) { if (s->comp_algo == NULL) {
if (msg->chunk_len || msg->sov) { if (msg->chunk_len || msg->next) {
msg->chunk_len += msg->sov; msg->chunk_len += msg->next;
msg->chunk_len -= channel_forward(res, msg->chunk_len); msg->chunk_len -= channel_forward(res, msg->chunk_len);
msg->next -= msg->sov; msg->next = 0;
msg->sov = 0;
} }
} }