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.

Sign up for GitHub

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

Jump to bottom

RFC-7239 "Forwarded" header not properly supported #70

Open
Waschnick opened this issue Sep 13, 2022 · 2 comments · May be fixed by #71
Open

RFC-7239 "Forwarded" header not properly supported #70

Waschnick opened this issue Sep 13, 2022 · 2 comments · May be fixed by #71

Comments

@Waschnick
Copy link

Waschnick commented Sep 13, 2022 •
edited
Loading

Current issue

  • The value of the Forwarded header is actually a complex object and not a simple IP, cf. RFC-7239
  • Example: for=123.34.567.89,for=192.0.2.43;by=[APIGW_IP];host=apiid.execute-api.us-east-1.amazonaws.com;proto=https
  • Example 2: for=94.134.90.17;host=public-api.example.org;proto=https (from our app logs)

Proposed solution

  • Write a function getClientIpFromForwarded similar to getClientIpFromXForwardedFor

Use Case

We are using AWS API Gateway with a private ALB (load balancer) and need the IP to use for getting the geo location. AWS API Gateway uses the Forwarded header for the client ip (see example 2). (And the ALB will set X-Forwarded-For with the private class-c IP from the ALB, but that's another issue)

I would also be open to contribute a PR with tests and the required changes. WDYT?

Sources
@Waschnick Waschnick changed the title RFC-7239 "forwarded" not properly supported RFC-7239 "Forwarded" header not properly supported Sep 13, 2022
@pbojinov
Copy link
Owner

@Waschnick thank you for posting this and for the detailed example.

I'd be open to accept a PR with tests for this as long as it's backwards compatible with the main function / we can expose a new public function getClientIpFromXForwardedFor

@martindeamorin martindeamorin linked a pull request Nov 4, 2022 that will close this issue
Open
@martindeamorin
Copy link

HI @pbojinov, @Waschnick! I created a PR to fix this issue, it would be cool if you could review it! You can fin it here: #71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add correct support for Forwarded header martindeamorin/request-ip
3 participants
@pbojinov @Waschnick @martindeamorin