Use reverse proxy headers (x-forwarded-proto, x-forwarded-path, ...) in url_for construction

[henryk]

Is your feature request related to a problem? Please describe. I'm running Sanic behind an nginx reverse proxy, mounted to a subdirectory. I want app.url_for(_external=True) to return correct absolute URLs. In my own code I have a helper function for that, but extensions (like the openapi code in sanic-ext) don't know about them.

Describe the solution you'd like The current documentation regarding proxy headers (https://github.com/sanic-org/sanic-guide/blob/927cebaf33394481810f0c50dac15898edd4ed67/src/en/guide/advanced/proxy-headers.md?plain=1#L48) suggests that the headers x-forwarded-proto, x-forwarded-host, x-forwarded-port, x-forwarded-path, and x-scheme could be used for URL construction.

Example: Suppose I have a route like

@app.route("/hi")
async def hello(request):
    return html('<a href="' + app.url_for('hello', _external=True) + '">here</a>')

I have app.config.REAL_IP_HEADER = 'X-Real-IP' and a request comes in as

GET /hi HTTP/1.0
X-Real-IP: 198.51.100.23
X-Forwarded-Host: example.com
X-Forwarded-Path: /api
X-Forwarded-Proto: https

I want it to return <a href="https://example.com/api/hi">here</a>

Additional context There is a slight problem here in that url_for() doesn't receive the request object. For reference, django-rest-api for example solves this by always passing in the request into Serializers (which might call reverse()). But even adding a _request doesn't solve the problem of existing naive code that's calling url_for() with no request object (and might not even have a request object reference).

Is there some sort of context variable for "the current request", like in Flask?

[sjsadowski]

Can you provide more context? Why wouldn't the following work?

external_host = request.headers.getone("X-Forwarded-Host", sanic.config.SERVER_NAME)
external_scheme = request.headers.getone("X-Forwarded-Proto", "http")
app.url_for('hello', _external=True, _scheme=external_scheme, _host=external_host)

It seems like the desire is url_for to be a bit more implicit in its approach, but it is currently explicit in nature. I think that it doesn't make a whole lot of sense to make a change.

[henryk]

So, the way I found this is that OpenAPI in sanic-ext doesn't work for me. It's here: https://github.com/sanic-org/sanic-ext/blob/50aa9782e37eb92cdef3db73e703ca6eb2c1ba1f/sanic_ext/extensions/openapi/extension.py#L38

I think that "figure out what the external URL of a route is, given some input and configuration" should be a centralized concept so that we're not getting independent re-implementations like the one in sanic_ext or the one you posted all over the place, each one with their own unique quirks and mistakes, depending on what edge-cases the individual author has thought of or not.

[henryk]

Ok, yeah, my real goal is getting external URL working for "weird" reverse proxy setups. Basically a config option "external base url" would suffice. I was mostly looking at the proxy headers because the documentation implied they would already do what I wanted. Looking through the code paths, I see that SERVER_NAME is mostly handled as a base url, including scheme, port, and path, even though the documentation doesn't explicitly promise this (and it's somewhat surprising, giving the variable name).

So, for example, this works:

app.config.SERVER_NAME = "https://example.com:2342/fnord"
assert app.url_for("hello", _external=True) == "https://example.com:2342/fnord/hi"

[Tronic]

To use runtime information such as proxy headers, you need request.url_for instead of app.url_for - this is the difference between the two. But setting up SERVER_NAME and relying on it is preferred if possible, as you seem to have done now.

[ahopkins]

@henryk is correct. There is some inconsistency here and there needs to be an overhaul to bring it all together. This is a bit of an effort in identifying some of this, but the bigger issue is the inherent breaking changes this will cause. While it is an open issue and has been for a while, I think that this should be tabled until after December to not introduce this break into the LTS.

Once this next release is out, assuming no one else has already taken up the problem, I can start to work on a solution for v23.