Skip to content

protocol: use scheme names lowercase - #21033

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/lowercase-protos
Closed

bagder wants to merge 1 commit into
masterfrom
bagder/lowercase-protos

Conversation

@bagder

@bagder bagder commented Mar 20, 2026 •

Copy link
Copy Markdown
Member

When composing the <scheme>_proxy environment variable, we assume the handler->scheme name is already lowercase.

This makes unit test 1627 verify that is the case.

Follow-up to c294f9c

Spotted by Codex Security

When composing the <scheme>_proxy environment variable, we assume the
handler->scheme name is already lowercase.

This makes unit test 1627 verify that is the case.

Follow-up to c294f9c

Spotted by Codex Security
@github-actions github-actions Bot added the tests label Mar 20, 2026
@bagder
bagder marked this pull request as ready for review March 20, 2026 13:31
@bagder
bagder requested a review from Copilot March 20, 2026 13:31

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 enforces the invariant that libcurl’s registered scheme names (conn->scheme->name) are lowercase, which is relied upon when forming <scheme>_proxy environment variable names.

Changes:

  • Update lib/protocol.c to use lowercase scheme names for sftp, scp, ws, and wss.
  • Extend unit test unit1627 to assert that Curl_get_scheme(...)->name matches a lowercased version of the input.

Reviewed changes

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

File Description
tests/unit/unit1627.c Adds an assertion ensuring returned scheme names are lowercase.
lib/protocol.c Normalizes several scheme name constants to lowercase to match expected invariants.

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

Comment thread tests/unit/unit1627.c
@testclutch

Copy link
Copy Markdown

Analysis of PR #21033 at 3497c947:

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

Generated by Testclutch

@bagder bagder closed this in 6d1d50d Mar 20, 2026
@bagder
bagder deleted the bagder/lowercase-protos branch March 20, 2026 15:01
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
When composing the <scheme>_proxy environment variable, we assume the
handler->scheme name is already lowercase.

This makes unit test 1627 verify that is the case.

Follow-up to c294f9c

Spotted by Codex Security

Closes curl#21033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants