Fix Cookie SameSite constants to Pascal case per RFC specification#3608
Fix Cookie SameSite constants to Pascal case per RFC specification#3608ReneWerner87 merged 14 commits intomainfrom
Conversation
Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3608 +/- ##
==========================================
+ Coverage 91.01% 91.13% +0.11%
==========================================
Files 112 112
Lines 11290 11293 +3
==========================================
+ Hits 10276 10292 +16
+ Misses 756 744 -12
+ Partials 258 257 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 653312e | Previous: 361d76c | Ratio |
|---|---|---|---|
Benchmark_GenericParseTypeString/benchmark_genericParseTypeString#02 |
13.56 ns/op 0 B/op 0 allocs/op |
9 ns/op 0 B/op 0 allocs/op |
1.51 |
Benchmark_GenericParseTypeString/benchmark_genericParseTypeString#02 - ns/op |
13.56 ns/op |
9 ns/op |
1.51 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Thanks for updating the SameSite constants to match RFC6265-bis with Pascal case (SameSite) and title case (Strict, Lax, None). Looks great for spec compliance! However, the case-sensitive comparison in the PR might be too strict, as browsers like Chromium and WebKit use ASCII case-insensitive comparisons for SameSite values (e.g., samesite=strict or SAMESITE=lax work fine). Since HTTP headers use only ASCII, we should avoid Unicode case folding like strings.EqualFold for performance and stick to an ASCII-only case-insensitive comparison. A lightweight custom function like EqualASCIIFold could be added to gofiber/utils or even utils.ToLower could do the trick while keeping GoFiber’s speed. Thoughts on swapping the comparison to something like this?
…tibility Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Great point about the case-sensitivity issue! I've implemented ASCII case-insensitive comparison using Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Okay that’s not bad. But if the strings are not already lowercase this will alloc. A function like: func EqualASCIIFold(s, t string) bool {
if len(s) != len(t) {
return false
}
for i := 0; i < len(s); i++ {
if s[i]|0x20 != t[i]|0x20 {
return false
}
}
return true
}would check for ascii case insensitive matches while guaranteeing no allocations. Can you create this in the https://github.com/gofiber/utils package and use that? |
Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
I've implemented the Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot |
Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Replaced the custom Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot fix test coverage |
…functionality Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Implemented automatic |
Should we document that change, for example in |
We should reference the links from Mozilla and Chrome |
Co-authored-by: sixcolors <6501251+sixcolors@users.noreply.github.com>
Documented the automatic |
…urity enforcement Co-authored-by: gaby <835733+gaby@users.noreply.github.com>
Added Mozilla and Chrome documentation links to both ctx.md and whats_new.md as requested. The documentation now includes references to the official Mozilla docs and Chrome developer guidelines for SameSite=None security requirements. Fixed in commit 69f4671. |
…utes, and ensure secure attribute is set correctly for SameSite=None
…nto copilot/fix-3607
…cure attribute validation
This PR fixes the Cookie SameSite constants to use proper Pascal case values as required by the RFC specification.
Problem
The Cookie SameSite constants were using lowercase values:
However, according to RFC 6265bis, the SameSite values should be Pascal case:
"Strict" / "Lax" / "None".Solution
Updated the constants to use RFC-compliant Pascal case:
Also removed the
utils.ToLower()call inctx.gothat was converting these values back to lowercase, ensuring the cookie headers output the correct Pascal case values.Verification
SameSite=Lax,SameSite=Strict,SameSite=NoneFixes #3607.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
exampleretry.com/tmp/go-build776519753/b273/client.test -test.paniconexit0 -test.v=test2json -test.timeout=10m0s -test.count=1 -test.shuffle=on(dns block)/tmp/go-build2352507060/b273/client.test -test.paniconexit0 -test.v=test2json -test.timeout=10m0s -test.count=1 -test.shuffle=on(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.