User-provided Content-Length header is missing from streaming requests #1168

Activity

1. lovelydinosaur commented on Aug 11, 2020

   

   lovelydinosaur

   on Aug 11, 2020

   Last edited by lovelydinosaur

   Member

   More actions

   Thanks for the report.

   You can see what's happening a bit more clearly with this...

   >>> c = httpx.Client()
   >>> c.build_request("POST", "https://www.example.org", headers={'Content-Length': '5'}, data=(i for i in b'abcde')).headers
   Headers({'host': 'www.example.org', 'user-agent': 'python-httpx/0.14.0', 'accept': '*/*', 'accept-encoding': 'gzip, deflate, br', 'connection': 'keep-alive', 'content-length': '5', 'transfer-encoding': 'chunked'})

   Our .build_request is including both the Transfer-Encoding: chunked that it expects from streaming data...

   httpx/httpx/_content_streams.py

   Line 86 in db4e417

   return {"Transfer-Encoding": "chunked"}

   While also including the content-length that you've set explicitly.

   Presumably the h11 library is then discounting the Content-Length. (Since it's invalid to set both Content-Length and Transfer-Encoding.)

   I think what we probably want is an explicit ignore case for setting Transfer-Encoding from the stream, if the Content-Length has been set explicitly. Probably something like changing this piece of code:

   httpx/httpx/_models.py

   Line 615 in db4e417

   for key, value in self.stream.get_headers().items():

   From...

   for key, value in self.stream.get_headers().items():
       self.headers.setdefault(key, value)

   To this...

   for key, value in self.stream.get_headers().items():
       if key.lower() == "transfer-encoding" and "content-length" in self.headers:
           continue
       self.headers.setdefault(key, value)

   Along with a test case to verify that a .build_request() operation like this... c.build_request("POST", "https://www.example.org", headers={'Content-Length': '5'}, data=(i for i in b'abcde')) results in the correct headers. (Ie. just the Content-Length one.)
2. lovelydinosaur

   added

   bugSomething isn't workingSomething isn't working

   good first issueGood for newcomersGood for newcomers

   on Aug 11, 2020
3. lovelydinosaur commented on Aug 11, 2020

   

   lovelydinosaur

   on Aug 11, 2020

   Member

   More actions

   Labelling this as a good first issue, since there's hopefully enough into in the comment for someone to be able to resolve this.
4. j178 commented on Aug 11, 2020

   

   j178

   on Aug 11, 2020

   Contributor

   More actions

   Run your scripts and running Wireshark, httpx actually contains the content-length header:
   
5. lovelydinosaur commented on Aug 11, 2020

   

   lovelydinosaur

   on Aug 11, 2020

   Member

   More actions

   @j178 Okay. It's possible that an intermediary could be stripping it before it gets to the server.
   Either way around the issue is valid, since we shouldn't be setting both headers.
6. j178 commented on Aug 11, 2020

   

   j178

   on Aug 11, 2020

   Contributor

   More actions

   Yes, I'll open a PR for this.

   👍React with 👍1Reacted by Kim Christie
7. vaultah commented on Aug 11, 2020

   

   vaultah

   on Aug 11, 2020

   Author

   More actions

   To clarify, I noticed this while trying to make requests to an API that checks the presence of the Content-Length header (B2 API, to be precise), so it's definitely not limited to httpbin. For example, if data is a bytes object, httpbin registers the header correctly.

   >>> r = await client.post('https://httpbin.org/post', data=b''.join([x async for x in f()]), headers={'content-length': '4'})
   >>> 'Content-Length' in r.json()['headers']
   True
8. j178

   closed this as completedin #1170on Aug 11, 2020