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

Add options for cookie domain, secure, and httponly #7

Merged
merged 3 commits into from Jan 22, 2019

Conversation

marcguyer
Copy link
Contributor

@marcguyer marcguyer commented Jan 3, 2019

This patch provides the ability to set additional options for for the generated session Set-Cookie header, including:

  • domain
  • secure
  • httponly

Changes include tests and documentation updates.

* Updated tests. Added assertions for new options.
* Updated docs with new config options and manual instantiation args.
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

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

This is great - I particularly appreciate the documentation updates!

I've noted a BC break in the constructor that needs to be addressed. Once updated, I can review and merge. Thanks, @marcguyer !

docs/book/v1/manual.md Outdated Show resolved Hide resolved
docs/book/v1/manual.md Outdated Show resolved Hide resolved
src/CacheSessionPersistence.php Outdated Show resolved Hide resolved
->withDomain($this->cookieDomain)
->withPath($this->cookiePath)
->withSecure($this->cookieSecure)
->withHttpOnly($this->cookieHttpOnly);
Copy link
Member

Choose a reason for hiding this comment

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

Quick question: will these calls be problematic if the values are null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only value that could be null is $this->cookieDomain and SetCookie::withDomain accepts a null value. Both $this->cookieSecure and $this->cookieHttpOnly are boolean values. Both SetCookie::withSecure and SetCookie::withHttpOnly only accept boolean values. In all cases, calling these methods with the default values as arguments will not change behavior.

@weierophinney weierophinney merged commit 8baae8e into zendframework:develop Jan 22, 2019
weierophinney added a commit that referenced this pull request Jan 22, 2019
weierophinney added a commit that referenced this pull request Jan 22, 2019
@weierophinney
Copy link
Member

Thanks, @marcguyer!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants