Opened 12 years ago

Closed 11 years ago

#18972 closed Bug (fixed)

Refactor bundled WSGI server's chuking logic

Reported by: amosonn@… 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)

0001-Fixed-18972-Refactored-bundled-wsgi-server-s-chunkin.patch (6.0 KB ) - added by Simon Charette 11 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 by Preston Holmes, 12 years ago

Component: Core (Other)HTTP handling
Triage Stage: UnreviewedAccepted
Type: Cleanup/optimizationBug
Version: 1.4master

Since this was originally written in 2007 - it would be worth looking any improved approaches to this chunking using more recently available Python features.

comment:2 by dominik, 11 years ago

Has patch: set

https://github.com/django/django/pull/415

./runtests.py --settings=test_sqlite
returns OK

comment:3 by Michael F. Lamb, 11 years ago

Keywords: sprint2013 added
Owner: changed from nobody to Michael F. Lamb
Status: newassigned

comment:4 by Michael F. Lamb, 11 years ago

Keywords: sprint2013 removed
Owner: Michael F. Lamb removed
Status: assignednew

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.

Last edited 11 years ago by Michael F. Lamb (previous) (diff)

comment:5 by woodm, 11 years ago

Owner: set to woodm
Status: newassigned

comment:6 by woodm, 11 years ago

Triage Stage: AcceptedReady for checkin

[REDACTED]

Wrong Ticket. :-(

Last edited 11 years ago by woodm (previous) (diff)

comment:7 by woodm, 11 years ago

Triage Stage: Ready for checkinAccepted

comment:9 by Simon Charette, 11 years ago

Reviewed the PR.

comment:10 by woodm, 11 years ago

Updated the PR to reflect charettes' suggestions.

comment:11 by woodm, 11 years ago

Heh, updated the PR to reflect Carl's suggestions as well.

comment:12 by Simon Charette, 11 years ago

Summary: calculation blunderRefactor 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 Simon Charette, 11 years ago

Triage Stage: AcceptedReady for checkin

@carljm reviewed the ticket. I will commit the patch soon.

comment:14 by Simon Charette <charette.s@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In a7960bcb3575fd9fcd5180986ddcdba1caa46dd5:

Fixed #18972 -- Refactored bundled wsgi server's chunking algorithm.

Thanks to amosonn at yahoo.com for the report, @doda for the initial patch and
@datagrok for the revamped logic and test case.

Note: See TracTickets for help on using tickets.
Back to Top