Hi Stefan,
Thanks to all your tests and observations, I managed to spot the bug and to
fix it. The headers are linked in a list whose tail is known as hdr_idx->tail.
This pointer is used when adding new headers. Unfortunately the header removal
function did not update it when it removed the last header. The effect is that
when adding the x-forwarded-for header, it's added just after the previously
removed Connection header so it does not appear in the list when walked from
the beginning. It's really when you said that it broke when Connection was
the last header that I understood what to look for. Thanks !
All the tests are now OK. I'm attaching the patch I'm going to commit to
1.5 and 1.4 (not checked whether 1.3 is affected, but that's probable).
I just have a minor comment about what you said below :
> Expected behaviour:
> Case a) should not jump between servers. An empty x-forwarded-for header
> means that always the "same" header (an empty one) should be hashed, you
> should always end up on the same server.
This is not true in haproxy, and there is an explicit test for this. If you
try to hash on an empty or non-existing header or parameter, then it falls
back to round robin. This is very important because there are many conditions
where you want to optimize stickiness when the information is present, but
not send all visitors to the same random server if they don't have the info.
However, now with the fix you will observe that the same server is always
selected, because the header hashing is performed when selecting a server,
which is after adding the x-forwarded-for header. So the balance code always
sees your header and it's never empty.
Regards,
Willy
diff --git a/src/proto_http.c b/src/proto_http.c
index 2d3c711..7150816 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -601,6 +601,8 @@ int http_remove_header2(struct http_msg *msg, struct buffer
*buf,
idx->used--;
hdr->len = 0; /* unused entry */
idx->v[ctx->prev].next = idx->v[ctx->idx].next;
+ if (idx->tail == ctx->idx)
+ idx->tail = ctx->prev;
ctx->idx = ctx->prev; /* walk back to the end of previous
header */
ctx->line -= idx->v[ctx->idx].len + idx->v[cur_idx].cr + 1;
ctx->val = idx->v[ctx->idx].len; /* point to end of previous
header */
