Skip to content
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

Case Sensitive Headers in httpx #1067

Closed
2 tasks done
dhofstetter opened this issue Oct 7, 2020 · 4 comments
Closed
2 tasks done

Case Sensitive Headers in httpx #1067

dhofstetter opened this issue Oct 7, 2020 · 4 comments

Comments

@dhofstetter
Copy link

dhofstetter commented Oct 7, 2020

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

In the project httpx there has been a new version released. They propagate the request.headers now in a case sensitive manner.

Within starlette (or in my case fastapi) this lead to unwanted behaviour as I'm unable now to determine wether or not headers are present.

The problem is, that if you check within starlette or fastapi for a header being present, the comparison is expected to be performed as lowercase comparison:

    def __getitem__(self, key: str) -> str:
        get_header_key = key.lower().encode("latin-1")
        for header_key, header_value in self._list:
            if header_key == get_header_key:
                return header_value.decode("latin-1")
        raise KeyError(key)

The problem is that the fields are propagated using the scope param:

class Headers(typing.Mapping[str, str]):
    """
    An immutable, case-insensitive multidict.
    """

    def __init__(
        self,
        headers: typing.Mapping[str, str] = None,
        raw: typing.List[typing.Tuple[bytes, bytes]] = None,
        scope: Scope = None,
    ) -> None:
        self._list = []  # type: typing.List[typing.Tuple[bytes, bytes]]
        if headers is not None:
            assert raw is None, 'Cannot set both "headers" and "raw".'
            assert scope is None, 'Cannot set both "headers" and "scope".'
            self._list = [
                (key.lower().encode("latin-1"), value.encode("latin-1"))
                for key, value in headers.items()
            ]
        elif raw is not None:
            assert scope is None, 'Cannot set both "raw" and "scope".'
            self._list = raw
        elif scope is not None:
            self._list = scope["headers"]

so there is no mutation of the given variable. As scope['headers'] is going to be "request.headers.raw" from httpx (see _client.py within httpx package):

    async def _send_single_request(
        self, request: Request, timeout: Timeout
    ) -> Response:
        """
        Sends a single request, without handling any redirections.
        """
        transport = self._transport_for_url(request.url)
        timer = Timer()
        await timer.async_start()

        with map_exceptions(HTTPCORE_EXC_MAP, request=request):
            (status_code, headers, stream, ext,) = await transport.arequest(
                request.method.encode(),
                request.url.raw,
                headers=request.headers.raw,
                stream=request.stream,  # type: ignore
                ext={"timeout": timeout.as_dict()},
            )
         .........

there is now a problem when comparing Header values.

To reproduce

Expected behavior

the expected behaviour is, that starlette changes the Header class initialization in a way, that headers received get definetly lowercased. Or starlette should not use the headers.raw field.
So that request.headers contains only keys in lowercase and any checks for existing headers succeed.

Actual behavior

Now the headers field within the request within starlette contains header values that are stored lowercased. Therefore any check for headers existing may fail, except they are passed as lowercase values (which was the behaviour until httpx version 0.15.5)

I hope you can understand the problem.

p.s. probably this behaviour is special to my use case. I use httpx and it's AsyncClient to write unit tests against my starlette/fastapi application (I'm not sure if this changes the way how requests get handled).

BR Daniel

@tomchristie
Copy link
Member

Ah right, it is actually an httpx bug, in the ASGITransport class. Since 0.16. The ASGI spec mandates that headers in the scope must be lowercased, which is no longer the case. The ASGITransport class in httpx now needs to explicitly lowercase whatever headers are passed to it.

@dhofstetter
Copy link
Author

dhofstetter commented Oct 7, 2020

So I need to create an Issue within httpx?

Thanks for your immediate response

@johnanthonyowens
Copy link

I ran into this issue in exactly the same way -- using HTTPX to unittest FastAPI. Thanks for the thorough investigation, @dhofstetter!

@tomchristie
Copy link
Member

Closed via encode/httpx#1351 and released as httpx 0.16.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants