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
urrlib2/httplib doesn't reset file position between requests #49288
since 2.6 httplib supports reading from file-like objects.
Now consider the following situation:
There are two handlers in urrlib2, first is plain http, second is basic
auth.
I want to POST a file to a service, and pass the open file object as
data parameter to urllib2.urlopen.
First handler is invoked, it sends the file data, but gets 401
Unauthorized return code and fails with that.
Second handler in chain is invoked (at least that's how i understand
urrlib2, please correct me if i'm talking rubbish). At that point the
open file is at EOF, so empty data is sent.
furthermore, the obvious solution "you can't do this through urllib so
go read the file yourself" doesn't apply that well - the file object in
question is actually a mmap.mmap instance.
This code is in production since python 2.4. Until file object support
in httplib was introduced, it worked fine, handling the mmap'ed file as
a string. Now it is picked up as read()-able and this problem occurs.
Only workaround to restore pre-2.6 behavior that comes to mind is
building a wrapper class for the mmap object that hides its read() method.
I think, this requires triaging in terms of is the feature request still applicable. Except 100 is sent by httplib and the support for this was added few years ago, much later then this bug was originally raised.
Actually, I do not think any “Expect: 100-continue” headers are explicitly sent by the Python standard library. The Python client does not support waiting for a “100 Continue” response; see bpo-1346874.
There is bpo-23740 opened about fixing or clarifying the various data types accepted by “http.client”.
On the other hand, the documentation for urlopen() says only bytes and iterables are supported. If mmap objects are being treated as file objects by urlopen() that is unexpected, and the documentation or implementation needs fixing there. Also, iterating a mmap() object is different from iterating either the equivalent bytearray() or file object, so there is something weird going on there.
LorenzMende commented on Sep 2, 2018
LorenzMendemannequin
Mannequin
More actions
Issue shall be closed, as no reproduction code is provided.
No patch provided and no comments since 2015.
Here is a demonstration script in case it helps. I haven’t tested it with versions before Python 2.6.
Older versions send “Content-Length: 11”, but leave the server hanging trying to read the data. Newer versions (I presume since bpo-12319, 3.6+) send a valid HTTP 1.1 chunked request, but with empty data.
kctherookie commented on Feb 15, 2019
kctherookiemannequin
Mannequin
More actions
PR 11843 should fix the issue in master, I didn't check python 2.6 or prior versions. The problem is that in the first request sent to HTTP service the POST data is sent correctly. After that the HTTP server responds with 401 and the request is resent but the mmap file pointer is pointing now to the end of the file because it has been fully read in the requests before. The PR just seeks to the beginning of the file after the file has been read and sends the request with auth credentials including POST body.
For 3.7+ (where iterable objects are supported), I suggest:
Document the problem as a limitation of handlers like AbstractBasicAuthHandler, and consider raising an exception instead of trying to upload a file or iterable a second time.
Clarify the behaviour for different types of the “urllib.request” data parameter. I understand “file-like objects” means objects with a “read” attribute, and the “read” method is called in preference to iteration or treating the parameter as a “bytes” object.
Despite the bug title, I don’t think the library should mess with the file position. Certainly not when making a single request. But it should already be possible for the caller to supply a custom iterable object that resets the file position:
I added a new pull request.
Martin, you are right I realized when looking through the code that just setting the file pointer to zero inside http lib might interfere with requests that don't have authentication enabled.
The new pull requests does number 2.) of your suggestion for both Basic and Digest authentication.
Can you please review the code? Thank you.
kctherookie commented on Feb 17, 2019
kctherookiemannequin
Mannequin
More actions
I will fix the build errors first.
kctherookie commented on Feb 17, 2019
kctherookiemannequin
Mannequin
More actions
the pull request now passed the build checks, please review the code.
Activity
matejcik commentedon Jan 23, 2009
since 2.6 httplib supports reading from file-like objects.
Now consider the following situation:
There are two handlers in urrlib2, first is plain http, second is basic
auth.
I want to POST a file to a service, and pass the open file object as
data parameter to urllib2.urlopen.
First handler is invoked, it sends the file data, but gets 401
Unauthorized return code and fails with that.
Second handler in chain is invoked (at least that's how i understand
urrlib2, please correct me if i'm talking rubbish). At that point the
open file is at EOF, so empty data is sent.
furthermore, the obvious solution "you can't do this through urllib so
go read the file yourself" doesn't apply that well - the file object in
question is actually a mmap.mmap instance.
This code is in production since python 2.4. Until file object support
in httplib was introduced, it worked fine, handling the mmap'ed file as
a string. Now it is picked up as read()-able and this problem occurs.
Only workaround to restore pre-2.6 behavior that comes to mind is
building a wrapper class for the mmap object that hides its read() method.
ggenellina commentedon Jan 23, 2009
This happens in other implementations too, not just urllib2.
If the server supports it, the best way is to send an 'Expect: 100-
Continue' header field before attempting to send the actual file.
orsenthil commentedon Mar 29, 2013
I think, this requires triaging in terms of is the feature request still applicable. Except 100 is sent by httplib and the support for this was added few years ago, much later then this bug was originally raised.
vadmium commentedon Apr 16, 2015
Actually, I do not think any “Expect: 100-continue” headers are explicitly sent by the Python standard library. The Python client does not support waiting for a “100 Continue” response; see bpo-1346874.
There is bpo-23740 opened about fixing or clarifying the various data types accepted by “http.client”.
On the other hand, the documentation for urlopen() says only bytes and iterables are supported. If mmap objects are being treated as file objects by urlopen() that is unexpected, and the documentation or implementation needs fixing there. Also, iterating a mmap() object is different from iterating either the equivalent bytearray() or file object, so there is something weird going on there.
LorenzMende commentedon Sep 2, 2018
Issue shall be closed, as no reproduction code is provided.
No patch provided and no comments since 2015.
vadmium commentedon Sep 2, 2018
Here is a demonstration script in case it helps. I haven’t tested it with versions before Python 2.6.
Older versions send “Content-Length: 11”, but leave the server hanging trying to read the data. Newer versions (I presume since bpo-12319, 3.6+) send a valid HTTP 1.1 chunked request, but with empty data.
kctherookie commentedon Feb 15, 2019
PR 11843 should fix the issue in master, I didn't check python 2.6 or prior versions. The problem is that in the first request sent to HTTP service the POST data is sent correctly. After that the HTTP server responds with 401 and the request is resent but the mmap file pointer is pointing now to the end of the file because it has been fully read in the requests before. The PR just seeks to the beginning of the file after the file has been read and sends the request with auth credentials including POST body.
vadmium commentedon Feb 16, 2019
For 3.7+ (where iterable objects are supported), I suggest:
Document the problem as a limitation of handlers like AbstractBasicAuthHandler, and consider raising an exception instead of trying to upload a file or iterable a second time.
Clarify the behaviour for different types of the “urllib.request” data parameter. I understand “file-like objects” means objects with a “read” attribute, and the “read” method is called in preference to iteration or treating the parameter as a “bytes” object.
Despite the bug title, I don’t think the library should mess with the file position. Certainly not when making a single request. But it should already be possible for the caller to supply a custom iterable object that resets the file position:
kctherookie commentedon Feb 17, 2019
I added a new pull request.
Martin, you are right I realized when looking through the code that just setting the file pointer to zero inside http lib might interfere with requests that don't have authentication enabled.
The new pull requests does number 2.) of your suggestion for both Basic and Digest authentication.
Can you please review the code? Thank you.
kctherookie commentedon Feb 17, 2019
I will fix the build errors first.
kctherookie commentedon Feb 17, 2019
the pull request now passed the build checks, please review the code.
pythongh-49288: Reset file-like POST body on HTTP auth retry