Rolf Krahl added the comment: I disagree. I believe that the suggested modification of AbstractHTTPHandler.do_request_() belongs into this change set for the following reasons:
1. "This module [http.client] defines classes which implement the client side of the HTTP and HTTPS protocols. It is normally not used directly — the module urllib.request uses it to handle URLs that use HTTP and HTTPS." Quote from the http.client documentation. urllib.request is the high level API for HTTP requests. Both modules must fit together. Since urllib's HTTPHandler directly calls HTTPConnection, it can and should rely on the abilities of HTTPConnection. 2. The code in AbstractHTTPHandler is based on the assumption that each HTTP request having a non empty body must have a Content-length header set. The truth is that a HTTP request must either have a Content-length header or use chunked transfer encoding (and then must not have a Content-length header). As long as the underlying low level module did not support chunked transfer encoding anyway, this assumption might have been acceptable. Now that this change set introduces support for chunked transfer encoding, this assumption is plain wrong and the resulting code just faulty. 3. This change set introduces a sophisticated determination of the correct content length, covering several different cases, including file like objects and iterables. There is no need any more for the high level API to care about the content length, if this is already done in the low level method. But even worse, all the efforts of HTTPConnection to determine the proper content length is essentially overridden by the rather blunt method in the high level API that get priority and that essentially insists the body to be a buffer like object. This is strange. 4. The very purpose of this change set is to implement chunked transfer encoding. But this is essentially disabled by a high level API that insists a Content-length header to be set. This is plain silly. Just to illustrate the current situation, I attach the modified version of my old chunkedhttp.py module, adapted to Demian's patch that I have used to test it. It shows how a user would need to monkey patch the high level API in order to be able to use the features that is implemented by this change in the low level module. I wouldn't mind to file another issue against urllib.request.HTTPHandler if this makes things easier. But what I really would like to avoid, is to have any Python version in the wild that has this current issue fixed, but HTTPHandler still broken. Having to support a wide range of different Python versions is difficult enough for third party library authors like me. Adding a switch to distinguish between 3.6 (I can use the standard lib right away) and older (I need to replace it be my own old chunkedhttp implementation) is ok. But having to support a third case (the low level HTTPConnection module would work, but I need to monkey patch the high level API in order to be able to use it) in-between would make things awkward. That's why I would prefer to see the fix for HTTPHandler in the same change set. ---------- Added file: http://bugs.python.org/file39466/chunkedhttp-2.py _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue12319> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
