Skip to content

config2setopts: use default protocol properly#21983

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/default-proto
Closed

config2setopts: use default protocol properly#21983
bagder wants to merge 1 commit into
masterfrom
bagder/default-proto

Conversation

@bagder

@bagder bagder commented Jun 12, 2026

Copy link
Copy Markdown
Member

No description provided.

@bagder bagder marked this pull request as ready for review June 12, 2026 08:00
@bagder bagder requested a review from Copilot June 12, 2026 08:00

Copilot AI left a comment

Copy link
Copy Markdown

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 adjusts how the curl tool derives the “active” protocol from a URL when --proto-default is set, aiming to ensure the default protocol is used (and scheme guessing is avoided) for schemeless URLs.

Changes:

  • Update url_proto_and_rewrite() to use CURLU_NO_GUESS_SCHEME when --proto-default is provided.
  • Update documentation for --proto-default to clarify that ipfs/ipns cannot be used as default protocols.

Reviewed changes

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

File Description
src/config2setopts.c Changes scheme extraction logic in url_proto_and_rewrite() when --proto-default is set.
docs/cmdline-opts/proto-default.md Documents that ipfs/ipns cannot be used as default protocols.

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

Comment thread src/config2setopts.c
Comment thread src/config2setopts.c
@testclutch

This comment was marked as resolved.

@github-actions github-actions Bot added the tests label Jun 12, 2026
@bagder

bagder commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode

augmentcode Bot commented Jun 12, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Adjusts curl tool URL scheme detection so --proto-default is applied as intended (use the requested default when the URL has no explicit scheme) while still honoring explicit schemes.

Changes:

  • Update src/config2setopts.c:url_proto_and_rewrite() to avoid hostname-based scheme guessing when config->proto_default is set (use CURLU_NO_GUESS_SCHEME and fall back to the default protocol token when needed).
  • Ensure explicit ipfs:///ipns:// URLs still go through the IPFS gateway rewrite path even when --proto-default is provided.
  • Document that --proto-default expects a case-insensitive bare protocol name (no :// suffix) and that it cannot be set to ipfs/ipns.
  • Add new testcases covering IPFS + --proto-default (test1724), SCP selection via --proto-default (test1725), and rejecting a ://-suffixed argument (test2036).

Technical Notes: The change continues to use proto_token() for stable pointer comparisons, and uses CURLU_NO_GUESS_SCHEME to treat guessed schemes as “no scheme” when a default protocol is requested.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread src/config2setopts.c
Comment thread docs/cmdline-opts/proto-default.md Outdated
Verified by test 1724, 1725 and 2036

Closes #21983
@bagder bagder force-pushed the bagder/default-proto branch from 345bc8b to 092fab0 Compare June 12, 2026 09:37
@bagder bagder closed this in ab3bb8c Jun 12, 2026
@bagder bagder deleted the bagder/default-proto branch June 12, 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.

3 participants