Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Fixes #95: SetCookie::setMaxAge() null, negative, and non-numeric redefinition #120

Merged
merged 3 commits into from
Oct 13, 2017
Merged

Fixes #95: SetCookie::setMaxAge() null, negative, and non-numeric redefinition #120

merged 3 commits into from
Oct 13, 2017

Conversation

samsonasik
Copy link
Contributor

I hope I don't missreading https://tools.ietf.org/html/rfc6265#section-5.2.2 document

if $maxAge = null or "string", ignore
if $maxAge = negative, set maxAge = 0
otherwise, set maxAge = $maxAge

This was referenced Apr 6, 2017
Closed
Closed
@samsonasik
Copy link
Contributor Author

Is there anything I can do to get it merged? Thank you.

@shellus
Copy link

shellus commented Jul 6, 2017

You can merge, thank you

@samsonasik
Copy link
Contributor Author

@shellus I've no merge access

Ocramius
Ocramius previously requested changes Jul 6, 2017
View reviewed changes
src/Header/SetCookie.php Outdated
}

$maxAge = (int) $maxAge;
if ($maxAge < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

$this->maxAge = max(0, (int) $maxAge)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ocramius done ;)

@Xerkus Xerkus dismissed Ocramius’s stale review October 13, 2017 08:54

Requested changes were applied

@Xerkus Xerkus changed the base branch from master to develop October 13, 2017 09:08
@Xerkus Xerkus added this to the 2.7.0 milestone Oct 13, 2017
@Xerkus Xerkus merged commit 600b72a into zendframework:develop Oct 13, 2017
Xerkus added a commit that referenced this pull request Oct 13, 2017
@Xerkus
Merge branch 'feature/120' into develop
2806810 
Close #120
@samsonasik samsonasik deleted the fix-95 branch October 13, 2017 11:13
@Xerkus Xerkus mentioned this pull request Oct 30, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Ocramius Ocramius Ocramius left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@samsonasik @shellus @Ocramius @Xerkus