]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MAJOR: fix regression on content-based hashing and http-send-name-header
authorWilly Tarreau <w@1wt.eu>
Fri, 18 May 2012 20:12:14 +0000 (22:12 +0200)
committerWilly Tarreau <w@1wt.eu>
Fri, 18 May 2012 20:23:01 +0000 (22:23 +0200)
The recent split between the buffers and HTTP messages in 1.5-dev9 caused
a major trouble : in the past, we used to keep a pointer to HTTP data in the
buffer struct itself, which was the cause of most of the pain we had to deal
with buffers.

Now the two are split but we lost the information about the beginning of
the HTTP message once it's being forwarded. While it seems normal, it happens
that several parts of the code currently rely on this ability to inspect a
buffer containing old contents :
  - balance uri
  - balance url_param
  - balance url_param check_post
  - balance hdr()
  - balance rdp-cookie()
  - http-send-name-header

All these happen after the data are scheduled for being forwarded, which
also causes a server to be selected. So for a long time we've been relying
on supposedly sent data that we still had a pointer to.

Now that we don't have such a pointer anymore, we only have one possibility :
when we need to inspect such data, we have to rewind the buffer so that ->p
points to where it previously was. We're lucky, no data can leave the buffer
before it's being connecting outside, and since no inspection can begin until
it's empty, we know that the skipped data are exactly ->o. So we rewind the
buffer by ->o to get headers and advance it back by the same amount.

Proceeding this way is particularly important when dealing with chunked-
encoded requests, because the ->som and ->sov fields may be reused by the
chunk parser before the connection attempt is made, so we cannot rely on
them.

Also, we need to be able to come back after retries and redispatches, which
might change the size of the request if http-send-name-header is set. All of
this is accounted for by the output queue so in the end it does not look like
a bad solution.

No backport is needed.

src/backend.c
src/proto_http.c

index 3f30bea0485f1dc48c81b24230bb7f08aae15c92..b8bf7dd666aaf03d8115fe5cf0b86cb75b835e75 100644 (file)
@@ -30,6 +30,7 @@
 #include <proto/acl.h>
 #include <proto/arg.h>
 #include <proto/backend.h>
+#include <proto/buffers.h>
 #include <proto/frontend.h>
 #include <proto/lb_chash.h>
 #include <proto/lb_fas.h>
@@ -256,11 +257,11 @@ 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 = req->p + msg->sov;
+       const char      *params = b_ptr(req, (int)(msg->sov - req->o));
        const char      *p    = params;
 
-       if (len > req->i - (msg->sov - msg->som))
-               len = req->i - (msg->sov - msg->som);
+       if (len > buffer_len(req) - (msg->sov - msg->som))
+               len = buffer_len(req) - (msg->sov - msg->som);
 
        if (len == 0)
                return NULL;
@@ -342,7 +343,7 @@ struct server *get_server_hh(struct session *s)
        ctx.idx = 0;
 
        /* if the message is chunked, we skip the chunk size, but use the value as len */
-       http_find_header2(px->hh_name, plen, s->req->p + msg->sol, &txn->hdr_idx, &ctx);
+       http_find_header2(px->hh_name, plen, b_ptr(s->req, (int)(msg->sol - s->req->o)), &txn->hdr_idx, &ctx);
 
        /* if the header is not found or empty, let's fallback to round robin */
        if (!ctx.idx || !ctx.vlen)
@@ -405,6 +406,7 @@ struct server *get_server_rch(struct session *s)
        int              ret;
        struct sample    smp;
        struct arg       args[2];
+       int rewind;
 
        /* tot_weight appears to mean srv_count */
        if (px->lbprm.tot_weight == 0)
@@ -417,9 +419,13 @@ struct server *get_server_rch(struct session *s)
        args[0].data.str.len = px->hh_len;
        args[1].type = ARGT_STOP;
 
+       b_rew(s->req, rewind = s->req->o);
+
        ret = smp_fetch_rdp_cookie(px, s, NULL, SMP_OPT_DIR_REQ|SMP_OPT_FINAL, args, &smp);
        len = smp.data.str.len;
 
+       b_adv(s->req, rewind);
+
        if (ret == 0 || (smp.flags & SMP_F_MAY_CHANGE) || len == 0)
                return NULL;
 
@@ -563,7 +569,7 @@ int assign_server(struct session *s)
                                if (s->txn.req.msg_state < HTTP_MSG_BODY)
                                        break;
                                srv = get_server_uh(s->be,
-                                                   s->req->p + s->txn.req.sol + s->txn.req.sl.rq.u,
+                                                   b_ptr(s->req, (int)(s->txn.req.sol + s->txn.req.sl.rq.u - s->req->o)),
                                                    s->txn.req.sl.rq.u_l);
                                break;
 
@@ -573,7 +579,7 @@ int assign_server(struct session *s)
                                        break;
 
                                srv = get_server_ph(s->be,
-                                                   s->req->p + s->txn.req.sol + s->txn.req.sl.rq.u,
+                                                   b_ptr(s->req, (int)(s->txn.req.sol + s->txn.req.sl.rq.u - s->req->o)),
                                                    s->txn.req.sl.rq.u_l);
 
                                if (!srv && s->txn.meth == HTTP_METH_POST)
@@ -892,17 +898,20 @@ static void assign_tproxy_address(struct session *s)
                        if (srv->bind_hdr_occ) {
                                char *vptr;
                                int vlen;
+                               int rewind;
 
                                /* bind to the IP in a header */
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_family = AF_INET;
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_port = 0;
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_addr.s_addr = 0;
 
+                               b_rew(s->req, rewind = s->req->o);
                                if (http_get_hdr(&s->txn.req, srv->bind_hdr_name, srv->bind_hdr_len,
                                                 &s->txn.hdr_idx, srv->bind_hdr_occ, NULL, &vptr, &vlen)) {
                                        ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_addr.s_addr =
                                                htonl(inetaddr_host_lim(vptr, vptr + vlen));
                                }
+                               b_adv(s->req, rewind);
                        }
                        break;
                default:
@@ -923,17 +932,20 @@ static void assign_tproxy_address(struct session *s)
                        if (s->be->bind_hdr_occ) {
                                char *vptr;
                                int vlen;
+                               int rewind;
 
                                /* bind to the IP in a header */
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_family = AF_INET;
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_port = 0;
                                ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_addr.s_addr = 0;
 
+                               b_rew(s->req, rewind = s->req->o);
                                if (http_get_hdr(&s->txn.req, s->be->bind_hdr_name, s->be->bind_hdr_len,
                                                 &s->txn.hdr_idx, s->be->bind_hdr_occ, NULL, &vptr, &vlen)) {
                                        ((struct sockaddr_in *)&s->req->cons->addr.from)->sin_addr.s_addr =
                                                htonl(inetaddr_host_lim(vptr, vptr + vlen));
                                }
+                               b_adv(s->req, rewind);
                        }
                        break;
                default:
index 5250f2359c265e01607507d8323c7704a4540466..ccf0ab27e72396fcd3b56f04c670ab7bee67feed 100644 (file)
@@ -3634,18 +3634,30 @@ int http_process_request_body(struct session *s, struct buffer *req, int an_bit)
        return 0;
 }
 
-/* send a server's name with an outgoing request over an established connection */
+/* send a server's name with an outgoing request over an established connection.
+ * Note: this function is designed to be called once the request has been scheduled
+ * for being forwarded. This is the reason why it rewinds the buffer before
+ * proceeding.
+ */
 int http_send_name_header(struct http_txn *txn, struct proxy* be, const char* srv_name) {
 
        struct hdr_ctx ctx;
 
        char *hdr_name = be->server_id_hdr_name;
        int hdr_name_len = be->server_id_hdr_len;
-
+       struct buffer *req = txn->req.buf;
        char *hdr_val;
+       unsigned int old_o, old_i;
 
        ctx.idx = 0;
 
+       old_o = req->o;
+       if (old_o) {
+               /* The request was already skipped, let's restore it */
+               b_rew(req, old_o);
+       }
+
+       old_i = req->i;
        while (http_find_header2(hdr_name, hdr_name_len, txn->req.buf->p + txn->req.sol, &txn->hdr_idx, &ctx)) {
                /* remove any existing values from the header */
                http_remove_header2(&txn->req, &txn->hdr_idx, &ctx);
@@ -3660,6 +3672,14 @@ int http_send_name_header(struct http_txn *txn, struct proxy* be, const char* sr
        hdr_val += strlcpy2(hdr_val, srv_name, trash + trashlen - hdr_val);
        http_header_add_tail2(&txn->req, &txn->hdr_idx, trash, hdr_val - trash);
 
+       if (old_o) {
+               /* If this was a forwarded request, we must readjust the amount of
+                * data to be forwarded in order to take into account the size
+                * variations.
+                */
+               b_adv(req, old_o + req->i - old_i);
+       }
+
        return 0;
 }