Opened 12 years ago
Closed 12 years ago
#18972 closed Bug (fixed)
Refactor bundled WSGI server's chuking logic
Reported by: | Owned by: | woodm | |
---|---|---|---|
Component: | HTTP handling | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
on core/servers/basehttp.py, lines 97-103:
if length > 33554432:
offset = 0
while offset < length:
chunk_size = min(33554432, length)
self._write(data[offset:offset+chunk_size])
self._flush()
offset += chunk_size
Since only offset is increased, chunksize will always be 33554432, and on the last write might not be the actual amount written. Probably
chunk_size = min(33554432, length-offset)
is better.
Attachments (1)
Change History (15)
comment:1 by , 12 years ago
Component: | Core (Other) → HTTP handling |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Cleanup/optimization → Bug |
Version: | 1.4 → master |
comment:2 by , 12 years ago
Has patch: | set |
---|
https://github.com/django/django/pull/415
./runtests.py --settings=test_sqlite
returns OK
comment:3 by , 12 years ago
Keywords: | sprint2013 added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:4 by , 12 years ago
Keywords: | sprint2013 removed |
---|---|
Owner: | removed |
Status: | assigned → new |
Patch provided above did not include new unit tests.
Note that although there is a bug here (the chunking math is wrong) it does not actually break anything. (Since x[a:b] where b>len(x) simply truncates and does not error.)
Rather than send this back marked "needs tests" I created a unit test: https://github.com/django/django/pull/780
I also created an alternative implementation of a bug fix that I suspect may be more efficient: https://github.com/django/django/pull/782 .
Since I contributed code, I cannot bump this to "Ready for Check-In."
I suggest previous patch contributor should resubmit their patch with unit tests, or just pull in the one I made which should be sufficient.
This should still be easy pickings, next owner should be able to simply review and pick which implementation they prefer.
comment:5 by , 12 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
I reviewed this patch. It looks GREAT Carney is a superstar's superstar. I owe this man so much. Thank you.
comment:7 by , 12 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
by , 12 years ago
Attachment: | 0001-Fixed-18972-Refactored-bundled-wsgi-server-s-chunkin.patch added |
---|
comment:12 by , 12 years ago
Summary: | calculation blunder → Refactor bundled WSGI server's chuking logic |
---|
Added a rebased patch with work from woodm, datagrok and dominik that passes on python 2 and 3.
Note that I refactored the test to assert two chunks were written.
comment:13 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
@carljm reviewed the ticket. I will commit the patch soon.
comment:14 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Since this was originally written in 2007 - it would be worth looking any improved approaches to this chunking using more recently available Python features.