-
Notifications
You must be signed in to change notification settings - Fork 617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix x-forwarded-for header processing for ws connections #860
Conversation
@leprechau could you take a look at this one? This rings of this old thread: |
proxy/http_headers.go
Outdated
r.Header.Set("X-Forwarded-For", remoteIP) | ||
targetHeader := []string{remoteIP} | ||
sourceHeader := r.Header.Get("X-Forwarded-For") | ||
if sourceHeader != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some subtle things going on here. It would be good to mirror the functionality in the standard library:
https://github.com/golang/go/blob/master/src/net/http/httputil/reverseproxy.go#L296
The Get() call on the header only grabs the first element if there is more than one.
proxy/http_headers.go
Outdated
if sourceHeader != "" { | ||
targetHeader = append([]string{sourceHeader}, targetHeader...) | ||
} | ||
r.Header.Set("X-Forwarded-For", strings.Join(targetHeader, ",")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Down here, these should be joined by
", "
as it is done in the standdard library here:
https://github.com/golang/go/blob/master/src/net/http/httputil/reverseproxy.go#L303
Agreed, we should be trying to mirror the functionality in the standard library where possible. This also shouldn't have an impact on the ACL functionality. |
a55de9d
to
04f958c
Compare
…golang standard library code
@nathanejohnson / @leprechau can this be released in a |
Fix
x-forwarded-for
header processing logic for ws connections.Instead of simply use remoteIP as
x-forwarded-for
header for backend, server add remoteIP to the end ofx-forwarded-for
header from client and send result to backend.This PR possibly connected with issue #828