Skip to content

fix(session): use deleteChunkedCookie in clearSession for proper cookie removal#1318

Merged
pi0 merged 1 commit into
mainfrom
fix/clear-session-cookie
Mar 14, 2026
Merged

fix(session): use deleteChunkedCookie in clearSession for proper cookie removal#1318
pi0 merged 1 commit into
mainfrom
fix/clear-session-cookie

Conversation

@productdevbook

@productdevbook productdevbook commented Mar 14, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Summary

  • `clearSession` was calling `setChunkedCookie(event, name, "")` which sets the cookie value to an empty string but does not set `Max-Age=0` — the browser retains the empty cookie until the session ends
  • Now uses `deleteChunkedCookie` which calls `deleteCookie` with `maxAge: 0`, properly instructing the browser to remove the cookie immediately
  • This also correctly handles chunked session cookies by deleting all chunk cookies (e.g. `h3.1`, `h3.2`, etc.)

Test plan

  • All existing session tests pass (10/10)
  • New test verifies `clearSession` produces `Max-Age=0` in the Set-Cookie header
  • Total: 12/12 passed

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Session clearing functionality is now available.

  • Bug Fixes

    • Session cookies are now properly deleted when clearing sessions, rather than being set to empty values.

  • Tests

    • Added test coverage for session clearing behavior.

@productdevbook @claude
fix(session): use deleteChunkedCookie in clearSession for proper cook…
7b14440 
…ie removal

clearSession was using setChunkedCookie with an empty string value, which
sets the cookie to "" but does not instruct the browser to delete it
(no Max-Age=0 or past Expires). The browser would retain the empty cookie
until the session ends.

Now uses deleteChunkedCookie which correctly sets Max-Age=0, telling the
browser to remove the cookie immediately. This also properly handles
chunked session cookies by deleting all chunk cookies.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@productdevbook
productdevbook requested a review from pi0 as a code owner March 14, 2026 15:55
@coderabbitai

coderabbitai Bot commented Mar 14, 2026 •
edited
Loading

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61e12af0-c915-4729-89ed-07d4c069786b

📥 Commits

Reviewing files that changed from the base of the PR and between c28356e and 7b14440.

📒 Files selected for processing (2)
  • src/utils/session.ts
  • test/session.test.ts
📝 Walkthrough

Walkthrough

The pull request modifies session cookie clearing behavior by replacing the practice of setting an empty value with explicit cookie deletion using a new utility function. A corresponding test validates that the cookie deletion directive (Max-Age=0) is properly triggered.

Changes

Cohort / File(s) Summary
Session Utilities
src/utils/session.ts		 Imported deleteChunkedCookie and replaced setChunkedCookie(..., "") with explicit deleteChunkedCookie(...) call in clearSession function to properly delete cookies instead of overwriting with empty values.
Test Coverage
test/session.test.ts		 Added test case verifying that clearSession triggers the cookie Max-Age=0 directive for proper cookie deletion. Exported clearSession from main module for test accessibility.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰✨ A cookie's farewell, no longer to stay,
We delete with purpose, Max-Age zero's way,
No empty shells lingering in the browser's keep,
Just clean-up magic, cookies in the deep! 🍪💫

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing setChunkedCookie with deleteChunkedCookie in clearSession for proper cookie removal, which is the core objective of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/clear-session-cookie
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@pkg-pr-new

pkg-pr-new Bot commented Mar 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/h3@1318

commit: 655f1b3

@pi0
pi0 merged commit 338965b into main Mar 14, 2026
7 checks passed
@pi0
pi0 deleted the fix/clear-session-cookie branch March 14, 2026 17:03
pi0 pushed a commit that referenced this pull request Mar 19, 2026
@productdevbook @pi0
fix(session): use deleteChunkedCookie in clearSession for proper cook…
1ffc93a 
…ie removal (#1318)
@coderabbitai coderabbitai Bot mentioned this pull request Apr 16, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pi0 pi0 pi0 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@productdevbook @pi0