Skip to content

tool_operate: drop the scheme-guessing in the -G handling#20992

Closed
bagder wants to merge 2 commits intomasterfrom
bagder/-G-default
Closed

tool_operate: drop the scheme-guessing in the -G handling#20992
bagder wants to merge 2 commits intomasterfrom
bagder/-G-default

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Mar 18, 2026

Prior to this, -G would override the scheme set with --proto-default and revert back to guessing the scheme based on the hostname.

Add test 2008 to verify the fix

Spotted by Codex Security

Prior to this, -G would override the scheme set with --proto-default and
revert back to guessing the scheme based on the hostname.

Add test 2008 to verify the fix

Spotted by Codex Security
@github-actions github-actions Bot added the tests label Mar 18, 2026
@bagder bagder requested a review from Copilot March 18, 2026 23:21
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 curl tool -G handling so it no longer reverts to hostname-based scheme guessing when --proto-default is set, and adds a regression test to ensure the intended scheme is preserved in proxy absolute-form requests.

Changes:

  • Update append2query() in src/tool_operate.c to stop reintroducing a guessed scheme when reconstructing the URL after appending query parameters.
  • Add new test case test2008 to cover -G with --proto-default on a schemeless hostname that would otherwise trigger scheme guessing.
  • Register test2008 in the tests data Makefile.

Reviewed changes

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

File Description
tests/data/test2008 New regression test covering -G + --proto-default behavior with proxy usage
tests/data/Makefile.am Adds test2008 to the test suite list
src/tool_operate.c Stops append2query() from reintroducing guessed schemes when rebuilding URLs

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

Comment thread tests/data/test2008 Outdated
Comment thread tests/data/test2008
Comment thread tests/data/test2008
@bagder bagder marked this pull request as ready for review March 18, 2026 23:31
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Mar 18, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Mar 18, 2026

🤖 Augment PR Summary

Summary: Fixes curl tool -G query handling so it no longer re-guesses the URL scheme, preserving the scheme implied by --proto-default.

Changes:

  • Use CURLU_NO_GUESS_SCHEME when reconstructing the URL after appending query parameters.
  • Add test 2008 to cover -G + --proto-default=http with a schemeless host that would otherwise be guessed as FTP.
  • Register the new test in tests/data/Makefile.am.

Why: Prevents -G from overriding --proto-default by falling back to host-based protocol guessing (reported by Codex Security).

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

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

@bagder bagder closed this in 756725a Mar 20, 2026
@bagder bagder deleted the bagder/-G-default branch March 20, 2026 10:57
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.

2 participants