Downloading a file: the name goes into the header unescaped, so a quote breaks it and a line break answers 500 #376

New issue

Closed

opened 2026-10-01 10:46:14 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:14 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the file-serving helpers and running them with awkward names.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at documents/models.py:605-606 (an upload's download_name) and documents/models.py:714-716 (a rendered document's); core/files.py and the callers are where they were.

serve_private_file writes the download name into filename="…" after doing nothing to it but dropping the characters outside ASCII. A double quote in a document's title ends the quoted string early, so the header is malformed; a line break makes Django refuse the header, and the download answers 500. The document then cannot be downloaded at all until its title is changed. documents/views.py:61 (handed_over) already builds the same header with Django's content_disposition_header, which handles both, so the two ways of naming a download have drifted apart.

Where: core/files.py:89-96 (serve_private_file), fed document.download_name from documents/views.py:663 and :676 and api/routers/documents.py:155-161, which is the title as stored (documents/models.py:577-578 for uploads, :686-688 for rendered documents). core/files.py:144-149 (serve_private_text) builds the header the same way, but its only caller (jobs/page_views.py:116-117) passes a slugified stem, so it cannot be reached with a bad name today.
What happens:
1. A document titled CV "final" downloads with attachment; filename="CV "final".pdf"; filename*=UTF-8''CV%20%22final%22.pdf. Browsers that prefer filename* still name it correctly; a client that reads only filename gets a truncated or mangled name.
2. A title with an interior line break raises BadHeaderError: Header values can't contain newlines and the download (page or API) answers 500. forms.CharField.clean only strips the ends, so "CV\r\nfor Acme" is saved as it is from a hand-made POST, and core/importer.py:845 restores upload and sent-document titles from an archive without any check.
  Both reproduced against the snapshot; content_disposition_header(True, name) gives filename="CV \"final\".pdf" and filename*=utf-8''CV%0AAcme.pdf for the same inputs.
Why it matters: a person's own file becomes undownloadable through a title that the form or an import accepted, and the error they see says nothing about why.
Fix: build the header with django.utils.http.content_disposition_header(as_attachment, name) in both serve_private_file and serve_private_text, as handed_over does, so the three places cannot drift again. Optionally also strip control characters from download_name before use, and reject them in the document title fields.

Done when

tests/test_document_formats.py downloads a document whose title is CV "final" and one whose title contains \r\n, and asserts a 200 with a well-formed Content-Disposition (quotes escaped, no raw line break); the second fails today with BadHeaderError.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the file-serving helpers and running them with awkward names. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `documents/models.py:605-606` (an upload's `download_name`) and `documents/models.py:714-716` (a rendered document's); `core/files.py` and the callers are where they were. `serve_private_file` writes the download name into `filename="…"` after doing nothing to it but dropping the characters outside ASCII. A double quote in a document's title ends the quoted string early, so the header is malformed; a line break makes Django refuse the header, and the download answers 500. The document then cannot be downloaded at all until its title is changed. `documents/views.py:61` (`handed_over`) already builds the same header with Django's `content_disposition_header`, which handles both, so the two ways of naming a download have drifted apart. - **Where:** `core/files.py:89-96` (`serve_private_file`), fed `document.download_name` from `documents/views.py:663` and `:676` and `api/routers/documents.py:155-161`, which is the title as stored (`documents/models.py:577-578` for uploads, `:686-688` for rendered documents). `core/files.py:144-149` (`serve_private_text`) builds the header the same way, but its only caller (`jobs/page_views.py:116-117`) passes a slugified stem, so it cannot be reached with a bad name today. - **What happens:** 1. A document titled `CV "final"` downloads with `attachment; filename="CV "final".pdf"; filename*=UTF-8''CV%20%22final%22.pdf`. Browsers that prefer `filename*` still name it correctly; a client that reads only `filename` gets a truncated or mangled name. 2. A title with an interior line break raises `BadHeaderError: Header values can't contain newlines` and the download (page or API) answers 500. `forms.CharField.clean` only strips the ends, so `"CV\r\nfor Acme"` is saved as it is from a hand-made POST, and `core/importer.py:845` restores upload and sent-document titles from an archive without any check. Both reproduced against the snapshot; `content_disposition_header(True, name)` gives `filename="CV \"final\".pdf"` and `filename*=utf-8''CV%0AAcme.pdf` for the same inputs. - **Why it matters:** a person's own file becomes undownloadable through a title that the form or an import accepted, and the error they see says nothing about why. - **Fix:** build the header with `django.utils.http.content_disposition_header(as_attachment, name)` in both `serve_private_file` and `serve_private_text`, as `handed_over` does, so the three places cannot drift again. Optionally also strip control characters from `download_name` before use, and reject them in the document title fields. ## Done when - `tests/test_document_formats.py` downloads a document whose title is `CV "final"` and one whose title contains `\r\n`, and asserts a 200 with a well-formed `Content-Disposition` (quotes escaped, no raw line break); the second fails today with `BadHeaderError`.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:46:14 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:46:14 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 03:10:35 +00:00

Build every download's Content-Disposition with Django's helper

tiagoagueda closed this issue

2026-10-04 03:10:35 +00:00