tool_operhlp: fix `add_file_name_to_url()` result on OOM by vszakats · Pull Request #21011 · curl/curl

vszakats · 2026-03-19T14:56:29Z

Return CURLE_OUT_OF_MEMORY instead of CURLE_URL_MALFORMAT when
curl_url(), curl_easy_escape(), or curl_maprintf() calls failed.

Found by Codex Security

Also reuse deinit code from a success branch.

vszakats · 2026-03-19T17:39:49Z

augment review

augmentcode · 2026-03-19T17:41:28Z

🤖 Augment PR Summary

Summary: Fixes error reporting in add_file_name_to_url() so allocation failures from curl_url(), curl_easy_escape(), or curl_maprintf() surface as CURLE_OUT_OF_MEMORY rather than CURLE_URL_MALFORMAT.
Changes: Reuses the common cleanup path by replacing an early return with a jump to the shared deinit/return block.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

Copilot

Pull request overview

This PR updates the curl tool helper add_file_name_to_url() to report out-of-memory conditions accurately instead of incorrectly returning URL-format errors, while simplifying cleanup paths.

Changes:

Default add_file_name_to_url() failure result to CURLE_OUT_OF_MEMORY so OOM from curl_url(), curl_easy_escape(), or curl_maprintf() is surfaced correctly.
Refactor early-return success path to reuse a shared cleanup label (out:).

💡 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.

vszakats · 2026-03-19T18:18:54Z

    uerr = curl_url_get(uh, CURLUPART_QUERY, &query, 0);
    if(!uerr && query) {
      curl_free(query);
-      curl_free(path);
-      curl_url_cleanup(uh);
-      return CURLE_OK;
+      result = CURLE_OK;
+      goto out;
    }


@bagder what do you think of this suggestion? (a pre-existing issue, I'd
say it's justified, to bail out also in case of a lower-level OOM, i.e. char *part = curlx_memdup0(ptr, partlen);)

It seems entirely accurate, yes. It should bail out on memory problems here.

Had to back out from this, for it broke a lot of tests:
1b8a23f

Seems to be breaking a lot of tests:
https://github.com/curl/curl/actions/runs/23343096423/job/67901964263?pr=21011

FAIL 107: 'FTP PASV upload file' FTP, EPSV, STOR
FAIL 108: 'FTP PORT upload with CWD' FTP, PORT, STOR
FAIL 109: 'FTP PASV upload append' FTP, EPSV, APPE
FAIL 112: 'FTP PASV upload resume' FTP, EPSV, APPE, Resume
FAIL 123: 'FTP upload resume with whole file already downloaded' FTP, EPSV, Resume
FAIL 128: 'FTP upload with --crlf' FTP, EPSV, STOR, --crlf
FAIL 149: 'FTP with multiple uploads' FTP
FAIL 204: '"upload" with file://' FILE
FAIL 205: '"upload" nonexisting with file://' FILE, FAILURE
FAIL 216: 'FTP upload two files to the same dir' FTP
FAIL 235: 'FTP resumed upload but no file present remotely' FTP, STOR
FAIL 236: 'FTP resume upload but denied access to remote file' FTP
FAIL 247: 'FTP upload time condition evaluates TRUE => skip upload' FTP, MDTM
FAIL 248: 'FTP upload time condition evaluates FALSE => upload anyway' FTP, STOR, MDTM
FAIL 285: 'TFTP send' TFTP, TFTP WRQ
FAIL 286: 'TFTP send of boundary case 512 byte file' TFTP, TFTP WRQ
FAIL 289: 'FTP resume upload but denied access to local file' FTP, STOR, Resume, FAILURE
FAIL 348: 'FTP upload file with 552 disk full response' FTP, EPSV, STOR
FAIL 362: 'FTP resume upload file with nothing to start from' FTP, EPSV, STOR
FAIL 475: 'FTP PASV upload ASCII file' FTP, EPSV, STOR, TYPE A
FAIL 476: 'FTP PASV upload ASCII file already using CRLF' FTP, EPSV, STOR, TYPE A
FAIL 496: 'parallel upload missing file' curl tool, cmdline, parallel
FAIL 1007: 'TFTP send with invalid permission on server' TFTP, TFTP WRQ, FAILURE
FAIL 1038: 'FTP PASV upload resume from end of file' FTP, EPSV, APPE, Resume
FAIL 1039: 'FTP PASV upload resume from end of empty file' FTP, EPSV, APPE, Resume
FAIL 1243: 'TFTP send without TFTP options requests' TFTP, TFTP WRQ
FAIL 1291: 'Attempt to upload 1000 files but fail immediately' HTTP, HTTP PUT
FAIL 1490: '"upload" with file:// overwriting existing' FILE

This reverts commit 62cf58b. Seems to be breaking a lot of tests: https://github.com/curl/curl/actions/runs/23343096423/job/67901964263?pr=21011 FAIL 107: 'FTP PASV upload file' FTP, EPSV, STOR FAIL 108: 'FTP PORT upload with CWD' FTP, PORT, STOR FAIL 109: 'FTP PASV upload append' FTP, EPSV, APPE FAIL 112: 'FTP PASV upload resume' FTP, EPSV, APPE, Resume FAIL 123: 'FTP upload resume with whole file already downloaded' FTP, EPSV, Resume FAIL 128: 'FTP upload with --crlf' FTP, EPSV, STOR, --crlf FAIL 149: 'FTP with multiple uploads' FTP FAIL 204: '"upload" with file://' FILE FAIL 205: '"upload" nonexisting with file://' FILE, FAILURE FAIL 216: 'FTP upload two files to the same dir' FTP FAIL 235: 'FTP resumed upload but no file present remotely' FTP, STOR FAIL 236: 'FTP resume upload but denied access to remote file' FTP FAIL 247: 'FTP upload time condition evaluates TRUE => skip upload' FTP, MDTM FAIL 248: 'FTP upload time condition evaluates FALSE => upload anyway' FTP, STOR, MDTM FAIL 285: 'TFTP send' TFTP, TFTP WRQ FAIL 286: 'TFTP send of boundary case 512 byte file' TFTP, TFTP WRQ FAIL 289: 'FTP resume upload but denied access to local file' FTP, STOR, Resume, FAILURE FAIL 348: 'FTP upload file with 552 disk full response' FTP, EPSV, STOR FAIL 362: 'FTP resume upload file with nothing to start from' FTP, EPSV, STOR FAIL 475: 'FTP PASV upload ASCII file' FTP, EPSV, STOR, TYPE A FAIL 476: 'FTP PASV upload ASCII file already using CRLF' FTP, EPSV, STOR, TYPE A FAIL 496: 'parallel upload missing file' curl tool, cmdline, parallel FAIL 1007: 'TFTP send with invalid permission on server' TFTP, TFTP WRQ, FAILURE FAIL 1038: 'FTP PASV upload resume from end of file' FTP, EPSV, APPE, Resume FAIL 1039: 'FTP PASV upload resume from end of empty file' FTP, EPSV, APPE, Resume FAIL 1243: 'TFTP send without TFTP options requests' TFTP, TFTP WRQ FAIL 1291: 'Attempt to upload 1000 files but fail immediately' HTTP, HTTP PUT FAIL 1490: '"upload" with file:// overwriting existing' FILE TESTFAIL: These test cases failed: 107 108 109 112 123 128 149 204 205 216 235 236 247 248 285 286 289 348 362 475 476 496 1007 1038 1039 1243 1291 1490

Bug: curl#21011 (comment)

Bug: #21011 (comment) Closes #21043

github-actions bot added the cmdline tool label Mar 19, 2026

vszakats changed the title ~~tool_operhlp: fix add_file_name_to_url result in error branches~~ tool_operhlp: fix add_file_name_to_url result on OOM Mar 19, 2026

vszakats requested a review from Copilot March 19, 2026 17:39

Copilot started reviewing on behalf of vszakats March 19, 2026 17:40 View session

augmentcode bot reviewed Mar 19, 2026

View reviewed changes

Copilot AI reviewed Mar 19, 2026

View reviewed changes

vszakats changed the title ~~tool_operhlp: fix add_file_name_to_url result on OOM~~ tool_operhlp: fix add_file_name_to_url() result on OOM Mar 20, 2026

vszakats added 4 commits March 20, 2026 13:36

step 1

1e5b730

simplify

0663733

tool_operhlp.c merge two lines

de722d6

apply valid llm suggestion

62cf58b

vszakats force-pushed the add_file_name_to_url_result branch from d03deed to 62cf58b Compare March 20, 2026 12:36

vszakats closed this in 07c10f0 Mar 20, 2026

vszakats deleted the add_file_name_to_url_result branch March 20, 2026 16:23

vszakats added a commit to vszakats/curl that referenced this pull request Mar 21, 2026

tool_operhlp: propagate low-level OOM in add_file_name_to_url()

cc969a1

Bug: curl#21011 (comment)

vszakats mentioned this pull request Mar 21, 2026

tool_operhlp: propagate low-level OOM in add_file_name_to_url() #21043

Closed

vszakats added a commit that referenced this pull request Mar 21, 2026

tool_operhlp: propagate low-level OOM in add_file_name_to_url()

e0038e1

Bug: #21011 (comment) Closes #21043

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

tool_operhlp: fix `add_file_name_to_url()` result on OOM#21011

tool_operhlp: fix `add_file_name_to_url()` result on OOM#21011
vszakats wants to merge 5 commits intocurl:masterfrom
vszakats:add_file_name_to_url_result

vszakats commented Mar 19, 2026 •

edited

Loading

Uh oh!

vszakats commented Mar 19, 2026

Uh oh!

augmentcode bot commented Mar 19, 2026

Uh oh!

augmentcode bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

vszakats Mar 19, 2026 •

edited

Loading

Uh oh!

bagder Mar 20, 2026

Uh oh!

vszakats Mar 20, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vszakats commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vszakats commented Mar 19, 2026

Uh oh!

augmentcode bot commented Mar 19, 2026

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

vszakats Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bagder Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

vszakats Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

vszakats commented Mar 19, 2026 •

edited

Loading

vszakats Mar 19, 2026 •

edited

Loading

vszakats Mar 20, 2026 •

edited

Loading