-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow SameSite flag to be used in cookies #2388
Conversation
f00f7fe
to
5767db9
Compare
I do not know why Travis failed on hhvm, not familiar with either and the errors look vague to me. If it is something I can fix I would love to hear suggestions. |
This looks great. Would you mind raising a PR in https://github.com/slimphp/Slim-Website to document it? Also, Slim 4.x will use https://github.com/slimphp/Slim-Http, so could you port this PR to that respository too, so we have it for the future? |
If you rebase against |
Thank you for the feedback. I can rebase & push quite easily, but looking back on my code I see that I have mistaken 'non-strict comparison' for 'case-insensitive comparison'. Which is a dumb mistake. To compare I need to normalize the input properly.
Cookies don't seem to be documented in v3 (I'm guessing the class shouldn't be present, it is mentioned in the v3 upgrade guide that it was removed) so where should I put this? I will cherry-pick the patch to Slim-Http/v4 after figuring out the above. |
Use strtolower() or stripos().
Yes. |
@sg3s are you likely to be able to work on this? |
5767db9
to
95fef86
Compare
95fef86
to
fa35755
Compare
@akrabat My apologies, I think I've missed the previous comment and I have been quite busy so I wasn't checking up on it either. Updated, added test & rebased. I think I've done that correctly, feedback is welcome :) |
Thanks @sg3s! Fancy putting the same patch into https://github.com/slimphp/Slim-Http ? |
This should resolve #2387 if accepted.
(btw, I think you should use trailing commas for array lists, but of course didn't add those to stay inline with the rest)