[PATCH] Fix for ticket #106: Correctly handle multiple X-Forwarded-For headers

[Tribble, Alex]

How do other people feel about this patch? I would love to see it get into
the
upstream, since I'm currently maintaining a local branch for it.

On 6/26/12 10:36 AM, "Tribble, Alex" <atribble at amazon.com> wrote:

>It looks good to me - thank you very much for doing this.
>
>Attached patch applies to the stable-1.2 branch.
>
>On 6/24/12 11:23 PM, "Ruslan Ermilov" <ru at nginx.com> wrote:
>
>>On Tue, Jun 19, 2012 at 06:47:56PM -0700, Tribble, Alex wrote:
>>> When nginx gets multiple X-Forwarded-For headers in a single request,
>>>it
>>> only keeps the last one in r->headers_in (and thus in
>>> $http_x_forwarded_for, $proxy_add_x_forwarded_for). Reverse proxies
>>>behind
>>> an nginx instance sometimes need the entire X-Forwarded-For chain -
>>>part
>>> of which is discarded in this case.
>>> 
>>> Per RFC 2616, it's equivalent to concatenate each header value
>>>(separated
>>> by a comma) and send the concatenated value to the upstream:
>>> 4.2
>>> -snip-
>>>    Multiple message-header fields with the same field-name MAY be
>>>    present in a message if and only if the entire field-value for that
>>>    header field is defined as a comma-separated list [i.e., #(values)].
>>>    It MUST be possible to combine the multiple header fields into one
>>>    "field-name: field-value" pair, without changing the semantics of
>>>the
>>>    message, by appending each subsequent field-value to the first, each
>>>    separated by a comma. The order in which header fields with the same
>>>    field-name are received is therefore significant to the
>>>    interpretation of the combined field value, and thus a proxy MUST
>>>NOT
>>>    change the order of these field values when a message is forwarded.
>>> -snip-
>>> 
>>> Attached is a patch that does exactly this, in the case of multiple
>>>headers.
>>> Please let me know if you have any comments about this patch - I'm
>>>happy
>>> to make any changes you suggest.
>>> 
>>> Relevant bug report:
>>> http://trac.nginx.org/nginx/ticket/106
>>
>>How's this patch instead?
>>
>><snip>
>