ProxyHeadersMiddleware does not handle multiple values of X-Forwarded-Proto header #2310
Replies: 4 comments
-
|
|
Beta Was this translation helpful? Give feedback.
-
Beta Was this translation helpful? Give feedback.
-
|
Hello, We have a uvicorn server running with 2 upstream reverse proxies:
The first proxy does TLS offloading, thus changing protocol from https to http and setting the X-Forwarded-Proto / F-Forwarded-Port accordingly This behavior is standard for this kind of headers : you can edit the previous header to add values, or you can add another header with the same name (for X-forwarded-for see https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For#syntax ) So when reaching uvicorn, the headers are: It seems that in this case uvicorn uses the latest headers and believes that the original flow in http : when trying to connect to airflow through keycloak, when calling keycloak, airflow sets the redirect uri as http://myairflowapp instead of https://myairflowapp. |
Beta Was this translation helpful? Give feedback.
-
|
Hello. My bad, I had anticipated this issue and I meant to configure HAProxy with "IfNone" mode but for some reason this was not deployed on this environment |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
While the
X-Forwarded-*headers are not standardized, the RFC 7239 states the following about theForwarded-*headers, which are supposed to be a replacement for the former (see #2237):Meaning, the value cannot just be
httporhttps, but alsohttp,https.Currently
ProxyHeadersMiddlewarejust uses the value as is:https://github.com/encode/uvicorn/blob/0efd3835da6dcc713f74aadf7b52779d0d1fa17d/uvicorn/middleware/proxy_headers.py#L55-L58
When building a URL from this, i.e. by FastAPI / starlette, we are effectively doing
url = f"{scheme}://...".When trying to process this further,
starlette.datastructures.URLprovides thereplacemethod, which allows users to replace parts of the URL. Internally this relies onurllib.parse.urlsplitand the._replacemethod of its result. However, in turn, this doesn't recognize the scheme and puts all the information into the path and ultimately losing the information:Contrast this with the case of a valid scheme:
We should process the header and only select one of the options for the scheme. For example, here is how Tornado is doing it:
I'm happy to provide a patch for this if we deem this an issue to be fixed.
Beta Was this translation helpful? Give feedback.
All reactions