Skip to content

[Bug] 72071 - Prevent Max-Age from being negative #2452

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

Closed
wants to merge 2 commits into from

Conversation

duncan3dc
Copy link
Member

As reported in the below bug report:
https://bugs.php.net/bug.php?id=72071

int diff;
diff = difftime(expires, time(NULL));
if (diff < 0) {
diff = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe emit a warning/notice?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a warning type event though is it? An expires at date in the past is valid, it's just that "max-age" shouldn't be negative

Copy link
Contributor

Choose a reason for hiding this comment

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

hm you are right... the docs state that a value in the past should be used when the cookies needs to be deleted. therefore a warning/notice doesnt fit in here.

leave it as is ;)

Copy link
Member

Choose a reason for hiding this comment

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

Is it a problem that this makes the expires and Max-Age inconsistent?

Copy link
Member

Choose a reason for hiding this comment

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

@krakjoe krakjoe added the Bug label Apr 4, 2017
@nikic
Copy link
Member

nikic commented Apr 9, 2017

Merged as ba6561d, thanks!

@nikic nikic closed this Apr 9, 2017
@duncan3dc duncan3dc deleted the bug-72071 branch April 28, 2019 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants