Skip to content

Bugfix/cookie validation#91

Closed
maxLatoche wants to merge 2 commits intohapijs:masterfrom
Tekrallc:bugfix/cookie-validation
Closed

Bugfix/cookie validation#91
maxLatoche wants to merge 2 commits intohapijs:masterfrom
Tekrallc:bugfix/cookie-validation

Conversation

@maxLatoche
Copy link

This resolves the issue here dherault/serverless-offline#1866

@damusix damusix requested review from Marsup and kanongil January 5, 2026 08:24
Copy link

@kanongil kanongil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to strict validation is not ok.

The change to loose could potentially be accepted to better match the docs that state strictHeader option:

allows any cookie value including values in violation of RFC 6265

This is not strictly true since ; is not allowed unless the value starts and ends with a ".

I still think the current implementation is acceptable, since the browsers / user agents that would need to parse the cookie would have a very hard time to parse it. Additionally it provides a degree of security protection, since user-provides values cannot add their own tags.

I also don't see why we should accommodate another projects incorrect usage of the API.

valueRx: {
strict: /^[^\x00-\x20",;\\\x7F]*$/,
loose: /^("[^"]*"|[^;]*)$/
strict: /^[^\x00-\x20",\\\x7F]*$/,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not suitable. Strict values cannot contain ;, which this would allow!

strict: /^[^\x00-\x20",;\\\x7F]*$/,
loose: /^("[^"]*"|[^;]*)$/
strict: /^[^\x00-\x20",\\\x7F]*$/,
loose: /^("[^"]*"|.*)$/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simply /.*/. If we want to allow any value, then the code should not use a regex.

@kanongil
Copy link

kanongil commented Jan 5, 2026

There is more discussion around the issue in #90.

@maxLatoche maxLatoche closed this Jan 15, 2026
@maxLatoche maxLatoche deleted the bugfix/cookie-validation branch January 15, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants