Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#17277 closed New feature (fixed)

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

Reported by: dlowe Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: grahamd, carljm 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 dlowe 4 years ago.
django.unreadablepost.patch.2 (2.5 KB) - added by dlowe 3 years ago.

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by dlowe

comment:1 Changed 4 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 4 years ago by aaugustin

  • Easy pickings unset

comment:3 Changed 3 years ago by carljm

  • Patch needs improvement set
  • Triage Stage changed from Design decision needed to Accepted

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 3 years ago by dlowe

comment:4 Changed 3 years ago by dlowe

  • Patch needs improvement unset

New patch (based on r17492) is attached.

comment:5 Changed 3 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

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 3 years ago by viciu

  • Triage Stage changed from Accepted to Ready for checkin
Note: See TracTickets for help on using tickets.
Back to Top