[Form] Changing CSRF token default name #60537
Conversation
|
Is it worth an entry in CHANGELOG? If yes, as bug or feature? |
|
On my side, I think it's fine merging as "minor". Let's see if anyone else thinks different. |
|
I am not convinced. This has the potential to break applications where people render forms by referencing the CSRF field explicitly with the „old“ name. |
|
I think this deserves being mentioned in the changelog, to account for the fact it might impact projects accessing the field by name. Note that this PR does not change the name for users of the fullstack framework as you haven't changed the default value in the FrameworkBundle config. |
| @@ -253,7 +253,7 @@ private function addFormSection(ArrayNodeDefinition $rootNode, callable $enableI | |||
| ->children() | |||
| ->scalarNode('enabled')->defaultNull()->end() // defaults to framework.csrf_protection.enabled | |||
| ->scalarNode('token_id')->defaultNull()->end() | |||
| ->scalarNode('field_name')->defaultValue('_token')->end() | |||
| ->scalarNode('field_name')->defaultValue('_csrf_token')->end() | |||
There was a problem hiding this comment.
I am still 👎 on this change as I don't think it's worth it potentially breaking applications just to change the name of this field.
If we really want to change the default name we should IMO follow the usual deprecation-first approach: Meaning here that not setting the value explicitly would trigger a deprecation which would then allow us to change the default in 8.0.
|
Thanks, I now changed it there too (and in the tests), and created a |
|
Hi, I tested this on a project where the field is rendered manually, and as @xabbuh pointed out, it does break the token verification. |
|
Let's close then. Thanks for proposing @ThomasLandauer but this looks like too costly for too little benefits. |
See #60534 (comment)