feat: add res.cookie, res.clearCookie, and fix writeHead header merging#398
Merged
Conversation
- Add serializeCookie() with full options support (domain, path, maxAge, expires, httpOnly, secure, sameSite, encode) - Add res.cookie(name, value, opts) to #decorateResponse - Add res.clearCookie(name, opts) for clearing cookies - Fix writeHead() to merge headers from res.getHeaders() instead of passing directly, preserving cookies and other headers set before res.send() - Update TypeScript definitions with CookieOptions, ClearCookieOptions, WoodlandResponse interfaces - Add 27+ new unit tests and 3 integration tests - response.js: 100% line coverage maintained
🤖 Augment PR SummarySummary: Adds first-class cookie helpers to Woodland responses and fixes a long-standing header-loss issue when writing responses. Changes:
Technical Notes: Cookie options support domain/path/maxAge→Max-Age+Expires, httpOnly/secure, sameSite normalization, and configurable encoding (including disabled encoding). 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
- README: Add res.cookie/res.clearCookie to Response Helpers - API: Add full cookie/res.clearCookie docs with option tables - OVERVIEW: Add cookie handling to Security Features and Response Methods - FLOWS: Update #decorateResponse flow, response methods table, writeHead comment
- Remove redundant second parameter in else branch - Move existing assignment inside conditional - No third parameter passed when headers is undefined
- Check typeof instead of truthiness to avoid encodeFn being set to boolean - encode: true now uses encodeURIComponent instead of throwing - encode: false uses passthrough function - encode: function uses the custom function
- Don't recompute Expires when caller provides their own expires date - clearCookie now produces Max-Age=0 with Expires=Thu, 01 Jan 1970
- createCookieHandler now uses getHeader+setHeader to accumulate cookies - clearCookieHandler appends rather than replaces existing set-cookie values - assert on both cookie count and values in integration test
- Overwrite coverage report with accurate numbers from coverage.txt - Fix table: response.js 99.12% line, 95.71% branch, woodland.js 98.63%/92.16% - Update FLOWS.md cookie flow to show array append semantics - Update FLOWS.md clearCookie from maxAge: -1 to maxAge: 0
Owner
Author
|
augment review |
Write tests for previously uncovered code: - writeHead(res, undefined) else branch - createCookieHandler/clearCookieHandler non-array getHeader - Missing middleware route throws - Origin validation in allowlist but too long - Disallowed origin returns 403 - Safe origin returns ACA-Origin header Cover edge cases: - isSafeOrigin: length check at >255 - cookie handler: string-to-array conversion Use node:coverage ignore for intentionally unreachable/race-condition code: - Body limit handler (timing issue) - isSafeOrigin: typeof check (always string from HTTP), control chars (HTTP parser strips)
- Use 'headers !== null && typeof headers === OBJECT' check - Call res.getHeaders() only if method exists and is a function - Handle non-object headers (null, string, number) by passing directly - Add test for null headers to writeHead
Send supports streams, buffers, strings, and objects with toString(). The previous string-only type would reject valid usage at compile time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
res.cookie(name, value, opts)andres.clearCookie(name, opts)to the response object for setting and clearing HTTP cookies. FixeswriteHead()to merge passed headers with existing response headers viares.getHeaders(), preserving cookies and other headers set beforeres.send()is called.Type of Change
Testing
All 365 existing tests pass. New tests added:
serializeCookiebasic cookie generation with pathserializeCookieencoding (default encodeURIComponent, custom encode function, encode: false)serializeCookiemaxAge with expires date conversionserializeCookiehttpOnly, secure, sameSite (strict/lax/none/pass-through)serializeCookiecustom domain and path optionscreateCookieHandlersets cookie via setHeader with optionscreateClearCookieHandlerclears cookie with expired date and domain optionwriteHeadmerge headers with existing response headersTests run via
npm test(lint + full test suite).Coverage
Checklist
npm run lintpasses