Skip to content

Fix scheme detection via port for regex remap#12637

Merged
bneradt merged 1 commit into
apache:masterfrom
bneradt:fix_scheme_ternary
Nov 4, 2025
Merged

Fix scheme detection via port for regex remap#12637
bneradt merged 1 commit into
apache:masterfrom
bneradt:fix_scheme_ternary

Conversation

@bneradt

@bneradt bneradt commented Nov 4, 2025

Copy link
Copy Markdown
Contributor

The ternary operator was checking literal 80 instead of request_port == 80, making an accidental tautology that always returned the HTTP scheme. This was introduced in the #12243 URL std::string_view refactor.


For reference, here's a link to the previous code:
https://github.com/apache/trafficserver/pull/12243/files#diff-4ba856cb52d86339783fd72f06f834f43ef3c1225c45ec735353fdd7b9aa1da4L1021

The ternary operator was checking literal 80 instead of
request_port == 80, making an accidental tautology that always returned
the HTTP scheme. This was introduced in the apache#12243 URL std::string_view
refactor.
@bneradt bneradt added this to the 10.2.0 milestone Nov 4, 2025
@bneradt bneradt requested a review from Copilot November 4, 2025 19:35
@bneradt bneradt self-assigned this Nov 4, 2025

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 a critical bug in the URL rewriting logic where the scheme determination for empty schemes was using an incorrect condition. The fix ensures that the request port is properly checked when deciding between HTTP and HTTPS schemes.

  • Corrected the ternary operator condition to actually evaluate request_port == 80 instead of just the literal 80

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

@bneradt bneradt merged commit 0d4f77c into apache:master Nov 4, 2025
21 checks passed
@bneradt bneradt deleted the fix_scheme_ternary branch November 4, 2025 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants