Repository navigation
[http.client] HTTPConnection.request not support "chunked" Transfer-Encoding to send data #56528
Description
| BPO | 12319 |
|---|---|
| Nosy | @orsenthil, @pitrou, @vstinner, @akheron, @vadmium, @eryksun, @demianbrecht, @matrixise, @RKrahl |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/vadmium'
closed_at = <Date 2016-08-27.04:12:55.150>
created_at = <Date 2011-06-12.10:47:13.084>
labels = ['type-feature', 'library']
title = '[http.client] HTTPConnection.request not support "chunked" Transfer-Encoding to send data'
updated_at = <Date 2016-08-27.04:12:55.149>
user = 'https://bugs.python.org/harobed'bugs.python.org fields:
activity = <Date 2016-08-27.04:12:55.149>
actor = 'martin.panter'
assignee = 'martin.panter'
closed = True
closed_date = <Date 2016-08-27.04:12:55.150>
closer = 'martin.panter'
components = ['Library (Lib)']
creation = <Date 2011-06-12.10:47:13.084>
creator = 'harobed'
dependencies = []
files = ['36493', '38351', '38352', '38528', '38670', '38772', '38797', '39447', '39466', '43398', '43579', '43973', '44057', '44076', '44133', '44217']
hgrepos = []
issue_num = 12319
keywords = ['patch', 'needs review']
message_count = 63.0
messages = ['138203', '138242', '138258', '138296', '138357', '138691', '171268', '226012', '226018', '226024', '236024', '236037', '237309', '237312', '237313', '237427', '237509', '238314', '239119', '239151', '239769', '239771', '239793', '243518', '243601', '243730', '243731', '243747', '243751', '243762', '243800', '243823', '243837', '243878', '268611', '268715', '268894', '269430', '269497', '271771', '271815', '272121', '272164', '272239', '272241', '272322', '272354', '272394', '272395', '272413', '272438', '272442', '272937', '273532', '273533', '273534', '273536', '273538', '273547', '273548', '273628', '273751', '273758']
nosy_count = 13.0
nosy_names = ['orsenthil', 'pitrou', 'vstinner', 'harobed', 'python-dev', 'petri.lehtinen', 'martin.panter', 'piotr.dobrogost', 'eryksun', 'demian.brecht', 'matrixise', 'whitemice', 'Rotkraut']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12319'
versions = ['Python 3.6']
Activity
harobed commentedon Jun 12, 2011
Hi,
HTTPConnection.putrequest not support "chunked" Transfer-Encodings to send data.
Exemple, I can't do PUT request with chunk transfert.
Regards,
Stephane
akheron commentedon Jun 13, 2011
What's the use case? Do you have an iterable that yields data whose size is unknown?
AFAIK, most web servers don't even support chunked uploads.
(Removing Python 2.7 from versions as this is clearly a feature request.)
harobed commentedon Jun 13, 2011
I use http.client in WebDAV client.
Mac OS X Finder WebDAV client perform all his request in "chunk" mode : PUT and GET.
Here, I use http.client to simulate Mac OS X Finder WebDAV client.
Regards,
Stephane
akheron commentedon Jun 14, 2011
harobed wrote:
Now I'm confused. Per the HTTP specification, GET requests don't have
a body, so "Transfer-Encoding: chunked" doesn't apply to them.
Are you sure you don't confuse with the response that the server
sends? In responses, "Transfer-Encoding: chunked" is very common.
harobed commentedon Jun 15, 2011
Sorry, yes GET requests have "Transfer-Encoding: chunked" in server response.
PUT requests can send body data in transfer-encoding chunked mode.
orsenthil commentedon Jun 20, 2011
We had support for chunked transfer encoding for POST method recently, which is exposed via urllib2 wrapper function. PUT is not exposed via urllib2 and users should use httplib. This feature of chunked transfer can be added to PUT by taking the body of the message as iterable.
pitrou commentedon Sep 25, 2012
I couldn't find what you're talking about.
If I look at AbstractHTTPHandler.do_request_, it actually mandates a Content-Length header for POST data (hence no chunked encoding).
RKrahl commentedon Aug 28, 2014
I'd like to support the request. I have a use case where I definitely need this feature: I maintain a Python client for a scientific metadata catalogue, see 1 for details. The client also features the upload of the data files. The files may come in as a data stream from another process, so my client takes a file like object as input. The files may be large (several GB), so buffering them is not an option, they must get streamed to the server as they come in. Therefore, there is have no way to know the Content-length of the upload beforehand.
I implemented chunked transfer encoding in a custom module that monkey patches the library, see the attached file. This works fine, but of course it's an awkward hack as I must meddle deeply into the internals of urllib and http.client to get this working. This module is tested to work with Python 2.7, 3.1, 3.2, 3.3, and 3.4 (for Python 3 you need to pass it through 2to3 first). I really would like to see this feature in the standard library in order to get rid of this hack in my package. I would happy to transform my module into a patch to the library if such a patch would have a chance to get accepted.
piotrdobrogost commentedon Aug 28, 2014
@rotkraut
The truth is http in stdlib is dead.
Your best option is to use 3rd party libs like requests or urllib3.
Authors of these libs plan to get rid of httplib entirely; see "Moving away from httplib" (urllib3/urllib3#58)
RKrahl commentedon Aug 28, 2014
Thanks for the notice! As far as I read from the link you cite, getting rid of the current httplib in urllib3 is planned but far from being done. Furthermore, I don't like packages with too many 3rd party dependencies. Since my package is working fine with the current standard lib, even though using an ugly hack in one place, I'd consider switching to urllib3 as soon as the latter makes it into the standard lib.
I still believe that adding chunked transfer encoding to http.client and urllib in the current standard lib would only require a rather small change that can easily be done such that the lib remains fully compatible with existing code. Still waiting for feedback if such a patch is welcome.
vadmium commentedon Feb 15, 2015
One interesting question is how to convey data to the chunked encoder. There are two sets of options in my mind, a pull interface:
and a push interface:
The advantage of the push interface is that you could fairly easily feed data from an iterable or file reader into it simply by just doing shutil.copyfileobj() or equivalent. But to adapt the pull interface to a push interface would require “asyncio” support or a separate thread or something to invert the flow of control. So I think building the encoder with a push interface would be best. Rolf’s ChunkedHTTPConnectionMixin class appears to only support the pull interface (iterables and stream readers).
I would welcome support for chunked uploading in Python’s “http.client” module, especially with push or stream writer support. I don’t think overwriting _send_request should be necessary; just call putrequest(), putheader() etc manually, and then call send() for each chunk. Perhaps there is scope for sharing the code with the “http.server” module (for encoding chunked responses).
RKrahl commentedon Feb 15, 2015
The design goal for my implementation was compatibility. My version can be used as a drop in replacement for the existing urllib's HTTPHandler. The only thing that need to be changed in the calling code is that it must call build_opener() to select the chunked handler in the place of the default HTTPHandler. After this, the calling code can use the returned opener in the very same way as usual.
I guess, switching to a push interface would require major changes in the calling code.
In principle, you could certainly support both schemes at the same time: you could change the internal design to a push interface and than wrap this by a pull interface for the compatibility with existing code. But I'm not sure whether this would be worth the effort. If, as Piotr suggests, the current urllib is going to be replaced by urllib3, then I guess, its questionable if it makes sense to add major design changes that are incompatible with existing code to the current standard lib.
45 remaining items
RKrahl commentedon Aug 11, 2016
Ok, here comes the next version of the patch. I made the changes discussed in the last post and addressed the review comments. Looks like we are converging towards a final version.
vadmium commentedon Aug 11, 2016
I am pretty happy with the latest patch. I left one comment. I will try to give it a more thorough review and actually test it out at some point, but I don’t anticipate any major problems.
vadmium commentedon Aug 17, 2016
Patch 12 has the following changes:
I think there are a few of my comments to Demian’s patches that are still relevant, but they are just opportunities for improving the code (e.g. making the chunking iteration more efficient and readable). This version is hopefully “good enough”.
python-dev commentedon Aug 24, 2016
New changeset 0effb34f3276 by Martin Panter in branch 'default':
Issue bpo-12319: Support for chunked encoding of HTTP request bodies
https://hg.python.org/cpython/rev/0effb34f3276
orsenthil commentedon Aug 24, 2016
Thank you and great work everyone involved with this change. Regards.
vadmium commentedon Aug 24, 2016
Rolf, just a note that I had to remove some trailing spaces on various continued lines in the Python code before it would let me push this.
Other tweaks I made:
The Windows buildbots fail the test sending a pipe with urlopen():
http://buildbot.python.org/all/builders/AMD64%20Windows8.1%20Non-Debug%203.x/builds/1166/steps/test/logs/stdio
======================================================================
FAIL: test_http_body_pipe (test.test_urllib2.HandlerTests)
----------------------------------------------------------------------
I cannot figure out why exactly. My experiments with Wine suggest that tell() on a BufferedReader wrapping a pipe raises OSError, so I would expect _get_content_length() to return None, yet the test failure shows that Content-Length is set to zero.
Is anyone able to investigate this on Windows? Will tell() and seek() reliably fail on a pipe or other unseekable file?
python-dev commentedon Aug 24, 2016
New changeset b004da19b869 by Martin Panter in branch 'default':
Issue bpo-12319: Move NEWS under beta 1 heading
https://hg.python.org/cpython/rev/b004da19b869
RKrahl commentedon Aug 24, 2016
First of all, thanks all for the great work, in particular to you Martin for your patience!
Martin, on your changes: as mentioned earlier, I'm in holidays right now, so I didn't had a close look yet. But I tested my application and it works, so I guess I'm happy with it.
I don't use Windows, so I can't comment on the issue test_http_body_pipe, sorry.
eryksun commentedon Aug 24, 2016
No they will not reliably fail.
The file position is stored in the FILE_OBJECT CurrentByteOffset. This value can be queried and set using the WinAPI function SetFilePointerEx, which is implemented via the system calls NtQueryInformationFile and NtSetInformationFile with the information class FilePositionInformation.
For files opened in synchronous mode, which is the only mode that the CRT supports, there's a fast path in the I/O manager to bypass the I/O request packet (IRP) path that would normally call the device driver. This fast-path code trivially updates the CurrentByteOffset field, regardless of the file type. In synchronous mode, a seek will only fail if the file is opened without buffering and the new position isn't sector-aligned, as is documented for SetFilePointerEx and NtSetInformationFile: FilePositionInformation.
Prior to Windows 8, seek() and tell() will also fail for a console handle because it's not actually a kernel File handle that can be used with I/O system calls. OTOH, Windows 8+ uses the ConDrv device driver, so console handles trivially allow seek() and tell() to succeed, though the console itself ignores the CurrentByteOffset in the File object. Console screen-buffer files have a positionable cursor, which is similar but not quite the same.
A fun fact is that for files opened in asynchronous mode, for FilePositionInformation the I/O manager does a regular device driver call with an IRP_MJ_QUERY_INFORMATION or IRP_MJ_SET_INFORMATION packet. It probably does this instead of using its fast path because there's no inherent meaning to the file position in asynchronous mode. I played around with this a bit with aynchronous named pipes. For setting FilePositionInformation, the NamedPipe filesystem driver always returns STATUS_INVALID_PARAMETER. However, querying the file 'position' returns the number of unread bytes in the queue for the handle's end of the pipe. This can only be non-zero if the handle allows reading, which depends on whether the pipe was created as duplex, inbound, or outbound. Querying this requires the NT API because SetFilePointerEx always calls NtSetInformationFile and fails.
vadmium commentedon Aug 24, 2016
Thankyou Eryksun for the detailed explanation. Unfortunately, that means that uploading an unseekable file via urllib.request, or via http.client, isn’t going to work by default on Windows. I see a couple of workarounds with the current code:
But I never liked how so much behaviour depends on specific details of the upload body object anyway. Perhaps this is an opportunity to change the behaviour so that a file object always triggers chunked encoding. We could add a note to <https://docs.python.org/3.6/whatsnew/3.6.html#changes-in-the-python-api\> to warn that uploading files to HTTP 1.0 servers will now require Content-Length to be manually specified.
vadmium commentedon Aug 25, 2016
Here is my attempt to drop automatic Content-Length with files. I also dropped the special handling of zero-length iterables, added some tests for that, and removed some unused and misleading bits from other tests.
python-dev commentedon Aug 27, 2016
New changeset 216f5451c35a by Martin Panter in branch 'default':
Issue bpo-12319: Always send file request bodies using chunked encoding
https://hg.python.org/cpython/rev/216f5451c35a
vadmium commentedon Aug 27, 2016
I committed my patch, so now file bodies are chunk-encoded by default. The Windows buildbots pass the relevant tests again.
This does mean that old code that uploaded a file to a HTTP-1.0-only server, and relied on Python automatically setting Content-Length, will no longer work in 3.6. Let me know if you think this is a serious problem, and maybe we can find another solution.