Opened 2 years ago

Closed 8 months ago

#20128 closed Bug (fixed)

csrfmiddleware sometimes raises IOError via _load_post_and_files

Reported by: wdoekes Owned by: nobody
Component: CSRF Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,

according the recently closed #12250, process_view middleware is not supposed to raise exceptions.

The documentation already states:
"""Accessing request.POST ... from process_request or process_view ... should normally be avoided."""
https://docs.djangoproject.com/en/dev/topics/http/middleware/#process-view

And here is another reason:

Sometimes the connection is reset/lost during HTTP request (during post/file data upload): an IOError is generated when read()ing from the data stream (from _load_post_and_files()).

Since the CsrfViewMiddleware attempts to get the csrfmiddlewaretoken from the POST data, the exception is propagated from there. And it's unhandleable, since we cannot catch exceptions from process_view().

The attached test simulates the situation by simply raising an IOError when an attempt to access POST is done. The attached patch silently ignores the IOError and treats it as if an empty csrfmiddleware token was supplied.

Regards,
Walter Doekes

P.S. Reproducing the IOError (for the generic non-csrf case) in the wild was trivial with apache2+mod_wsgi, but harder on nginx+uwsgi. For the former it was a matter of killing the netcat after feeding it a large-ish amount of multipart/form-data.

Attachments (1)

20128.patch (3.3 KB) - added by wdoekes 2 years ago.

Download all attachments as: .zip

Change History (3)

Changed 2 years ago by wdoekes

comment:1 Changed 2 years ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 8 months ago by Tim Graham <timograham@…>

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

In 815e7a57216b3e6ef716e924016acb09633ea8d1:

Fixed #20128 -- Made CsrfViewMiddleware ignore IOError when reading POST data.

Thanks Walter Doekes.

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