sourcegraph/internal/requestclient
Geoffrey Gilmore 7a3da57188
fix/internal/requestclient: read all instances of x-forwarded-for header, not just the first (#64137)
Closes https://linear.app/sourcegraph/issue/SRC-454/extract-and-propagate-user-ip-address-throughout-the-request-lifecycle

According to [HTTP1.1/RFC 2616](https://www.rfc-editor.org/rfc/rfc2616): Headers may be repeated, and any comma-separated list-headers (like `X-Forwarded-For`) should be treated as a single value.

In section 4.2:

>   Multiple message-header fields with the same field-name MAY bepresent 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.

For Example:

For the following HTTP request, it's valid to have multiple instances of x-forwarded-for:

| Header Name      | Header Value                |
|------------------|---------------------------|
| X-Forwarded-For  | 203.0.113.195, 70.41.3.18 |
| X-Forwarded-For  | 150.172.238.178           |
| X-Forwarded-For  | 123.45.67.89              |
| ... | ...|

That must be interpret-able as `X-Forwarded-For: 203.0.113.195, 70.41.3.18, 150.172.238.178, 123.45.67.89`

Previously, our code used http.Header.Get():

9e26623d90/internal/requestclient/http.go (L81-L95)

However, [func (Header) Get](https://pkg.go.dev/net/http#Header.Get) only returns the first value of the header:

> Get gets the first value associated with the given key. If there are no values associated with the key, Get returns "". ...

In our example, this means that our code would only get `X-Forwarded-For: 203.0.113.195, 70.41.3.18`, which is invalid according to RFC 2616.


## Test plan

There were no unit tests, so I added some.

## Changelog

<!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
2024-07-30 08:35:23 -07:00
..
geolocation Replace all traditional for-loops (#60988) 2024-03-11 16:05:47 +02:00
BUILD.bazel fix/internal/requestclient: read all instances of x-forwarded-for header, not just the first (#64137) 2024-07-30 08:35:23 -07:00
client_test.go requestclient: add country-level geolocation data (#58386) 2023-11-22 14:51:48 -08:00
client.go feat/requestclient: propagate original User-Agent as X-Forwarded-For-User-Agent (#64113) 2024-07-29 14:17:25 -07:00
grpc_test.go feat/requestclient: propagate original User-Agent as X-Forwarded-For-User-Agent (#64113) 2024-07-29 14:17:25 -07:00
grpc.go feat/requestclient: propagate original User-Agent as X-Forwarded-For-User-Agent (#64113) 2024-07-29 14:17:25 -07:00
http_test.go fix/internal/requestclient: read all instances of x-forwarded-for header, not just the first (#64137) 2024-07-30 08:35:23 -07:00
http.go fix/internal/requestclient: read all instances of x-forwarded-for header, not just the first (#64137) 2024-07-30 08:35:23 -07:00