]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: Rework slightly commit 9962f8fc to clean code and avoid mistakes
authorChristopher Faulet <christopher.faulet@capflam.org>
Tue, 28 Jun 2016 13:54:44 +0000 (15:54 +0200)
committerWilly Tarreau <w@1wt.eu>
Tue, 28 Jun 2016 14:34:50 +0000 (16:34 +0200)
In commit 9962f8fc (BUG/MEDIUM: http: unbreak uri/header/url_param hashing), we
take care to update 'msg->sov' value when the parser changes to state
HTTP_MSG_DONE. This works when no filter is used. But, if a filter is used and
if it loops on 'http_end' callback, the following block is evaluated two times
consecutively:

    if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
            msg->sov -= ret;

Today, in practice, because this happens when all data are parsed and forwarded,
the second test always fails (after the first update, msg->sov is always lower
or equal to 0). But it is useless and error prone. So to avoid misunderstanding
the code has been slightly changed. Now, in all cases, we try to update msg->sov
only once per iteration.

No backport is needed.

src/proto_http.c

index ef5a15ccbc059f25389400f639952939defb9689..7d1b6787276dea08b29be928438fe094a61e8133 100644 (file)
@@ -6850,11 +6850,10 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg)
                               /* on_error    */ goto error);
        b_adv(chn->buf, ret);
        msg->next -= ret;
-       if (msg->next)
-               goto waiting;
-
        if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
                msg->sov -= ret;
+       if (msg->next)
+               goto waiting;
 
        FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg),
                         /* default_ret */ 1,
@@ -6870,12 +6869,11 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg)
                               /* on_error    */ goto error);
        b_adv(chn->buf, ret);
        msg->next -= ret;
-
-  waiting:
        if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0)
                msg->sov -= ret;
        if (!HAS_DATA_FILTERS(s, chn))
                msg->chunk_len -= channel_forward(chn, msg->chunk_len);
+  waiting:
        return 0;
   error:
        return -1;
@@ -6968,11 +6966,10 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
                          /* on_error    */ goto error);
        b_adv(chn->buf, ret);
        msg->next -= ret;
-       if (msg->next)
-               goto waiting;
-
        if (unlikely(!(chn->flags & CF_WROTE_DATA) || msg->sov > 0))
                msg->sov -= ret;
+       if (msg->next)
+               goto waiting;
 
        FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg),
                    /* default_ret */ 1,
@@ -6988,12 +6985,11 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg)
                          /* on_error    */ goto error);
        b_adv(chn->buf, ret);
        msg->next -= ret;
-
-  waiting:
        if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0)
                msg->sov -= ret;
        if (!HAS_DATA_FILTERS(s, chn))
                msg->chunk_len -= channel_forward(chn, msg->chunk_len);
+  waiting:
        return 0;
 
   chunk_parsing_error: