[issue25439] Add type checks to urllib.request.Request

Sanyam Khurana added the comment: PR is up for a review: https://github.com/python/cpython/pull/10616 -- ___ Python tracker ___

[issue25439] Add type checks to urllib.request.Request

Change by Sanyam Khurana : -- pull_requests: +9862 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue25439] Add type checks to urllib.request.Request

Sanyam Khurana added the comment: I'm working on applying the patch cleanly to master branch for this. Also adding 3.8 and 3.7 as candidates for the bug. -- versions: +Python 3.7, Python 3.8 ___ Python tracker

[issue25439] Add type checks to urllib.request.Request

Change by Sanyam Khurana : -- assignee: -> CuriousLearner nosy: +CuriousLearner ___ Python tracker ___ ___ Python-bugs-list

[issue25439] Add type checks to urllib.request.Request

Ezio Melotti added the comment: > But maybe I am being too nitpicky and paranoid. Ezio? IIUC your last example used to work because technically it is an iterable of bytes, but the patch would give an error instead. Even in the unlikely case someone is relying on this behavior, simply adding

[issue25439] Add type checks to urllib.request.Request

Nan Wu added the comment: Martin: Sorry for missing that line. Under https, byte iterable seems has not been supported: >>> r = Request('https://www.python.org', {b'post': 'data'}, >>> {'Content-Length':10}) >>> urlopen(r) ... hanging here... Meanwhile, I assumed bytearray works as expected.

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: This illustrates my concern with the dict-of-bytes check: >>> headers = {"Content-Length": "3"} >>> byteslike_iterable = {memoryview(b"123"): None} >>> urlopen(Request("http://python.org/;, headers=headers, >>> data=byteslike_iterable)) With the current patch

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: The bytes-like thing applies equally well to both the direct and iterable cases. Your first attempt hangs because the client only sends the four bytes in b"post", but the server is waiting for a total of 10 bytes. The SSL problem doesn’t show up unless you use

[issue25439] Add type checks to urllib.request.Request

Nan Wu added the comment: The do_request_() method which is defined in AbstractHTTPHandler seems not cover the check at least for the first case Ezio brought up. `unknown_open` has been called and gives out a relatively confusing message. -- ___

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: I meant removing one of the checks in AbstractHTTPHandler.do_request_(). I left a note in the review at the relevant line. One more thing I forgot to mention, although I am afraid it is 90% nit picking the dict iterable check. For the “http:” scheme, an

[issue25439] Add type checks to urllib.request.Request

Nan Wu added the comment: Will fix the other two issues. Thanks. -- ___ Python tracker ___ ___

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: There is already a check for request.data being str in do_request_(). Maybe it is okay to remove this check; I think it would be equivalent to the new check. Regarding the dict check, here is a strange class that currently works, but would be broken by the

[issue25439] Add type checks to urllib.request.Request

Changes by Nan Wu : Added file: http://bugs.python.org/file40850/urllib_request_param_type_check_3.patch ___ Python tracker ___

[issue25439] Add type checks to urllib.request.Request

Nan Wu added the comment: Uploaded a patch. Also update the test. Seems there are at least two old test cases using empty dict as data param. has dict been supported before? -- keywords: +patch nosy: +Nan Wu Added file:

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: The iterable option is documented under the urlopen(data=...) parameter, albeit not under the Request.data attribute. IMO this isn’t right, because you need to use Request to add Content-Length, but that is a separate documentation issue. Technically, an empty

[issue25439] Add type checks to urllib.request.Request

Nan Wu added the comment: I see. Empty dict is considered as iterable of bytes makes good sense. I just left the old tests there. And explicitly give warnings when data field is of type str or type dict with non-bytes key. -- Added file:

[issue25439] Add type checks to urllib.request.Request

Martin Panter added the comment: Type checking on the URL should be reasonably straightforward. For the request data, the question is what types do we currently support? This may be a can of worms. We would have to be careful not to break existing use cases, even undocumented ones. Looking

[issue25439] Add type checks to urllib.request.Request

Ezio Melotti added the comment: > For the request data, the question is what types do we currently support? This is what I was wondering as well. The doc says "data must be a bytes object specifying additional data to send to the server, or None if no such data is needed."[0] However one of

[issue25439] Add type checks to urllib.request.Request

New submission from Ezio Melotti: Currently urllib.request.Request seems to accept invalid types silently, only to fail later on with unhelpful errors when the request is passed to urlopen. This might cause users to go through something similar: >>> r = Request(b'https://www.python.org') >>>