Opened 5 weeks ago
Closed 11 days ago
#36023 closed Bug (fixed)
Update content_disposition_header to handle control chars.
Reported by: | Alex Vandiver | Owned by: | Alex Vandiver |
---|---|---|---|
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: | no | UI/UX: | no |
Description
content_disposition_header does not correctly handle newlines and other control characters in values that are given to it.
See https://github.com/django/django/pull/18890 for a fix.
Change History (7)
comment:1 by , 5 weeks ago
Component: | Uncategorized → HTTP handling |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
Type: | Uncategorized → Bug |
Version: | 5.1 → dev |
follow-up: 4 comment:2 by , 4 weeks ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
Headers cannot contain newline characters and other control characters. The current code passes them through unchanged, and filenames should not be assumed to be free from them. For instance:
def index(request): filename = "foo\nbar.pdf" return HttpResponse( "Some PDF content", headers={"Content-Disposition": content_disposition_header(True, filename)}, )
...will produce:
Traceback (most recent call last): File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/core/handlers/exception.py", line 55, in inner response = get_response(request) File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/core/handlers/base.py", line 197, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "/home/zulipdev/newline-disposition/newlines/views.py", line 7, in index return HttpResponse( File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/http/response.py", line 374, in __init__ super().__init__(*args, **kwargs) File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/http/response.py", line 115, in __init__ self.headers = ResponseHeaders(headers) File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/http/response.py", line 41, in __init__ self[header] = value File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/http/response.py", line 87, in __setitem__ value = self._convert_to_charset(value, "latin-1", mime_encode=True) File "/srv/zulip-py3-venv/lib/python3.10/site-packages/django/http/response.py", line 61, in _convert_to_charset raise BadHeaderError( django.http.response.BadHeaderError: Header values can't contain newlines (got 'attachment; filename="foo\nbar.pdf"')
The PR switches that to correctly percent-encode them, so that they are can be passed as an HTTP header.
comment:3 by , 4 weeks ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:4 by , 4 weeks ago
Triage Stage: | Unreviewed → Accepted |
---|
Replying to Alex Vandiver:
filenames should not be assumed to be free from them.
I guess it is possible control characters might be in a filename but very rare. This is also mentioned here: https://datatracker.ietf.org/doc/html/rfc6266#section-4.3
Recipients SHOULD strip or replace character sequences that are known to cause confusion both in user interfaces and in filenames, such as control characters and leading and trailing whitespace.
comment:5 by , 4 weeks ago
We ran into this in the context of Zulip; it stores files uploaded by arbitrary web browsers, and attempts to serve the files back to them with the same filenames they were uploaded as. In our production deploy, we observe filenames with newlines on an infrequent but non-zero frequency.
I agree that non-newline control characters is a bit more of a "did you really mean that," but filtering them should be the job of the application calling the function. The function should either produce the requested output correctly, or throw an exception -- not corrupt the HTTP response. Given that it's a SHOULD, not a MUST, I feel it's reasonable to produce the requested output.
comment:6 by , 12 days ago
Triage Stage: | Accepted → Ready for checkin |
---|
Hello Alex, thank you for taking the time to create this report. I have seen the PR but I think this ticket need more background information about why this change is needed. What I mean is that the PR shows the *what*, but in order to accept the ticket, we need to understand the *why*.
I'll close as
needsinfo
but please reopen when you can provide a use case describing the need/issue being solved. A small Django project or a way to trigger the bug would also help. Thanks again!