-
-
Notifications
You must be signed in to change notification settings - Fork 951
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
url_for only returns URLs with an autrority component #843
Comments
I don't quite agree with the assessment that it's not possible to determine the incoming host - proper setup of proxies & uvicorn should relay that information correctly. As an example Django REST framework uses absolute URLs if it's been configured to generate hyperlinked APIs, and we don't have a stack of backlogged issues with the behavior there. However I do agree that it'd be more resiliant to just generate URL paths as the default case. Potentially a bit of a breaking change so may need some thinking about. |
This is quite critical when setting up OAuth, since the third party checks the callback URL and complains if it's not a whitelisted one. On Flask I solve this by passing What I'm doing now to fix the protocol is redirect_uri = request.url_for('auth')
if 'X-Forwarded-Proto' in request.headers:
redirect_uri = redirect_uri.replace('http:', request.headers['X-Forwarded-Proto'] + ':') |
I think I have this problem too I'm deploying an app to fly.io, I get an <link rel="stylesheet" href="{{ url_for('admin:statics', path='css/main.css') }}"> ...which results in urls like It's bizarre to have the url scheme rewritten from https -> http I saw in #538 that the recommended fix is to specify both But it seems this requires one to know the IP address of the proxy in advance in order to trust it. I can't see how to do that on fly.io Alternatively I guess I will do that for this low risk demo app, but it feels like something that may allow for exploits (presumably a malicious client could spoof the hostname and inject their own js file of same name into my site?) I'd much prefer either:
are either of these possible at the moment? |
I have gone with |
Why not introduce an additional Anyhow, for now, I am just overriding templates.env.globals["url_for"] = app.url_path_for |
(Re-raising from #604, since that issue was only partially fixed)
TL;DR:
url_for
should behave like it does in Flask and render absolute paths by default (unless an optional authority parameter is passed), not complete URLs.Unlike Flask's
url_for
and Django'sHttpRequest.build_absolute_uri(location)
, Starlette'sRequest.url_for
always returns a complete URL with authority component. This tends to be unneccessarry when rendering templates that link to internal pages, but the important problem is that it also makes it so the app cannot render valid URLs unless it is aware of the authority the client is using to connect to it. Starlette uses theHost
header to determine what authority to prepend to the generated URLs, which is a routing header and is spec-mandated to be overriden by any intermediate clients so that the application is only aware of what hostname the final request was directed towards. This meansurl_for
is always going to break when Starlette is behind a proxy. Theport
andserver
values in the ASGI scope cannot differ from theHost
header either, as they are meant to mirror what WSGI and CGI do.Those are all meant to let the application reconstruct the URL of the request, but they can't be relied upon to build externally-valid URLs. Some proxies and application servers will send the
X-Forwarded-Host
orForward
headers to let the application know what authority the user agent was connecting from, but even the spec defining this header saysGunicorn removed the use of proxy headers several years ago when they realized overriding
REMOTE_ADDR
with the values from other headers was against the WSGI and CGI spec, because an application shouldn't have that sort of responsibility.You simply can't dynamically generate externally valid URLs with an authority component. You either have to know the authority you want to use in advance from a config file (usually for internal redirection, like pointing towards a CDN) or just omit them and let the user agent understand that it is meant to be sent to the same server it got the rendered URL from.
Important
The text was updated successfully, but these errors were encountered: