Skip to content

Support RFC 7239 Forwarded header proto in Scheme()#3061

Open
sonnemusk wants to merge 2 commits into
labstack:masterfrom
sonnemusk:feat/forwarded-header-scheme
Open

Support RFC 7239 Forwarded header proto in Scheme()#3061
sonnemusk wants to merge 2 commits into
labstack:masterfrom
sonnemusk:feat/forwarded-header-scheme

Conversation

@sonnemusk

Copy link
Copy Markdown
Contributor

Summary

Fixes #2694 by teaching Context.Scheme() to honor the standardized Forwarded header (proto= parameter, RFC 7239).

Previous attempts (#2702, #2744) either returned the raw header without parsing proto=, or introduced API surface / RealIP changes that were hard to get right. This PR is intentionally narrow:

  • Scheme only — no RealIP / IPExtractor changes (those belong in a dedicated extractor if needed later)
  • Valid protos only — same allow-list as existing proxy headers: http, https, ws, wss (case-insensitive match)
  • No new Context methods — no interface break

Resolution order

  1. Direct TLS connection → https
  2. Forwarded proto= (RFC 7239), left-most valid value
  3. X-Forwarded-Proto
  4. X-Forwarded-Protocol
  5. X-Forwarded-Ssl == "on"https
  6. X-Url-Scheme
  7. default http

Forwarded is preferred over the non-standard X-Forwarded-* headers when both are present and valid. If Forwarded is missing, has no proto, or has an invalid proto, resolution falls through unchanged — existing X-Forwarded-Proto behavior is preserved.

Parsing notes

  • Comma-separated multi-proxy elements and multiple Forwarded header fields
  • Semicolon-separated pairs within an element
  • Case-insensitive parameter names (Proto=HTTPS)
  • Token and quoted-string values (quoted-pair escapes)
  • Unknown parameters are ignored; malformed input is skipped rather than failing closed incorrectly

Test plan

  • go test . -run 'TestContext_Scheme|TestFirstForwardedProto' -count=1
  • go test ./... -count=1
  • Cases covered: proto=https, full for;proto;by, multi-proxy left-most, multi-header fields, quoted values, invalid/empty fallback to X-Forwarded-Proto, precedence vs TLS / X-Forwarded-Proto, malformed input

Parse the standardized Forwarded header for a valid proto= value
when resolving the request scheme. Prefer Forwarded over the
non-standard X-Forwarded-* headers while keeping TLS first and
falling through on missing or invalid proto.

Only http, https, ws, and wss are accepted. Multi-proxy and
multi-header Forwarded values are handled with left-most valid
proto; quoted-string values are unescaped per RFC 7230.

Fixes labstack#2694
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.43%. Comparing base (dcb05f0) to head (77d5781).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
context.go 97.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3061      +/-   ##
==========================================
+ Coverage   93.34%   93.43%   +0.08%     
==========================================
  Files          43       43              
  Lines        4735     4810      +75     
==========================================
+ Hits         4420     4494      +74     
  Misses        192      192              
- Partials      123      124       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add firstForwardedProto cases for quoted garbage, missing '=', unclosed
quotes, trailing OWS, and isTokenChar checks so codecov/patch covers the
RFC 7239 parser branches.
@aldas

aldas commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

I really do not like this parsing logic from maintenance perspective. This is on par with Router.Route complexity and for feature that is not implemented even in Gin/Fiber. My comments in #2702 (comment) and comments here #2744 I think if Forwarded parsing is to be implemnented it has to be some kind of central Proxy logic that can be used by all methods that deal with these chained header values and chooses which one to trust. (note to self: somewhat related Fiber has "isProxyTrusted" thingie).

@sonnemusk why are you so persistent to make it happen? You are working on quite low value item here - these days most of the internet is on HTTPS anyway and browsers will complain even if your site is using HTTP. so if you already have proxy in front of your application you have probably configured HTTP -> HTTPS redirect anyway.

May I suggest something more beneficial for society - alternative router implemtation that stores routes by method in separate trees - so less frequent methods could be accessed "possibly" faster as their trees are smaller.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support 'Forwarded' header

2 participants