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.

diff --git a/src/backend.c b/src/backend.c
index 72c12d1c6ebe1314d96246ad0b2cb55ebccf2fdb..17e4b7fcbf75f3d0a426b3eedd27ab735bc3a79e 100644 (file)
--- 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 868083263cf92b7ac93d7ca592312f0cfc3796b6..0fd684692a8da3f0e61d26adab2288f19f2f7aaa 100644 (file)
--- 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 943439cbe4efdc04906f7abb157ee3af61b3cc88..1334579cb5d6fccd177f266dd6f76683fc1967f2 100644 (file)
--- 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 <ptr> 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;
                }
        }