Skip to content

Fix download filename truncation for multibyte characters - #1975

Open
nikhiljangid120 wants to merge 2 commits into
httpie:masterfrom
nikhiljangid120:fix/multibyte-download-filenames
Open

nikhiljangid120 wants to merge 2 commits into
httpie:masterfrom
nikhiljangid120:fix/multibyte-download-filenames

Conversation

@nikhiljangid120

Copy link
Copy Markdown

A multibyte download filename can fit the character count but exceed the POSIX filesystem's byte limit. On a filesystem with PC_NAME_MAX=255, 猫 repeated 85 times plus .txt has 89 characters but 259 bytes. An RFC 2231 Content-Disposition filename of that form currently raises OSError: [Errno 36] File name too long.

Measure POSIX filenames with os.fsencode, truncate only whole characters, and reserve space for the extension and collision suffix. Preserve the existing character-count fallback on Windows and the ASCII truncation behavior. This extends the filename-length handling introduced for #451; it does not change upload filename encoding or URL decoding.

Regression coverage includes 2-/3-byte characters, multibyte extensions, extension fallback, and actual creation/writing of two response-selected files with a collision suffix. Six new cases fail before the source fix. All 39 download tests pass afterward.

Validation on Linux, Python 3.13.14:

  • python -m pytest tests/test_downloads.py -q: 39 passed.
  • Full suite: 1,023 passed, 3 failed, 5 skipped, 4 xfailed. The same three failures reproduce in an unchanged baseline checkout (5b604c3): the inaccessible-config test under this privileged runner and two Big5 charset-detection cases. This is not a claim of a fully green suite.
  • Full configured flake8 httpie tests and git diff --check: passed.
  • Windows/macOS execution was not performed; the new POSIX-specific tests skip Windows.

AI disclosure: an AI coding assistant investigated, implemented, and ran the stated checks for this contribution. No human line-by-line review is claimed.

@nikhiljangid120

Copy link
Copy Markdown
Author

CI follow-up: checked the failing Linux, Windows, and coverage logs rather than treating the whole matrix as a filename regression.

  • The Python 3.12 Linux and Windows jobs report the CLI invalid-choice wording assertion and the two Big5 charset-detection assertions. All three also reproduce on unchanged upstream 5b604c37c6c67e18e7c3e9aee6c88a8c22b98345 with Python 3.12.15 on Linux: python -m pytest tests/test_cli_ui.py tests/test_encoding.py -q gives 3 failed / 54 passed. Python 3.13.14 reproduces the two Big5 failures (2 failed / 55 passed), matching the coverage job's failing cases.
  • The Python 3.7 Ubuntu job fails during setup because that interpreter is unavailable on Ubuntu 24.04.
  • The PR's download suite was additionally rerun on Python 3.12.15: all 39 tests pass (one ResourceWarning). Code-style and documentation checks are green.

Example logs: Linux 3.12, Windows 3.12, coverage, Python 3.7 setup.

Leaving the filename patch scoped rather than changing unrelated charset/argparse behavior or weakening tests to make the matrix green. This is not a claim that all CI passes. AI-assisted investigation and local validation; Windows execution was inspected in CI, not rerun locally.

@carlitogwapito

Copy link
Copy Markdown

Small Windows counterpart for the character-count fallback: test-only commit. It's one new test file, with no implementation changes.

The five cases cover non-ASCII names that fit, trimming before an extension, the long-extension fallback, a non-ASCII extension, and creating/readback of two collision-named files. Two useful guards: don't over-trim BMP text by counting UTF-8 bytes on Windows, and keep the collision suffix inside the same injected 10-character budget.

I ran the unchanged test bodies against the source-bound filename helpers on real Windows/Python 3.14.4. All five pass on both base and branch. An intentionally byte-count-only variant fails all five, so these detect loss of the fallback rather than a new Windows regression.

Focused Ruff checks pass. I haven't run full Flake8, the complete suite, or the HTTP download pipeline. This checks BMP filenames at the small test budget, not every Windows Unicode/path limit. Feel free to take whichever cases help.

Adopt the test-only cases offered on httpie#1975 from
carlitogwapito/cli commit fec8e28.
No production changes.

Co-authored-by: carlitogwapito <20962552+carlitogwapito@users.noreply.github.com>
@nikhiljangid120

Copy link
Copy Markdown
Author

Thanks @carlitogwapito — adopted all five cases from your test-only commit fec8e28 in 8ab1f20, with attribution in the commit message and a Co-authored-by trailer. No production changes.

Local validation on Linux / Python 3.12.15:

  • python -m pytest tests/test_downloads.py tests/test_windows_filename_fallback.py -q: 39 passed, 5 skipped (the Windows-only cases correctly skip here), with one ResourceWarning.
  • Ran the five unchanged test bodies in an isolated Windows-branch simulation: 5 passed. A byte-count-only mutation fails all five, confirming the fallback guards. This simulation is not native Windows execution; your native Windows result remains separately attributed to you.
  • Full configured flake8 httpie tests and git diff --check pass.

These are fallback-preservation controls, not five new regressions against upstream, and do not cover every Windows Unicode/path limit. The previously documented unrelated CI failures remain separate. AI-assisted review and validation.

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