Skip to content

fix(download): parse quoted and extended response filenames - #65

Merged
zhanghanduo merged 2 commits into
ApodexAI:mainfrom
KennyMcSimpson:fix/download-response-filenames
Oct 14, 2026
Merged

zhanghanduo merged 2 commits into
ApodexAI:mainfrom
KennyMcSimpson:fix/download-response-filenames

Conversation

@KennyMcSimpson

Copy link
Copy Markdown
Contributor

The download runner treats a response filename as a regular-expression match, which truncates quoted names such as filename="paper;v2.pdf". It also does not handle filename* correctly, so a server-provided Unicode filename can be lost or replaced by the fallback URL name.

Use the standard email header parser for quoted parameters and RFC 2231/5987 extended filenames. Prefer the extended filename when both forms are present, and leave literal percent sequences in ordinary filename values intact. The result still goes through the existing basename sanitization.

Validation:

  • pytest tests/test_download_filenames.py -q: 20 passed, covering quoted punctuation, extended filenames, parameter order, URL fallback, explicit-name precedence, and encoded path separators.
  • The CI Ruff command and the new test file pass lint.
  • Earlier full-suite run on this exact source tree: 4,798 passed, 5 skipped; the redirect-validation test fails because the container cannot resolve www.iana.org. That test and its fetch implementation are unchanged.
  • Pyright and import smoke passed. The external API preflight could not complete without a valid API key.

@KennyMcSimpson
KennyMcSimpson marked this pull request as ready for review October 8, 2026 11:32
Comment thread plugins/tools/_download_runner.py Outdated
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@zhanghanduo zhanghanduo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now

@zhanghanduo
zhanghanduo merged commit e4cb239 into ApodexAI:main Oct 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants