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
A middleware that adjusts request.host and request.scheme according to the Forwarded header (rfc7239) #175
Conversation
…ing to the Forwarded header as described by rfc7239.
I would argue for this to be under |
I realized over the weekend I need to catch the exception raised and return 400 Bad Request if it's raised. I also need to make available the WSGI version, not just the handler. So stand by. :) |
I now convert a parsing error of the Forwarded error into a 400 bad request (as per spec). I've added an actual WSGI middleware. I also made sure the coverage is 100%. We could instead use the wsgify.middleware decorator to create the middleware. In that case I'd still like to expose a function that turns this middleware back into the original |
assert_raises(forwarded.ForwardedError, | ||
forwarded.parse, 'for= _something') | ||
assert_raises(forwarded.ForwardedError, | ||
forwarded.parse, 'for=_something; by=192.51.100.17') |
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.
Are you sure that whitespaces are not allowed after a semicolon?
According to the RFC this header field is valid:
Forwarded: for=192.0.2.43, for="[2001:db8:cafe::17]", for=unknown
but are the following valid?
Forwarded: for=192.0.2.43 , for="[2001:db8:cafe::17]" , for=unknown
Forwarded: for=192.0.2.43 ,for="[2001:db8:cafe::17]" ,for=unknown
Forwarded: for=192.0.2.60; proto=http; by=203.0.113.43
Forwarded: for=192.0.2.60 ; proto=http ; by=203.0.113.43
Forwarded: for=192.0.2.60 ;proto=http ;by=203.0.113.43
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 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.
After reading the spec I'm not sure either, it's not very clear. I don't even know where to ask this.
@faassen |
This PR has been open for 6 years, so I felt it was time to close it. Feel
free to pursue it further, though!
…On Thu, Jul 1, 2021, 19:19 Piotr Dobrogost ***@***.***> wrote:
@faassen <https://github.com/faassen>
Close? What's the reason?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#175 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACP6MBBMFCD4DBDWJV5WGDTVSPSLANCNFSM4AX2GSZA>
.
|
As discussed on issue #174, implement the Forwarded header as a tween.
If you know you have a trusted proxy that sets the Forwarded header, you can use this Tween to affect request.host and request.scheme in WebOb (and thus all code that generates links based on this information).
This is a more secure alternative than doing it inside of WebOb as in pull request #160, as web applications should not accept the Forwarded header from untrusted sources as this opens them up to various attacks. In addition this tween also deals with the scheme bit, not just the host.
This does not support older forwarded headers like X_FORWARDED_HOST and X_FORWARDED_PROTO. We should do this in a similar tween along the same lines.