Open sm-Fifteen opened 4 years ago
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 _external=True
to url_for
, maybe it could be useful to have something like that in Starlette as well.
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 https://<myapp>.fly.dev
url. But the static files are not displaying and it seems to be because the templates use:
<link rel="stylesheet" href="{{ url_for('admin:statics', path='css/main.css') }}">
...which results in urls like http://<myapp>.fly.dev/statics/css/main.css
which fail.
It's bizarre to have the url scheme rewritten from https -> http
I saw in https://github.com/encode/starlette/issues/538 that the recommended fix is to specify both --proxy-headers
and --forwarded-allow-ips
flags to uvicorn
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 --forwarded-allow-ips
(or the uvicorn ProxyHeadersMiddleware
) accept "*"
wildcard to trust all X-Forwarded-For
headers
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:
https://<myapp>.fly.dev
hostname of my site manually via Starlette app config - this is a value I can know ahead of timeare either of these possible at the moment?
I have gone with --proxy-headers --forwarded-allow-ips=*
args to uvicorn and can confirm that it does work at least
Why not introduce an additional path_for
template function that returns the URL path instead of an absolute URL? That would be consistent with request.path
and wouldn't cause any backward incompatibilities.
Anyhow, for now, I am just overriding url_for
with app.url_path_for
like this:
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.