[FrameworkBundle] Change the default value of cookie_httponly #15372
Conversation
jderusse
commented
Jul 27, 2015
jderusse
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | yes |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #15303 |
| License | MIT |
| Doc PR | symfony/symfony-docs#5561 |
| @@ -468,6 +468,8 @@ UPGRADE FROM 2.x to 3.0 | |||
| interface. | |||
| The `security.csrf.token_manager` should be used instead. | |||
|
|
|||
| * The default value of the parameter `cookie_httponly` is now `true`. | |||
There was a problem hiding this comment.
I don't know if the UPGRADE guide is the best place for this, but should we also add a short phrase exaplining the consequences of this change?
|
It's a BC break because the default value changed. But it's for the own good of the application. It could be a issue on an application which retrieve the cookie value in javascript on purpose. BTW I don't see how to notify the developper: I'm 👎 for adding a deprecated notice when the parameter is not overrided. Other question, should I add a tests who asserts defaults values? |
|
@jderusse I wasn't thinking about a deprecation notice. If I understand this change right, any Symfony app that uses JavaScript to read cookies values will stop working. I was wondering if we could say that explicitly somewhere. |
|
@javiereguiluz You're right, thats why it's a BC break. Adding a deprecated notice when the parameter is not overrided will force everybody to explicitly sets this optional parameter :( |
jderusse
force-pushed
the
default_value_cookie_httponly
branch
from
db2a01b to
17c9fa7
Compare
|
👍 |
nicolas-grekas
changed the title
|
If this should be merged into the |
|
I think this change is worth the (minor) BC break it provides pre-3.0. |
| @@ -340,7 +340,7 @@ private function addSessionSection(ArrayNodeDefinition $rootNode) | |||
| ->scalarNode('cookie_path')->end() | |||
| ->scalarNode('cookie_domain')->end() | |||
| ->booleanNode('cookie_secure')->end() | |||
| ->booleanNode('cookie_httponly')->end() | |||
| ->booleanNode('cookie_httponly')->defaultValue(true)->end() | |||
There was a problem hiding this comment.
please use `defaultTrue()``instead for consistency
|
@javiereguiluz this will only break things if you use Javascript to read the session cookie. and 👍 for this change. It makes the session cookie safer by default |
jderusse
force-pushed
the
default_value_cookie_httponly
branch
from
17c9fa7 to
5425d8c
Compare
|
I set the description in both |
|
if the change is done in 2.8, there is nothing to add in |
jderusse
force-pushed
the
default_value_cookie_httponly
branch
from
5425d8c to
682a681
Compare
|
fixed |
| * The default value of the parameter `session`.`cookie_httponly` is now `true`. | ||
| It prevents scripting languages, such as JavaScript to access the cookie, | ||
| which help to reduce identity theft through XSS attacks. If your | ||
| application need to access the session cookie override this parameter: |
There was a problem hiding this comment.
And there should be a coma before override.
jderusse
force-pushed
the
default_value_cookie_httponly
branch
from
682a681 to
a7bef1e
Compare
|
Thank you @jderusse. |
…ttponly (jderusse) This PR was merged into the 2.8 branch. Discussion ---------- [FrameworkBundle] Change the default value of cookie_httponly | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15303 | License | MIT | Doc PR | symfony/symfony-docs#5561 Commits ------- a7bef1e Change the default value of cookie_httponly to fix #15303
This PR was merged into the 2.8 branch. Discussion ---------- Change default value of cookie_httponly | Q | A | ------------- | --- | Doc fix? | no | New docs? | symfony/symfony#15372 | Applies to | 2.8 | Fixed tickets | symfony/symfony#15303 Commits ------- e6c8b6c Change default value of cookie_httponly
