http.client ungracefully raises internal "TypeError: can't concat str to bytes" when dict is (incorrectly) submitted as payload

Bug report

Bug description:

import urllib.request

server = {
	'host' : 'example.com',
	'port' : 443
}
endpoint = '/api/v1/get_key'
data = {'payload': {'key': 'username'}}

print(f"Attempting to contact: https://{server['host']}:{server['port']}{endpoint}")
req = urllib.request.Request(f"https://{server['host']}:{server['port']}{endpoint}")
req.add_header('Accept', 'application/json; charset=utf-8')
req.add_header('Content-Type', 'application/json; charset=utf-8')
req.add_header('Content-Length', f"{len(data)}")

print(f"Sending: {data}")
response = urllib.request.urlopen(req, data)

Output:

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.

CPython versions tested on:

3.11

Operating systems tested on:

Linux

Linked PRs

gh-110462: Document that urllib.request does not serialize data #158106

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?

That would be slightly 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.

Would it not be possible to modify:

cpython/Lib/http/client.py

Lines 1109 to 1113 in def7ea5

    
           if encode_chunked and self._http_vsn == 11: 
        
               # chunked encoding 
        
               chunk = f'{len(chunk):X}\r\n'.encode('ascii') + chunk \ 
        
                   + b'\r\n' 
        
           self.send(chunk)

                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.

Navigation Menu

Repository navigation

http.client ungracefully raises internal "TypeError: can't concat str to bytes" when `dict` is (incorrectly) submitted as payload #110462

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	if encode_chunked and self._http_vsn == 11:
	# chunked encoding
	chunk = f'{len(chunk):X}\r\n'.encode('ascii') + chunk \
	+ b'\r\n'
	self.send(chunk)

http.client ungracefully raises internal "TypeError: can't concat str to bytes" when dict is (incorrectly) submitted as payload #110462

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

gaogaotiantian commented on Oct 6, 2023

Torxed commented on Oct 9, 2023

gaogaotiantian commented on Oct 9, 2023

Torxed commented on Oct 10, 2023

gaogaotiantian commented on Oct 11, 2023

Torxed commented on Oct 11, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

http.client ungracefully raises internal "TypeError: can't concat str to bytes" when `dict` is (incorrectly) submitted as payload #110462