From 86bf0e1de788b84cfe587aad5519e4b72e7cec93 Mon Sep 17 00:00:00 2001 From: KennySimpson <70093673+KennyMcSimpson@users.noreply.github.com> Date: Thu, 8 Oct 2026 03:22:41 +0800 Subject: [PATCH 1/2] fix(download): parse quoted and extended response filenames --- plugins/tools/_download_runner.py | 21 ++++++----- tests/test_download_filenames.py | 62 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 tests/test_download_filenames.py diff --git a/plugins/tools/_download_runner.py b/plugins/tools/_download_runner.py index 3e8ca44..5d0e7e9 100644 --- a/plugins/tools/_download_runner.py +++ b/plugins/tools/_download_runner.py @@ -5,6 +5,7 @@ import argparse import contextlib import email.message +import email.utils import fcntl import hashlib import http.client @@ -56,10 +57,6 @@ "image/", "text/", ) -_CONTENT_DISPOSITION_NAME_RE = re.compile( - r"""filename\*?=(?:UTF-8''|["']?)([^;"']+)""", - re.IGNORECASE, -) class DownloadError(RuntimeError): @@ -270,11 +267,17 @@ def _preflight( def _header_filename(headers: email.message.Message) -> str: - disposition = headers.get("Content-Disposition") or "" - match = _CONTENT_DISPOSITION_NAME_RE.search(disposition) - if not match: - return "" - return urllib.parse.unquote(match.group(1)).strip() + fallback = "" + for name, value in headers.get_params(header="content-disposition", failobj=[]): + if name != "filename": + continue + # The parser represents filename* as an RFC 2231 tuple. Prefer it + # over the ordinary filename, regardless of parameter order. + if isinstance(value, tuple): + return email.utils.collapse_rfc2231_value(value).strip() + if not fallback: + fallback = value.strip() + return fallback def _safe_filename(candidate: str) -> str: diff --git a/tests/test_download_filenames.py b/tests/test_download_filenames.py new file mode 100644 index 0000000..86d22ca --- /dev/null +++ b/tests/test_download_filenames.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +from email.message import Message + +import pytest + +from plugins.tools._download_runner import _header_filename, _resolve_basename + + +@pytest.mark.parametrize( + ("disposition", "expected"), + [ + ('attachment; filename="report.pdf"', "report.pdf"), + ('attachment; filename="report;final.pdf"', "report;final.pdf"), + ('attachment; filename="O\'Reilly.pdf"', "O'Reilly.pdf"), + ('attachment; filename = "report.pdf"', "report.pdf"), + ('attachment; filename="report%20draft.pdf"', "report%20draft.pdf"), + ("attachment; filename*=UTF-8''report%20final.pdf", "report final.pdf"), + ("attachment; filename*=UTF-8'en'report%20final.pdf", "report final.pdf"), + ("attachment; filename*=UTF-8''caf%C3%A9.pdf", "café.pdf"), + ("attachment; filename*=ISO-8859-1''caf%E9.pdf", "café.pdf"), + ( + 'attachment; filename="fallback.txt"; filename*=UTF-8\'\'actual.pdf', + "actual.pdf", + ), + ( + 'attachment; filename*=UTF-8\'\'actual.pdf; filename="fallback.txt"', + "actual.pdf", + ), + ("attachment", ""), + ], +) +def test_response_filename(disposition: str, expected: str) -> None: + headers = Message() + headers["Content-Disposition"] = disposition + + assert _header_filename(headers) == expected + + +@pytest.mark.parametrize( + ("path_arg", "disposition", "expected"), + [ + ("", 'attachment; filename="report;final.pdf"', "report_final.pdf"), + ("", 'attachment; filename="O\'Reilly.pdf"', "O_Reilly.pdf"), + ( + "", + 'attachment; filename="fallback.txt"; filename*=UTF-8\'\'actual.pdf', + "actual.pdf", + ), + ("", 'attachment; filename="../../outside.pdf"', "outside.pdf"), + ("", "attachment; filename*=UTF-8''..%2Foutside.pdf", "outside.pdf"), + ("", "attachment", "url name.pdf"), + ("", "", "url name.pdf"), + ("chosen.pdf", 'attachment; filename="report;final.pdf"', "chosen.pdf"), + ], +) +def test_download_basename(path_arg: str, disposition: str, expected: str) -> None: + headers = Message() + if disposition: + headers["Content-Disposition"] = disposition + + assert _resolve_basename(path_arg, "https://example.com/url%20name.pdf", headers) == expected From ac87e2a7a5dbc9193519ca285a7b020e4f39b148 Mon Sep 17 00:00:00 2001 From: Zhang Handuo Date: Wed, 14 Oct 2026 15:50:10 +0800 Subject: [PATCH 2/2] fix(download): fall back to filename when filename* decodes empty Co-Authored-By: Claude Opus 5.5 --- plugins/tools/_download_runner.py | 12 ++++++++---- tests/test_download_filenames.py | 4 ++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/plugins/tools/_download_runner.py b/plugins/tools/_download_runner.py index 5d0e7e9..e9f59d1 100644 --- a/plugins/tools/_download_runner.py +++ b/plugins/tools/_download_runner.py @@ -267,17 +267,21 @@ def _preflight( def _header_filename(headers: email.message.Message) -> str: + extended = "" fallback = "" for name, value in headers.get_params(header="content-disposition", failobj=[]): if name != "filename": continue # The parser represents filename* as an RFC 2231 tuple. Prefer it - # over the ordinary filename, regardless of parameter order. + # over the ordinary filename, regardless of parameter order, but only + # when it decodes to something usable: an empty filename* must not + # discard a valid ordinary filename. if isinstance(value, tuple): - return email.utils.collapse_rfc2231_value(value).strip() - if not fallback: + if not extended: + extended = email.utils.collapse_rfc2231_value(value).strip() + elif not fallback: fallback = value.strip() - return fallback + return extended or fallback def _safe_filename(candidate: str) -> str: diff --git a/tests/test_download_filenames.py b/tests/test_download_filenames.py index 86d22ca..bbedf59 100644 --- a/tests/test_download_filenames.py +++ b/tests/test_download_filenames.py @@ -27,6 +27,9 @@ 'attachment; filename*=UTF-8\'\'actual.pdf; filename="fallback.txt"', "actual.pdf", ), + ('attachment; filename="paper.pdf"; filename*=UTF-8\'\'', "paper.pdf"), + ("attachment; filename*=UTF-8''; filename=\"paper.pdf\"", "paper.pdf"), + ("attachment; filename*=UTF-8''%20%20; filename=\"paper.pdf\"", "paper.pdf"), ("attachment", ""), ], ) @@ -50,6 +53,7 @@ def test_response_filename(disposition: str, expected: str) -> None: ("", 'attachment; filename="../../outside.pdf"', "outside.pdf"), ("", "attachment; filename*=UTF-8''..%2Foutside.pdf", "outside.pdf"), ("", "attachment", "url name.pdf"), + ("", 'attachment; filename="paper.pdf"; filename*=UTF-8\'\'', "paper.pdf"), ("", "", "url name.pdf"), ("chosen.pdf", 'attachment; filename="report;final.pdf"', "chosen.pdf"), ],