Repository navigation
Conversation
Verify in test 2805 Reported-by: Stanislav Fort
There was a problem hiding this comment.
🟢 Approval recommended
The core fix is narrowly scoped, aligns with existing escaping behavior, and is backed by a focused new regression test (only minor test-data clarity nits remain).
Pull request overview
This PR fixes FTP wildcard downloads by URL-encoding each matched file name when constructing subsequent transfer paths, preventing %xx sequences in literal file names from being misinterpreted during URL decoding. It adds a regression test (2805) that reproduces the issue with a file named some%2etxt.
Changes:
- URL-encode
finfo->filenamein the FTP wildcard state machine before appending it to the path. - Add new test case
test2805plus a newlib2805libtest driver. - Extend the fully simulated FTP directory listings with a
percentdirectory containingsome%2etxt.
File summaries
| File | Description |
|---|---|
| lib/ftp.c | URL-escapes wildcarded file names before rebuilding the per-file transfer path. |
| tests/data/test2805 | Adds regression coverage verifying correct RETR of a literal %-containing file name. |
| tests/libtest/lib2805.c | New libtest driver enabling wildcard matching and printing match callbacks for verification. |
| tests/libtest/Makefile.inc | Registers lib2805.c in the libtest build. |
| tests/directories.pm | Adds synthetic /fully_simulated/percent/ listing including some%2etxt. |
| tests/data/Makefile.am | Registers test2805 in the test suite. |
| tests/ftpserver.pl | Adds LIST-line logging when using the synthetic ftp content list. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Analysis of PR #22860 at 1e7701e4: Test ../../tests/http/test_02_download.py::TestDownload::test_02_07_download_reuse[h3] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Generated by Testclutch |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Verify in test 2805
Reported-by: Stanislav Fort