#32329: CSRF failure incorrectly reported on upload when there is a problem with
storage
--------------------------------------+------------------------------------
Reporter: IO | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: 3.1
Severity: Normal | Resolution:
Keywords: uploads csrf admin | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Tim McCurrach):
I think this is the offending code (`django.middleware.csrf` lines
295-304):
{{{
if request.method == "POST":
try:
request_csrf_token =
request.POST.get('csrfmiddlewaretoken', '')
except OSError:
# Handle a broken connection before we've completed
reading
# the POST data. process_view shouldn't raise any
# exceptions, so we'll ignore and serve the user a 403
# (assuming they're still listening, which they
probably
# aren't because of the error).
pass
}}}
I think what's happening is:
- `request.POST` isn't actually accessed until the csrf middleware.
- When it's accessed here, `request._load_post_and_files` is called.
- The actual error is raised during the creation of `NamedTemporaryFile`
(see traceback below)
- The error remains unhandled until it is caught in CSRF middleware, and a
403 is returned.
{{{
File ".../lib/python3.8/site-packages/django/core/files/uploadedfile.py",
line 63, in __init__
file = tempfile.NamedTemporaryFile(suffix='.upload' + ext,
dir=settings.FILE_UPLOAD_TEMP_DIR)
File
"/usr/local/Cellar/python@3.8/3.8.6/Frameworks/Python.framework/Versions/3.8/lib/python3.8/tempfile.py",
line 540, in NamedTemporaryFile
(fd, name) = _mkstemp_inner(dir, prefix, suffix, flags, output_type)
File
"/usr/local/Cellar/python@3.8/3.8.6/Frameworks/Python.framework/Versions/3.8/lib/python3.8/tempfile.py",
line 250, in _mkstemp_inner
fd = _os.open(file, flags, 0o600)
FileNotFoundError: [Errno 2] No such file or directory:
'media/tmp/tmpg5wy5s9f.upload.pages'
}}}
I'm happy to write a patch, but I'm not sure what the best solution is:
- Raise a 500?
- Raise a 400?
- Ignore the file? (probably not, since this would lead to pretty
confusing and hard to debug behaviour)
Either way, I think we should probably add a check that if
`FILE_UPLOAD_TEMP_DIR` is set then it also exists.
If we did add such a check, could we ignore handling `OSError`s in
`TemporaryUploadedFile.__init__`? My immediate feeling is that even if a
check would mitigate this particular error, there are probably other
errors that could occur here, and letting them bubble up to wherever they
happen to get caught is confusing and difficult to debug. As such, a
decision is needed as to how they should be handled.
--
Ticket URL: <https://code.djangoproject.com/ticket/32329#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/061.943df5fcaa4882b69275ac3e99dc28fa%40djangoproject.com.
