Skip to content
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

Limit the maxAge of cookies to prevent cookie errors #4931

Closed
wants to merge 1 commit into from

Conversation

bdombro
Copy link

@bdombro bdombro commented Jun 16, 2022

The cookie library now validates expires to be valid dates. This PR helps prevent unexpected errors when someone sets the maxAge to some crazy high number.

The `cookie` library [now validates expires to be valid dates](jshttp/cookie@042073f). This PR helps prevent unexpected errors when someone sets the maxAge to some crazy high number.
Copy link
Contributor

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for your pull request! This is definitely an issue, as the data input leads to a bad error message. Would you be willing to add at lest one test for this condition to our test suite? In addition, I don't think altering the user's input is expected. This should likely throw an error if the maxAge input is out of bounds for setting the cookie.

@dougwilson dougwilson added the bug label Jun 16, 2022
@bdombro
Copy link
Author

bdombro commented Jul 2, 2022

Thinking more about this, if we are okay with an error the current status quo is prob fine imo -- because the underlying cookie library throws a reasonable error. Thoughts?

@bdombro bdombro closed this Jul 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants