New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
httplib read status memory usage #51040
Comments
During writing some code I discovered some behaviour of httplib. When we My patch was written originally on python2.4, but I’ve tested it on |
I've also check patch against code in svn tree: |
Sumar, to get this moved forward could you please provide a unit test. |
Attached is a unit test which tests the issue. The given patch appears to fix the issue well. I think this should be taken as a security issue (even if a rather odd one) since a malicious http server could be set up in place of the normal one and crash any http python clients that connect to it. Eg: import httplib
h = httplib.HTTPConnection('localhost', 8888)
h.connect()
h.request('GET', '/')
r = h.getresponse() This should cause python to use up all the memory available. |
A py3k patch against revision 87228. |
First, I don't think the resource module needs to be used here. Second, I don't see why getcode() would return 200. If no valid response was received then some kind of error should certainly be raised, shouldn't it? |
By the way, looking at the code, readline() without any parameter is used all over http.client, so fixing only this one use case doesn't really make sense. |
That's true. Near the bottom of the code, it says: # The status-line parsing code calls readline(), which normally Limiting the length of the status line would break 0.9 responses so maybe this issue should be closed? |
Well, the HTTP 1.0 RFC was filed in 1996 and HTTP 1.1 is most commonly |
I just read the whole discussion and it seems that code was in place so that client can tolerant of a BAD HTTP 0.9 Server response. Given that bpo-10711 talks about removing HTTP/0.9 support (+1 to that), this issue will become obsolete. I too support removing HTTP/0.9. There are hardly any advantages in keeping it around. |
Well, removing 0.9 support doesn't make this obsolete, does it? |
On Thu, Dec 16, 2010 at 01:18:30PM +0000, Antoine Pitrou wrote:
It does. Doesn't it? Because I saw in your patch that you fall back on |
I don't think you understood the issue here. Calling readline() without |
On Thu, Dec 16, 2010 at 02:02:10PM +0000, Antoine Pitrou wrote:
Yeah, I seem to have misunderstood the issue. Even if the response wa
Both this need to be addressed by having a limit on reading. I thought readline() is being called only when parsing headers which |
Now that 0.9 client support has been removed, this can proceed (at least for 3.2). |
Here is a patch limiting line length everywhere in http.client, + tests (it also affects http.server since the header parsing routine is shared). |
In the morning, I had a comment on the patch wondering why read _MAXLENGH + 1 and then check for len of header > _MAXLENGH. Instead of just reading _MAXLENGH (and if the length matched rejecting). ( Looks like it did not go through). I think that either way is okay. I am taking the privilege of committing the patch. Fixed for py3k in 87373. So it is be available in the next beta. Shall merge the changes to other codelines. |
Partially backported in r87382 (3.1) and r87383 (2.7). Not everything could be merged in because of HTTP 0.9 support and (in 2.7) a slightly different architecture. Thank you. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: