Skip to content

fix(http): handle empty Cookie headers - #7328

Open
dennisimoo wants to merge 2 commits into
denoland:mainfrom
dennisimoo:codex/empty-cookie-header
Open

dennisimoo wants to merge 2 commits into
denoland:mainfrom
dennisimoo:codex/empty-cookie-header

Conversation

@dennisimoo

@dennisimoo dennisimoo commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #7320.

Treat an empty Cookie header like a missing header and ignore empty cookie pairs, including trailing semicolons. Preserve nonempty values and existing malformed-key validation.

Regression tests cover empty headers, empty pairs, whitespace, and malformed keys. The new trailing-semicolon test failed before the fix.

Tests: deno test -A --doc http/ — 479 passed, 987 steps, 2 ignored. Full lint, formatting and browser checks passed. The earlier full deno task ok run was killed (exit 137) in the unrelated CBOR test that allocates a 4–8 GiB buffer, including on a serial retry.

AI-assisted with Codex.

@github-actions github-actions Bot added the http label Sep 23, 2026
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (01fe755) to head (ff38944).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7328      +/-   ##
==========================================
- Coverage   95.04%   95.04%   -0.01%     
==========================================
  Files         618      620       +2     
  Lines       51746    52016     +270     
  Branches     9402     9452      +50     
==========================================
+ Hits        49181    49437     +256     
- Misses       2022     2031       +9     
- Partials      543      548       +5     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@CLAassistant

CLAassistant commented Sep 23, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@bartlomieju

Copy link
Copy Markdown
Member

Thanks for the PR, it looks mostly okay. I'm still wary that it will throw on a=1; - can you handle that case before we merge the PR?

@dennisimoo

Copy link
Copy Markdown
Contributor Author

Handled in ff38944. Empty cookie pairs are now skipped, including a=1;, while malformed nonempty keys still throw. Added regression coverage; all 479 HTTP tests, lint, formatting and browser checks pass.

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getCookies() throws error when cookie header is an empty string

3 participants