BUG/MAJOR: fix regression on content-based hashing and http-send-name-header

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.
This commit is contained in:
Willy Tarreau 2012-05-18 22:12:14 +02:00
parent 13e66dad26
commit d1de8af362
2 changed files with 40 additions and 8 deletions

View 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:

View 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;
}