Skip to content

split cookie values on & only#1728

Closed
yanick wants to merge 3 commits intomainfrom
cookie-semicolon
Closed

split cookie values on & only#1728
yanick wants to merge 3 commits intomainfrom
cookie-semicolon

Conversation

@yanick
Copy link
Contributor

@yanick yanick commented Jun 12, 2025

Seems that ::Request was being overly zealous and was splitting the cookie value on ; as well. This is not the same behavior as HTTP::XSCookie, which only split on &.

Fix #1701

Seems that ::Request was being overly zealous and was splitting the
cookie value on `;` as well. This is not the same behavior as
HTTP::XSCookie, which only split on `&`.

Fix #1701
diag "If you want extra speed, install HTTP::XSCookies"
if !Dancer2::Core::Cookie::_USE_XS;

sub run_test {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same tests as before, I just wrapped them in subtests for better output.

@cromedome
Copy link
Contributor

Thanks @yanick! 👍

@cromedome cromedome added this to the Dancer2 2.0.0 milestone Jun 18, 2025
Copy link
Contributor

@cromedome cromedome left a comment

Choose a reason for hiding this comment

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

Still 👍 from me. Thanks for looking at the test failures.

@yanick yanick self-assigned this Jul 10, 2025
@yanick
Copy link
Contributor Author

yanick commented Jul 10, 2025

It's obviously a 👍 from me too. So I say: merge it!

@veryrusty veryrusty self-requested a review July 11, 2025 16:10
Copy link
Member

@veryrusty veryrusty left a comment

Choose a reason for hiding this comment

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

Consistency with HTTP::XSCookies gets a 👍 from me. Thanks @yanick

@cromedome cromedome closed this Jul 11, 2025
@cromedome cromedome deleted the cookie-semicolon branch July 11, 2025 20:32
@cromedome
Copy link
Contributor

Merged, thanks!

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.

Cookie parsing bug

3 participants