ProxyHeadersMiddleware does not handle multiple values of X-Forwarded-Proto header

[pmeier]

While the X-Forwarded-* headers are not standardized, the RFC 7239 states the following about the Forwarded-* headers, which are supposed to be a replacement for the former (see #2237):

A proxy server that wants to add a new "Forwarded" header field value can either append it to the last existing "Forwarded" header field after a comma separator or add a new field at the end of the header block. (emphasis mine)

Meaning, the value cannot just be http or https, but also http,https.

Currently ProxyHeadersMiddleware just uses the value as is:

uvicorn/uvicorn/middleware/proxy_headers.py

Lines 55 to 58 in 0efd383

	if scope["type"] == "websocket":
	scope["scheme"] = x_forwarded_proto.replace("http", "ws")
	else:
	scope["scheme"] = x_forwarded_proto

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.URL provides the replace method, which allows users to replace parts of the URL. Internally this relies on urllib.parse.urlsplit and the ._replace method 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:

>>> components = urlsplit("http,https://github.com/encode/uvicorn")
>>> components
SplitResult(scheme='', netloc='', path='http,https://github.com/encode/uvicorn', query='', fragment='')
>>> components._replace(path="/encode/httpx")
SplitResult(scheme='', netloc='', path='/encode/httpx', query='', fragment='')

Contrast this with the case of a valid scheme:

>>> components = urlsplit("https://github.com/encode/uvicorn")
>>> components
SplitResult(scheme='https', netloc='github.com', path='/encode/uvicorn', query='', fragment='')
>>> components._replace(path="/encode/httpx")
SplitResult(scheme='https', netloc='github.com', path='/encode/httpx', query='', fragment='')

We should process the header and only select one of the options for the scheme. For example, here is how Tornado is doing it:

if proto_header:
  # use only the last proto entry if there is more than one
  # TODO: support trusting multiple layers of proxied protocol
  proto_header = proto_header.split(",")[-1].strip()

I'm happy to provide a patch for this if we deem this an issue to be fixed.