Skip to content

tool_operhlp: iterate through all slashes to find name#21165

Closed
bagder wants to merge 2 commits intomasterfrom
bagder/name-slashes
Closed

tool_operhlp: iterate through all slashes to find name#21165
bagder wants to merge 2 commits intomasterfrom
bagder/name-slashes

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 30, 2026

If there is no trailing file name for -O or --remote-name-all, continue searching until there is no more to search. A URL ending with multiple slashes would previously make it do wrong.

Add test 1639 to verify.

Follow-up to e26eefd

Reported-by: James Fuller

If there is no trailing file name for -O or --remote-name-all, continue
searching until there is no more to search. A URL ending with multiple
slashes would previously make it do wrong.

Add test 1639 to verify.

Follow-up to e26eefd

Reported-by: James Fuller
@bagder bagder marked this pull request as ready for review March 30, 2026 21:37
@bagder bagder requested a review from Copilot March 30, 2026 21:37
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes filename extraction for -O / --remote-name-all when the URL path ends with multiple trailing slashes, ensuring curl keeps trimming slashes until a usable “name” is found (or falls back to curl_response). It also adds a regression test to lock in the corrected behavior.

Changes:

  • Update get_url_file_name() to iteratively strip trailing / (and \) until a non-empty name can be derived.
  • Add new test coverage for URLs that end with many trailing slashes and -O --output-dir.
  • Register the new test in the test manifest.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/data/test1639 New regression test for -O filename fallback with many trailing slashes
tests/data/Makefile.am Adds test1639 to the test list
src/tool_operhlp.c Adjusts URL filename extraction to handle multiple trailing separators

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder bagder force-pushed the bagder/name-slashes branch from 2fcdaa0 to 0e21fd3 Compare March 30, 2026 21:47
@testclutch
Copy link
Copy Markdown

Analysis of PR #21165 at 0e21fd3d:

Test 593 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 8 different CI jobs (the link just goes to one of them).

Test 2090 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be exact) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@bagder bagder closed this in 8e89646 Mar 31, 2026
@bagder bagder deleted the bagder/name-slashes branch March 31, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants