Skip to content
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

feat(ndt7): Use X-Forwarded-For hint for the client's IP address #403

Conversation

auvik-bheesham
Copy link

@auvik-bheesham auvik-bheesham commented May 17, 2024

When running behind a proxy the RemoteAddr in various places won't return the real client's IP. This commit uses various hints to get it, falling back to using the default.

cc @pthankappan


This change is Reviewable

@auvik-bheesham auvik-bheesham force-pushed the use-forwarded-for-hint-of-client-ip branch from ca034c3 to 998b805 Compare May 17, 2024 18:20
When running behind a proxy the RemoteAddr in various places won't return the
real client's IP. This commit uses various hints to get it, falling back to
using the default.

Co-authored-by: Prabhath Thankappan <pthankappan@auvik.com>
@auvik-bheesham auvik-bheesham force-pushed the use-forwarded-for-hint-of-client-ip branch from 998b805 to 723c6fe Compare May 17, 2024 18:20
@auvik-bheesham auvik-bheesham changed the title Use X-Forwarded-For hint for the client's IP address feat(ndt7): Use X-Forwarded-For hint for the client's IP address May 17, 2024
@auvik-bheesham
Copy link
Author

Example:

; go run ./cmd/ndt7-client/main.go -service-url 'wss://speedtest.example.com/ndt/v7/upload'
upload in progress with speedtest.example.com
Avg. speed  :   114.6 Mbit/s
upload: complete

Test results

    Server: speedtest.example.com
    Client: 100.96.17.113

              Upload
     Throughput:   114.6 Mbit/s
        Latency:     0.6 ms

The Client: 100.96.17.113 is incorrect -- we're running the server in Kubernetes behind various proxies, so it's not quite returning the expected value.

@auvik-bheesham
Copy link
Author

auvik-bheesham commented May 19, 2024

Retracted for now since I noticed a couple of other places where RemoteAddr was being used.

In the process of being reviewed internally: auvik-bheesham@81df512

@auvik-bheesham
Copy link
Author

Updated MR here: #404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant