fix: invalid cookie error#1885
Merged
Merged
Conversation
Fix invalid cookie error
chore: remove redundant import
Owner
|
Not sure about this. Let's see what copilot has to say. Merging this today if good. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses “invalid cookie” errors introduced by stricter @hapi/statehood cookie validation by changing how Set-Cookie response headers are applied to Hapi responses (using h.state() with properly separated cookie value vs. attributes).
Changes:
- Added a new
generateHapiCookieutility to parseSet-Cookiestrings into{ name, value, options }. - Updated
HttpServerresponse handling to callh.state()using the parsed cookie name/value/options instead of passing attributes inside the value. - Exported the new utility via
src/utils/index.js.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/utils/index.js |
Exports the new generateHapiCookie utility for use by the HTTP server. |
src/utils/generateHapiCookie.js |
Introduces parsing logic to split cookie name/value from attributes and map attributes into Hapi cookie options. |
src/events/http/HttpServer.js |
Uses generateHapiCookie() when translating Lambda Set-Cookie output into Hapi h.state() calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2
to
+28
| const name = cookieString.slice(0, cookieString.indexOf("=")); | ||
| const value = cookieString.slice(cookieString.indexOf("=") + 1, cookieString.indexOf(";")); | ||
|
|
||
| // Parse attributes into a map | ||
| const attributes = cookieString | ||
| .split(";") | ||
| .slice(1) // skip the name=value part | ||
| .reduce((acc, part) => { | ||
| const [key, val] = part.trim().split("="); | ||
| acc[key.trim().toLowerCase()] = val ? val.trim() : true; | ||
| return acc; | ||
| }, {}); | ||
|
|
||
| return { | ||
| name, | ||
| value, | ||
| options: { | ||
| ttl: attributes["max-age"] ? parseInt(attributes["max-age"]) * 1000 : undefined, | ||
| isSecure: attributes["secure"] === true, | ||
| isHttpOnly: attributes["httponly"] === true, | ||
| path: attributes["path"], | ||
| domain: attributes["domain"], | ||
| isSameSite: attributes["samesite"] || false, | ||
| encoding: "none", | ||
| strictHeader: false, | ||
| }, | ||
| }; |
Owner
|
Hi, thank you for the PR. Please rebase/merge master, pass the tests and address some (if not all) of the review comments. |
…xpires attribute scenario
Chore/revise hapicookie
Owner
|
Great job, let's ship it! |
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
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
added @Hapi cookie generator, to fulfill .state() cookie requirement of @Hapi
Motivation and Context
this change fixes issue #1866 by setting the @Hapi cookie value correctly as per the new @Hapi regexp validation. and setting @Hapi cookie property as per desired.
How Has This Been Tested?
ran the serverless-offline fix with @hapi/statehood 8.2.1, and invalid cookie error is resolved.
Sample cookie used:
cookieID=test-3121359fb6ced7c2ca330e2783be73b748a847f0; Max-Age=1800; Expires=Wed, 29 Apr 2026 09:01:55 GMT; Secure; HttpOnly; Domain=localhost; Path=/