ClientIP from RemoteIPHeaders calculation should concatenate all header values #4468
Description
Description
As of now, the ClientIP function parses each header in c.engine.RemoteIPHeaders by calling c.requestHeader(headerName), which calls c.Request.Header.Get(key). The official Golang docs state that (h Header) Get(key string) string "gets the first value associated with the given key" (https://pkg.go.dev/net/http#Header.Get).
However, consider this scenario with multiple X-Forwarded-For headers being set:
X-Forwarded-For: 1.2.3.4, 127.0.0.1
X-Forwarded-For: 5.6.7.8
Let 127.0.0.1 be the only trusted proxy. As of now, Gin would take the first header value, X-Forwarded-For: 1.2.3.4, 127.0.0.1, identify 127.0.0.1 as trusted proxy and would therefore return 1.2.3.4 as client IP.
However, citing heroku/roadmap#238, when a request with multiple X-Forwarded-For headers is received by the router/proxy it should handle the values as one continuous list, as specified by the HTTP spec:
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.
Thus, the above headers should be treated equivalently to:
X-Forwarded-For: 1.2.3.4, 127.0.0.1, 5.6.7.8
In which case Gin would return 5.6.7.8 as client IP.
Hence, to properly handle multiple X-Forwarded-For header values (or any other header name specified in RemoteIPHeaders), Gin should first join all header values by a comma (e.g. using strings.Join(c.Request.Header.Values(key), ",") instead of only looking at the first header value.
Gin Version
master
Can you reproduce the bug?
No
Source Code
No response
Go Version
No response
Operating System
No response