fixes case sensitivity for SameSite directive #207
Conversation
michalbundyra
left a comment
michalbundyra
left a comment
There was a problem hiding this comment.
@wilcol Thanks for your contribution. I have couple minor comments. In general it looks good. See my comments below. Thanks!
|
@michalbundyra, I processed your feedback. Thank you. |
|
@Xerkus, I processed your feedback. |
|
Please also add a separate test for asserting that SameSite value is canonicalized in setSameSite |
…code for readability
|
@Xerkus all done! |
Xerkus
left a comment
Xerkus
left a comment
There was a problem hiding this comment.
Looks good, just a bit of extra changes.
|
|
||
| $sameSite = $this->getSameSite(); | ||
| if ($sameSite !== null && in_array($sameSite, self::SAME_SITE_ALLOWED_VALUES, true)) { | ||
| if ($sameSite !== null && array_key_exists(strtolower($sameSite), self::SAME_SITE_ALLOWED_VALUES)) { |
There was a problem hiding this comment.
coding style: used functions must be imported with use function statements (under use block, separated by blank line)
Import just those you touched, that will be enough.
| 9, | ||
| SetCookie::SAME_SITE_STRICT | ||
| ); | ||
| $this->assertEquals(SetCookie::SAME_SITE_STRICT, $setCookieHeader->getSameSite()); |
There was a problem hiding this comment.
Please also test setSameSite() explicitly. Just once would be enough.
Forward port #207 Conflicts: CHANGELOG.md
|
As this was a bugfix, I rebased against master and merged there for a new 2.11.2 release. Thanks, @wilcol ! |
4 participants
This fix handles case sensitivity for SameSite directive in cookie headers.
Prior to this fix the SameSite values were case sensitive compared with their capitalized names (
Lax,Strict,None). This fix allows for case insensitive comparison, allowing values likelaxandLAX.See #206 for more information