Skip to content

[FrameworkBundle] Sessions: configurable "use_strict_mode" option for NativeSessionStorage #22730

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

Merged
merged 1 commit into from
Jun 18, 2017

Conversation

MacDada
Copy link
Contributor

@MacDada MacDada commented May 17, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

It is currently not possible to configure the use_strict_mode option for NativeSessionStorage in a proper manner.

The reason of this PR: #22352 (comment)

It could be considered a new feature, but I wish it wouldn't, as I don't want to do any ugly hacking to get it working.

What else could be done?

  • implement more options from NativeSessionStorage in the config?
  • get rid of duplication somehow (maybe a static method in NativeSessionStorage that would return the option list and could be used in FrameworkExtension?)
  • update FrameworkExtensionTest?
  • update ConfigurationTest?
  • update the docs?

I'm willing to do those if decided.

@xabbuh
Copy link
Member

xabbuh commented May 27, 2017

You will also need to update the XML schema definition.

@MacDada
Copy link
Contributor Author

MacDada commented Jun 2, 2017

You will also need to update the XML schema definition.

@xabbuh ?

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2017

What I mean is that you will have to update this block to allow to use the new option in XML configs.

MacDada added a commit to MacDada/symfony that referenced this pull request Jun 3, 2017
@fabpot
Copy link
Member

fabpot commented Jun 14, 2017

I think being able to configure all options in the config make sense (rebase on current 2.7 as we've just added 4 new options from 7.1). Doing that in a somewhat automated way would be even better if possible.

@MacDada
Copy link
Contributor Author

MacDada commented Jun 15, 2017

I don't know how could it be automated.

@MacDada MacDada force-pushed the use_strict_mode_config branch from 793474c to 90e192e Compare June 16, 2017 18:49
@MacDada
Copy link
Contributor Author

MacDada commented Jun 16, 2017

I rebased against 2.7 and squashed my commits into one.

@fabpot
Copy link
Member

fabpot commented Jun 18, 2017

Thank you @MacDada.

@fabpot fabpot merged commit 90e192e into symfony:2.7 Jun 18, 2017
fabpot added a commit that referenced this pull request Jun 18, 2017
… option for NativeSessionStorage (MacDada)

This PR was merged into the 2.7 branch.

Discussion
----------

[FrameworkBundle] Sessions: configurable "use_strict_mode" option for NativeSessionStorage

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

It is currently not possible to configure the `use_strict_mode` option for `NativeSessionStorage` in a proper manner.

The reason of this PR: #22352 (comment)

It could be considered a new feature, but I wish it wouldn't, as I don't want to do any ugly hacking to get it working.

What else could be done?

* implement more options from `NativeSessionStorage` in the config?
* get rid of duplication somehow (maybe a static method in `NativeSessionStorage` that would return the option list and could be used in `FrameworkExtension`?)
* update `FrameworkExtensionTest`?
* update `ConfigurationTest`?
* update [the docs](https://symfony.com/doc/current/reference/configuration/framework.html#session)?

I'm willing to do those if decided.

Commits
-------

90e192e Sessions: configurable "use_strict_mode" option for NativeSessionStorage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants