-
Notifications
You must be signed in to change notification settings - Fork 190
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
fix: allow special use domains by default #249
Conversation
To avoid breaking behavior the `allowSpecialUseDomain` option should have been set to `true` by default. This PR also adds tests that cover when a default `CookieStore` is created it does allow cookies with special use domains. closes #246
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @colincasey, great work 🎉
Looks good to me as well @colincasey; nice to see a fix that makes a change and adds a test. :) I've also hand-checked straight localhost creation:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Merging with single approval and discussion with @colincasey. Will work on our 4.1.1 release next. |
Hey @awaterma I am still having the same issue with v4.1.1. |
To avoid breaking behavior the
allowSpecialUseDomain
option should have been set totrue
by default.This PR also adds tests that cover when a default
CookieStore
is created it does allow cookies with special use domains.closes #246