Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(56480)

#26499: http.client.IncompleteRead from HTTPResponse read after part reading file

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 3 months ago by p.j.a.cock
Modified:
3 years, 3 months ago
Reviewers:
vadmium+py
CC:
krisvale, SilentGhost, p.j.a.cock_googlemail.com, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 1

Patch Set 4 #

Total comments: 2

Patch Set 5 #

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/http.client.rst View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
Lib/http/client.py View 1 2 3 4 5 3 chunks +8 lines, -0 lines 0 comments Download
Lib/test/test_httplib.py View 1 2 3 4 5 4 chunks +70 lines, -9 lines 0 comments Download
Misc/NEWS View 1 2 3 4 5 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 2
Martin Panter
https://bugs.python.org/review/26499/diff/16700/Lib/http/client.py File Lib/http/client.py (right): https://bugs.python.org/review/26499/diff/16700/Lib/http/client.py#newcode642 Lib/http/client.py:642: n = 0 What I had in mind was ...
3 years, 3 months ago #1
Martin Panter
3 years, 3 months ago #2
https://bugs.python.org/review/26499/diff/16731/Lib/test/test_httplib.py
File Lib/test/test_httplib.py (right):

https://bugs.python.org/review/26499/diff/16731/Lib/test/test_httplib.py#newc...
Lib/test/test_httplib.py:889: self.assertEqual(resp.read1(), expected*100)
IMO it would be wiser to just test say read1(3000), using an explicit oversized
argument. The reason is that read1(-1) isn’t well defined or widely supported.
See Issue 22359 and the funny ValueError handling in the implementation.

https://bugs.python.org/review/26499/diff/16731/Lib/test/test_httplib.py#newc...
Lib/test/test_httplib.py:901: self.assertEqual(resp.read(), expected*90)
Isn’t this really testing the same bug as your original test_mixed_reads() test?
If so, maybe test_mixed_reads() can go.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+