You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
http.client ungracefully raises internal "TypeError: can't concat str to bytes" when dict is (incorrectly) submitted as payload #110462
Attempting to: https://example.com:443/api/v1/get_key
Sending: {'payload': {'key': 'username'}}
Traceback (most recent call last):
File "/home/anton/test.py", line 17, in <module>
response = urllib.request.urlopen(req, data)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 216, in urlopen
return opener.open(url, data, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 519, in open
response = self._open(req, data)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 536, in _open
result = self._call_chain(self.handle_open, protocol, protocol +
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 496, in _call_chain
result = func(*args)
^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 1391, in https_open
return self.do_open(http.client.HTTPSConnection, req,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 1348, in do_open
h.request(req.get_method(), req.selector, req.data, headers,
File "/usr/lib/python3.11/http/client.py", line 1286, in request
self._send_request(method, url, body, headers, encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1332, in _send_request
self.endheaders(body, encode_chunked=encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1281, in endheaders
self._send_output(message_body, encode_chunked=encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1078, in _send_output
chunk = f'{len(chunk):X}\r\n'.encode('ascii') + chunk \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
TypeError: can't concat str to bytes
I understand that urllib.request.urlopen will work given json.dumps(data).encode() instead.
What I consider a "bug" here is that the easy mistake to make gives a non-descriptive error which does not help the end user diagnose the issue.
I would propose to gracefully handle the error when an incompatible iterable data-type is submitted to urllib.request.urlopen.
I agree that this is not ideal. I don't think it's feasible to detect all "incompatible iterables" because the generators could be one time only.
I do think, however, we can add some sanity check in urllib.request.Request for data argument - specifically check against str and dict, which are probably the ones that users tend to use by mistake the most.
It'll still be an exception, maybe with a better explanation like TypeError: data takes bytes, file-like objects, and iterables of bytes-like objects, not {type}. However the exception will be raised from Request, not urlopen directly. Is this something you'd consider better?
A sanity check on data would be nice for sure, the fact that we would check the iterator and raise exception if it's not bytes - and point to the fact that the type inside the iterator is not compatible would be nice.
It's better than failing on concatenation internally :)
Like I said above - the issue is we can't check what's in the iterator without causing side effects. Not that it's completely impossible, but the outcome is definitely not worth the cost.
Checking against string and dict is simple, but a generic iterator could be one-pass only, and we simply can't check the internal data easily.
if encode_chunked and self._http_vsn == 11:
+ if not isinstance(chunk, bytes):+ raise ValueError(f"body of request must be of type bytes or iterator containing bytes types, not {type(message_body)} containing {type(chunk)}.")
# chunked encoding
chunk = f'{len(chunk):X}\r\n'.encode('ascii') + chunk \
+ b'\r\n'
self.send(chunk)
ValueError: body of request must be of type bytes or iterator containing bytes types, not <class 'dict'> containing <class 'str'>.
Or would that not catch all scenarios?
If it does, is the concern then performance impact?
You can, but that's not what you asked. I don't think the users would be much less (maybe a little bit?) confused when they saw a ValueError from http.client saying body of request has some problem - when they were using urllib.request.urlopen and passed no "body of request" that they knew of. It's almost as difficult to link this to the fact that the data they passed is wrong as the original exception.
Your propose is to handle this gracefully in urlopen, which I would support as that's the function the users actually use. It would be helpful to tell the users - this function you are using, the argument you passed in is wrong! But from a very deep stack, saying a variable that is derived from what users passed in (which most of users have no idea how) is problematic, is not that helpful.
Also, I do believe that a bytes-like object is not necessarily a subclass of bytes, so this might break existing code.
As you mentioned, due to design limitations put into place it's hard to evaluate the data earlier (without looping over the data twice).
Not sure I follow your logic in regards to urllib.request.urlopen. Sure I only did a quick glance, but the only time that I could find that this exception would be raised is if there was user supplied data parameter, with chunked encoding - where a chunk had the wrong type. So it would be a user-supplied data or request body (I'll leave phrasing to someone else, because I can't go into linguistic fights).
A caught error with a meaningful message - is better than incorrectly process user supplied data and letting it crash arbitrarily. I'll leave it here, as I get the impression that there's a better solution than what I quickly suggested in an attempt to help guide the work in a good direction.
Considering typeshed is typing it accordingly and that docs still mentions that we need an iterable of bytes-like objects (which dict of strings don't satisfy), I decided not to change the docs nor the runtime itself (changing it would possibly create breaking changes and it's not worth IMO)
Activity
gaogaotiantian commentedon Oct 6, 2023
I agree that this is not ideal. I don't think it's feasible to detect all "incompatible iterables" because the generators could be one time only.
I do think, however, we can add some sanity check in
urllib.request.Requestfordataargument - specifically check againststranddict, which are probably the ones that users tend to use by mistake the most.It'll still be an exception, maybe with a better explanation like
TypeError: data takes bytes, file-like objects, and iterables of bytes-like objects, not {type}. However the exception will be raised fromRequest, noturlopendirectly. Is this something you'd consider better?Torxed commentedon Oct 9, 2023
That would be slightly better.
A sanity check on
datawould be nice for sure, the fact that we would check the iterator and raise exception if it's not bytes - and point to the fact that the type inside the iterator is not compatible would be nice.It's better than failing on concatenation internally :)
gaogaotiantian commentedon Oct 9, 2023
Like I said above - the issue is we can't check what's in the iterator without causing side effects. Not that it's completely impossible, but the outcome is definitely not worth the cost.
Checking against string and dict is simple, but a generic iterator could be one-pass only, and we simply can't check the internal data easily.
Torxed commentedon Oct 10, 2023
Would it not be possible to modify:
cpython/Lib/http/client.py
Lines 1109 to 1113 in def7ea5
Or would that not catch all scenarios?
If it does, is the concern then performance impact?
gaogaotiantian commentedon Oct 11, 2023
You can, but that's not what you asked. I don't think the users would be much less (maybe a little bit?) confused when they saw a
ValueErrorfromhttp.clientsayingbody of requesthas some problem - when they were usingurllib.request.urlopenand passed no "body of request" that they knew of. It's almost as difficult to link this to the fact that thedatathey passed is wrong as the original exception.Your propose is to handle this gracefully in
urlopen, which I would support as that's the function the users actually use. It would be helpful to tell the users - this function you are using, the argument you passed in is wrong! But from a very deep stack, saying a variable that is derived from what users passed in (which most of users have no idea how) is problematic, is not that helpful.Also, I do believe that a bytes-like object is not necessarily a subclass of
bytes, so this might break existing code.Torxed commentedon Oct 11, 2023
As you mentioned, due to design limitations put into place it's hard to evaluate the data earlier (without looping over the data twice).
Not sure I follow your logic in regards to urllib.request.urlopen. Sure I only did a quick glance, but the only time that I could find that this exception would be raised is if there was user supplied
dataparameter, with chunked encoding - where a chunk had the wrong type. So it would be a user-supplieddataor request body (I'll leave phrasing to someone else, because I can't go into linguistic fights).A caught error with a meaningful message - is better than incorrectly process user supplied data and letting it crash arbitrarily. I'll leave it here, as I get the impression that there's a better solution than what I quickly suggested in an attempt to help guide the work in a good direction.
picnixz commentedon Oct 16, 2026
Considering typeshed is typing it accordingly and that docs still mentions that we need an iterable of bytes-like objects (which dict of strings don't satisfy), I decided not to change the docs nor the runtime itself (changing it would possibly create breaking changes and it's not worth IMO)