🔥 feat: Add ExpireCookie method for cookies with Path/Domain #3976
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-07-19T14:06:29.884ZApplied to files:
📚 Learning: 2024-10-08T19:06:06.583ZApplied to files:
📚 Learning: 2025-07-17T11:39:38.265ZApplied to files:
🧬 Code graph analysis (1)res.go (1)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @darwin808, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the framework's cookie management capabilities by introducing a dedicated Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new ExpireCookie method to correctly expire cookies with specific Path or Domain attributes, addressing a limitation in the existing ClearCookie method. The implementation is a good start, but it's missing support for SameSite and Partitioned attributes, which are crucial for expiring modern cookies correctly. I've provided suggestions to add this functionality, update the corresponding documentation, and enhance the unit tests for better coverage and maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ctx_test.go (1)
4506-4562: Solid coverage for ExpireCookie; consider a couple of small assertionsThe test exercises the important combinations (Path, Domain, Path+Domain, Secure+HttpOnly) and verifies the Set-Cookie line in a robust, order‑independent way. Two optional tightenings you might consider:
- Also assert
require.Contains(t, setCookie, "expires=")in the Domain and Path+Domain (and possibly Secure+HttpOnly) cases, so every variant explicitly checks that the cookie is actually expired, not just that attributes are present.- Optionally add a single scenario that calls
c.ExpireCookie(...)instead ofc.Res().ExpireCookie(...)to exercise the Ctx helper as well as the Res method.Using
Response().Header.Peek(HeaderSetCookie)with string matching is consistent with the established pattern of manually inspecting Set-Cookie in Fiber tests, instead of relying onHeader.Cookie. Based on learnings, ...
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ctx_interface_gen.goctx_test.godocs/api/ctx.mdres.gores_interface_gen.go
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Files:
ctx_interface_gen.gores.goctx_test.gores_interface_gen.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/api/ctx.md
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
make markdownto lint all Markdown files when modifying code
Files:
docs/api/ctx.md
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
When adding Go tests, always invoke
t.Parallel()at the start of each test and subtest to maximize concurrency
Files:
ctx_test.go
🧠 Learnings (8)
📓 Common learnings
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-07-01T03:44:03.672Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-07-02T13:29:56.992Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
📚 Learning: 2024-09-25T15:57:10.221Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
ctx_interface_gen.gores.godocs/api/ctx.mdctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Applied to files:
ctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
ctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3051
File: middleware/session/session.go:215-216
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Applied to files:
ctx_test.go
📚 Learning: 2024-07-01T03:33:22.283Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-07-01T03:33:22.283Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Applied to files:
ctx_test.go
📚 Learning: 2024-10-12T10:01:44.206Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-10-12T10:01:44.206Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
Applied to files:
ctx_test.go
📚 Learning: 2024-11-29T12:37:27.581Z
Learnt from: efectn
Repo: gofiber/fiber PR: 3162
File: app_test.go:893-895
Timestamp: 2024-11-29T12:37:27.581Z
Learning: In the `Test_App_ShutdownWithContext` function in `app_test.go`, the `clientDone` channel is used to synchronize the client's request completion before proceeding, eliminating the need for additional `time.Sleep` calls.
Applied to files:
ctx_test.go
🧬 Code graph analysis (2)
ctx_interface_gen.go (3)
res.go (1)
Cookie(104-116)client/request.go (1)
Cookie(778-778)bind_test.go (2)
Cookie(1512-1516)Cookie(1757-1761)
ctx_test.go (3)
res_interface_gen.go (1)
Res(13-174)res.go (1)
Cookie(104-116)constants.go (1)
HeaderSetCookie(197-197)
🔇 Additional comments (5)
res_interface_gen.go (1)
21-34: ClearCookie note and ExpireCookie API look consistent and clearThe new documentation accurately scopes ClearCookie’s limitations and positions ExpireCookie as the attribute-aware alternative; the ExpireCookie signature is minimal and consistent with the Cookie struct fields.
ctx_interface_gen.go (1)
327-340: Ctx-level ExpireCookie matches Res API and clarifies ClearCookie behaviorAdding ExpireCookie to Ctx with matching docs keeps the public surface coherent and clearly directs users away from ClearCookie for Domain/Path-specific cookies.
docs/api/ctx.md (2)
1743-1774: LGTM! Clear documentation of ClearCookie limitations.The updated caution note and reference to ExpireCookie provide excellent guidance for developers who need to clear cookies with specific Path or Domain attributes.
1775-1813: LGTM! Comprehensive and well-structured documentation.The ExpireCookie documentation is excellent:
- Clear method signature and description
- Practical examples covering common scenarios
- Important note about which Cookie fields are actually used
This will help developers properly expire cookies with Path/Domain attributes.
res.go (1)
202-219: LGTM! Documentation accurately reflects the limitation.The updated ClearCookie documentation correctly notes that it cannot clear cookies with specific Path or Domain attributes, and appropriately directs users to ExpireCookie for those cases.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3976 +/- ##
==========================================
- Coverage 91.67% 91.63% -0.04%
==========================================
Files 119 119
Lines 10206 10290 +84
==========================================
+ Hits 9356 9429 +73
- Misses 538 544 +6
- Partials 312 317 +5
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:
|
Fixes gofiber#2878 ClearCookie() does not work for cookies that were set with specific Path or Domain attributes because browsers require matching attributes to properly clear a cookie. This adds ExpireCookie(cookie Cookie) which allows specifying the Path, Domain, Secure, and HTTPOnly attributes when expiring a cookie, ensuring the browser properly matches and clears it.
darwin808
force-pushed
the
fix/expire-cookie-path-domain
branch
from
2a2f69c to
cf04f3f
Compare
|
The CI failure in The test expected <10 leaked goroutines but got 14, which can vary based on:
Could a maintainer please re-run CI? The |
|
@darwin808 Don't use force push, we can't see what's changing between commits. Makes reviewing harder. |
|
@darwin808 Tag me once the comments raised as addressed. Thanks 💪 |
- Handle SameSite attribute (Strict/Lax/None) for proper cookie expiration - Handle Partitioned attribute for CHIPS cookies - Auto-set Secure=true when SameSite=None or Partitioned=true (per RFC) - Update documentation with new examples - Refactor tests to table-driven style with comprehensive coverage
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/api/ctx.md (1)
1806-1810: Consider using the typed constant in documentation for consistency.The example uses the string literal
"Strict"which works correctly due to case-insensitive matching. For documentation clarity, you might consider showing the constant usage alongside the string literal, since users might import and usefiber.CookieSameSiteStrictMode.// Using the constant: c.Res().ExpireCookie(&fiber.Cookie{ Name: "csrf", SameSite: fiber.CookieSameSiteStrictMode, })This is a minor suggestion for consistency with how the codebase defines these values.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ctx_interface_gen.goctx_test.godocs/api/ctx.mdres.gores_interface_gen.go
🚧 Files skipped from review as they are similar to previous changes (2)
- res_interface_gen.go
- ctx_test.go
🧰 Additional context used
📓 Path-based instructions (3)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/api/ctx.md
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run
make markdownto lint all Markdown files when modifying code
Files:
docs/api/ctx.md
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Files:
ctx_interface_gen.gores.go
🧠 Learnings (3)
📓 Common learnings
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
📚 Learning: 2024-09-25T15:57:10.221Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
docs/api/ctx.mdres.go
📚 Learning: 2025-07-19T14:06:29.884Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
Applied to files:
docs/api/ctx.mdctx_interface_gen.gores.go
🧬 Code graph analysis (2)
ctx_interface_gen.go (2)
res.go (1)
Cookie(104-116)client/request.go (1)
Cookie(778-778)
res.go (3)
bind_test.go (2)
Cookie(1512-1516)Cookie(1757-1761)constants.go (3)
CookieSameSiteStrictMode(315-315)CookieSameSiteNoneMode(316-316)CookieSameSiteLaxMode(314-314)client/response.go (1)
Response(19-25)
🔇 Additional comments (4)
res.go (2)
204-206: LGTM!Clear documentation update that properly warns users about the ClearCookie limitation for cookies with Domain/Path attributes and directs them to use ExpireCookie.
221-265: Well-implemented cookie expiration with full attribute support.The implementation correctly handles:
- Path and Domain matching for proper cookie deletion per RFC 6265
- SameSite attribute with case-insensitive comparison
- Partitioned cookies (CHIPS)
- Auto-enforcement of
Secure=truewhenSameSite=NoneorPartitioned=trueper browser requirementsThe Acquire/Release pattern for fasthttp cookies is correctly used, and the deferred release ensures cleanup.
ctx_interface_gen.go (1)
329-340: LGTM!The interface definition correctly mirrors the implementation. The documentation properly notes all fields used (Name, Path, Domain, Secure, HTTPOnly, SameSite, Partitioned) and clarifies that Value/Expires are overwritten.
docs/api/ctx.md (1)
1773-1824: Comprehensive documentation for the new ExpireCookie API.The examples effectively demonstrate all supported use cases:
- Path-specific cookies
- Domain-specific cookies
- Security flags (Secure, HTTPOnly)
- SameSite attribute
- Partitioned cookies (CHIPS)
The note block clearly explains which fields are consumed and that Value/Expires are overwritten.
|
@gaby Addressed the review comments - added SameSite and Partitioned support, updated docs, and refactored tests to table-driven style. |
- Add default case to SameSite switch statement (revive) - Reorder struct fields for better memory alignment (govet)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ctx_test.go (1)
4506-4598: Solid ExpireCookie coverage; consider releasing acquired contexts.The table‑driven test nicely exercises Path/Domain, SameSite variants, Secure/HTTPOnly, and Partitioned behavior with parallel subtests, which aligns well with the new API.
Minor nit: each subtest calls
app.AcquireCtx(&fasthttp.RequestCtx{})but never releases the context. For consistency with other tests and better pool hygiene, consider:Suggested cleanup
- app := New() - c := app.AcquireCtx(&fasthttp.RequestCtx{}) + app := New() + c := app.AcquireCtx(&fasthttp.RequestCtx{}) + defer app.ReleaseCtx(c)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ctx_test.gores.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer
github.com/gofiber/utils/v2helpers (for example,utils.Trim) when performing common operations such as string manipulation, whenever it is practical and appropriate for the surrounding code
Files:
res.goctx_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
When adding Go tests, always invoke
t.Parallel()at the start of each test and subtest to maximize concurrency
Files:
ctx_test.go
🧠 Learnings (9)
📓 Common learnings
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-09-25T15:57:10.221Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
📚 Learning: 2025-07-19T14:06:29.884Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
Applied to files:
res.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/csrf/csrf_test.go:188-193
Timestamp: 2024-10-08T19:06:06.583Z
Learning: In the Fiber framework tests, using `ctx.Response.Header.Cookie` may not be suitable for parsing cookies from the response header, as it requires a `*Cookie` and fills it rather than returning a string value; thus, manual parsing of the `Set-Cookie` header may be necessary.
Applied to files:
res.goctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:22-25
Timestamp: 2024-10-08T19:06:06.583Z
Learning: The `encryptcookie_test.go` file contains unit tests that validate key lengths for both `EncryptCookie` and `DecryptCookie` functions, ensuring that invalid key lengths raise appropriate errors.
Applied to files:
ctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:20-23
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Unit tests for key length enforcement in both `EncryptCookie` and `DecryptCookie` functions have been added to ensure robust validation and prevent potential runtime errors.
Applied to files:
ctx_test.go
📚 Learning: 2024-07-01T03:33:22.283Z
Learnt from: gaby
Repo: gofiber/fiber PR: 3056
File: middleware/encryptcookie/utils.go:51-54
Timestamp: 2024-07-01T03:33:22.283Z
Learning: Unit tests for key length enforcement in `DecryptCookie` have been added to ensure consistency and security in the encryption processes.
Applied to files:
ctx_test.go
📚 Learning: 2024-10-08T19:06:06.583Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3051
File: middleware/session/session.go:215-216
Timestamp: 2024-10-08T19:06:06.583Z
Learning: Parallel tests for `Session.Save` already exist in the `middleware/session/session_test.go` file, specifically in the `Test_Session_Save` and `Test_Session_Save_Expiration` functions.
Applied to files:
ctx_test.go
📚 Learning: 2025-12-07T15:07:23.885Z
Learnt from: CR
Repo: gofiber/fiber PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T15:07:23.885Z
Learning: Applies to **/*_test.go : When adding Go tests, always invoke `t.Parallel()` at the start of each test and subtest to maximize concurrency
Applied to files:
ctx_test.go
📚 Learning: 2024-10-12T10:01:44.206Z
Learnt from: sixcolors
Repo: gofiber/fiber PR: 3016
File: middleware/session/middleware_test.go:190-191
Timestamp: 2024-10-12T10:01:44.206Z
Learning: When testing session `IdleTimeout` expiration, it's acceptable to use `time.Sleep` to simulate the passage of time in tests.
Applied to files:
ctx_test.go
🧬 Code graph analysis (1)
ctx_test.go (3)
res.go (1)
Cookie(104-116)constants.go (3)
CookieSameSiteStrictMode(315-315)CookieSameSiteLaxMode(314-314)CookieSameSiteNoneMode(316-316)res_interface_gen.go (1)
Res(13-174)
🔇 Additional comments (1)
res.go (1)
202-207: ExpireCookie implementation and docs align with the intended semantics.The ClearCookie comment accurately documents its Path/Domain limitation, and
DefaultRes.ExpireCookiecorrectly:
- Constructs a deletion cookie (
""value,CookieExpireDelete).- Mirrors Path/Domain, when provided.
- Applies SameSite via case‑insensitive compare, and forces
SecureforSameSite=NoneandPartitionedcookies.- Preserves/overrides only the documented fields and emits the header via
SetCookie.This matches the new Res/Ctx interface contract and the RFC/best‑practice requirements around
SameSite=Noneand partitioned cookies. I don’t see further changes needed here.Also applies to: 221-267
- Add defer app.ReleaseCtx(c) for proper context cleanup - Reorder struct fields for optimal memory alignment
|
@darwin808 can you check my hint |
- Add Secure enforcement for Partitioned cookies per CHIPS spec - Update test to reflect new auto-fix behavior
3 participants
Summary
ExpireCookie(cookie Cookie)method to properly expire cookies that were set with specificPathorDomainattributesClearCookiedocumentation to note its limitation with Path/Domain cookiesFixes #2878
Problem
ClearCookie()does not work for cookies that were set with specificPathorDomainattributes. This is because browsers require matching attributes to properly clear a cookie (per RFC 6265). The underlyingfasthttp.DelClientCookiedoes not support setting these attributes.Solution
Added a new
ExpireCookie(cookie Cookie)method that allows specifying thePath,Domain,Secure, andHTTPOnlyattributes when expiring a cookie. This ensures the browser properly matches and clears it.Test plan