Skip to content

X-Forwarded-For loses entries when the header arrives as multiple lines #580

Description

@hoffmaen

Current behavior

Since v0.386.0, gorouter drops all but the first X-Forwarded-For value when a request
arrives with the header split across multiple header lines.

In #573 the reverse proxy was migrated from Director to Rewrite mode (Director is
deprecated in newer Go and was flagged by staticcheck). In Rewrite
mode httputil.ReverseProxy strips the X-Forwarded-* headers from the outgoing request,
so proxy.go rebuilds X-Forwarded-For by hand:

if prior := r.In.Header.Get("X-Forwarded-For"); prior != "" {
    r.Out.Header.Set("X-Forwarded-For", prior+", "+clientIP)
}

Header.Get only returns the first X-Forwarded-For header line. A request can
legitimately carry several — for example when a proxy in front of gorouter (in our case
HAProxy with option forwardfor) appends its own X-Forwarded-For line in addition to
one the client or an upstream hop already set. Every line after the first is silently
dropped, and the appended IP is often the one that mattered (the true client IP as seen
by the fronting proxy).

Example, same request before and after the upgrade:

before (v0.385.0): x-forwarded-for: 3.125.117.248, 52.58.41.143, 10.0.72.1
after  (v0.386.0+): x-forwarded-for: 3.125.117.248, 10.0.72.1

The 52.58.41.143 entry (added as a second header line) is gone. The old Director-based
code did not have this problem because ReverseProxy handled X-Forwarded-For itself and
folded all the inbound lines together.

Desired behavior

All inbound X-Forwarded-For header lines should be preserved, matching the pre-0.386.0
behavior. Joining r.In.Header.Values("X-Forwarded-For") instead of using Header.Get
restores this.

Affected Version

v0.386.0 through current develop (introduced in #573, unchanged as of v0.390.0).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions