Skip to content

tool_cb_wrt: fix no-clobber error handling#20939

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/no-clobber-fail
Closed

tool_cb_wrt: fix no-clobber error handling#20939
bagder wants to merge 1 commit intomasterfrom
bagder/no-clobber-fail

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 16, 2026

When saving a file with --no-clobber, make sure the existing file name remains set when creating the name fails. In a retry scenario, it comes back and uses that variable again.

Add test 3036 to verify.

Reported-by: James Fuller

When saving a file with --no-clobber, make sure the existing file name
remains set when creating the name fails. In a retry scenario, it comes
back and uses that variable again.

Add test 3036 to verify.

Reported-by: James Fuller
@github-actions github-actions bot added the tests label Mar 16, 2026
@bagder bagder marked this pull request as ready for review March 16, 2026 13:55
@bagder bagder requested a review from Copilot March 16, 2026 13:55
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 16, 2026

@aisle-analyzer

@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Mar 16, 2026

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

Aisle supplements but does not replace security review.


Analyzed PR: #20939 at commit 4ad107f

Last updated on: 2026-03-16T14:07:28Z

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

Fixes curl tool --no-clobber retry behavior so that a failed attempt to create an alternative output filename does not clobber the previously selected output filename, preventing subsequent retries from reusing a NULL/invalid filename.

Changes:

  • Update tool_create_output_file() to only overwrite outs->filename when a new, non-NULL numbered filename was actually produced.
  • Add new test case test3036 to exercise --no-clobber + --retry + --output-dir pointing to a file (non-directory) and verify the retry occurs.
  • Register test3036 in the test cases list.

Reviewed changes

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

File Description
src/tool_cb_wrt.c Prevents outs->filename from being overwritten with NULL when filename creation didn’t yield a valid new name.
tests/data/test3036 New regression test covering retry behavior when output file creation fails with --output-dir pointing to a file.
tests/data/Makefile.am Adds test3036 to the test suite list so it runs in CI.

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

You can also share your feedback on Copilot code review. Take the survey.

@testclutch
Copy link
Copy Markdown

Analysis of PR #20939 at 4ad107ff:

Test 809 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 (7, 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 29cb750 Mar 16, 2026
@bagder bagder deleted the bagder/no-clobber-fail branch March 16, 2026 14:32
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