Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17277 closed New feature (fixed)

make IOError exceptions during POST data read() easy to identify

Reported by: J. David Lowe Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: grahamd, Carl Meyer Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is related to #10046 and #17069. I'm wading into contentious territory, I know ;)

In short, I want to be able to suppress the IOError exceptions that are raised under common, but completely uncontrollable, client conditions e.g. connection abruptly closed. Like others, though, I find it unacceptable to completely suppress IOError exceptions, though -- it's a very broad exception class, and I don't want to hide real bugs.

The attached patch addresses this by catching IOErrors that occur while read()ing POST data, and re-throwing them as a new subclass of IOError.

IMO this would simplify the filtering logic for #17069, and would also enable a narrowly-targeted fix to #10046 (which was closed, essentially, because globally handling IOError isn't appropriate.)

Cc: grahamd and carljm, who are associated with the other two tickets.

Attachments (2)

django.unreadablepost.patch (2.6 KB) - added by J. David Lowe 5 years ago.
django.unreadablepost.patch.2 (2.5 KB) - added by J. David Lowe 5 years ago.

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by J. David Lowe

Attachment: django.unreadablepost.patch added

comment:1 Changed 5 years ago by Aymeric Augustin

Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

comment:3 Changed 5 years ago by Carl Meyer

Patch needs improvement: set
Triage Stage: Design decision neededAccepted

Thanks for the contribution! Sorry for the delay in moving it out of DDN. Approach and patch look reasonable to me. Patch no longer applies cleanly to trunk, needs updating.

Changed 5 years ago by J. David Lowe

comment:4 Changed 5 years ago by J. David Lowe

Patch needs improvement: unset

New patch (based on r17492) is attached.

comment:5 Changed 5 years ago by Carl Meyer

Resolution: fixed
Status: newclosed

In [17493]:

Fixed #17277 - Wrap IOErrors raised due to client disconnect in a specific IOError subclass so they can be distinguished from more serious errors. Thanks David Lowe.

comment:6 Changed 5 years ago by Wiktor

Triage Stage: AcceptedReady for checkin
Note: See TracTickets for help on using tickets.
Back to Top