-
Notifications
You must be signed in to change notification settings - Fork 155
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
[bug] Default MaxAge never applies #119
Labels
Comments
Sorry, I meant it was introduced in #39. |
Good catch.
Are you explicitly setting 0 in your application?
e.g. would this fix, if you pulled a fixed version, break your session
cookies?
…On Mon, Aug 12, 2019 at 11:11 AM Andrew Hodges ***@***.***> wrote:
Sorry, I meant it was introduced in #39
<#39>.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#119?email_source=notifications&email_token=AAAEQ4HQOYHCFLQWG6QJFF3QEGRWHA5CNFSM4ILDSY72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4DLXMA#issuecomment-520534960>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAEQ4BTYLC2VVPJ27K2MC3QEGRWHANCNFSM4ILDSY7Q>
.
|
We are now. We weren't before, and I have no idea what other people are doing. I suspect, based on what we did, that people who didn't specify it assumed the docs were correct, and that the default of 12 hours was in place. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Ever since #38, the default MaxAge of 12 hours has not applied, because the check ignores zero.
For us, the current behavior is fine, we actually want session lifetime, but the documentation should be updated, or the default should apply when there is no explicit option given. Perhaps a good idea would be to move the defaulting into
parseOptions
instead?Versions
We found this behavior in v1.6.0, but the bug was introduced in #38.
Steps to Reproduce
Use
csrf.Protect
without specifying theMaxAge
option.Expected behavior
The
Max-Age
property on the cookie should be whatever the documentation says the default will be.The text was updated successfully, but these errors were encountered: