8000 [BC break] Exception thrown when "session save path is ignored without a handler service" · Issue #32837 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[BC break] Exception thrown when "session save path is ignored without a handler service" #32837

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.

Sign up for GitHub

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

Jump to bottom
Closed
luispabon opened this issue Jul 31, 2019 · 11 comments
Closed

[BC break] Exception thrown when "session save path is ignored without a handler service" #32837

luispabon opened this issue Jul 31, 2019 · 11 comments
Labels
BC Break Bug FrameworkBundle Status: Needs Review
Milestone
3.4

Comments

@luispabon
Copy link
Contributor
luispabon commented Jul 31, 2019 •
edited by nicolas-grekas
Loading

Symfony version(s) affected: 3.4.30

Description
Upon update from 3.4.29 to 3.4.30 a new exception is being thrown by framework bundle when save_path is used at framework.session when handler is set to null. This breaks previously working config (site doesn't use sessions).

It was introduced here: #31620

How to reproduce
https://github.com/phpdocker-io/phpdocker.io/blob/master/app/config/config.yml#L30

That was IIRC default config. Site doesn't use sessions.

Possible Solution
Remove the LogicException and instead throw a deprecation warning.
@nicolas-grekas
Copy link
Member

/cc @gnat42 maybe?

@gnat42
Copy link
Contributor
gnat42 commented Jul 31, 2019

Definitely related - I'm not sure what the best course of action is.

@yceruto yceruto added this to the 3.4 milestone Jul 31, 2019
@gnat42
Copy link
Contributor
gnat42 commented Jul 31, 2019

So when I submitted the PR, I did mention that I wasn't sure if it was considered a BC, but that the default setting has two problems.

  • When the session handler is null the save_path is ignored. This may be fine and still work however
  • When php is run as a non-standard user like can be done in an php-fpm, session files can't be stored and everything silently stops working.

Thus, it is a BC but no one addressed that part of my PR/question and then it got merged. The solution to me is to fix the default to either remove the save_path or change the handler_id. Obviously now that this is in there it may some issues. Like just reported. I hit the same because my dev and prod weren't 100% identical.

I'm willing to help with fixing it but would need some guidance on what should be done I wasn't evern 100% sure what the proper solution was as you could see in my messages in #31620. Totally open to ideas or clarification on what should have happened there.

@luispabon
Copy link
Contributor Author
luispabon commented Aug 1, 2019 •
edited
Loading

As a user I totally get where that change's coming from.

So I would recommend reverting from 3.4 & 4.2+ but not master and issuing a deprecation warning on 4.4.

@javiereguiluz
Copy link
Member

I can confirm this BC break too when updating from Symfony 4.3.0 to 4.3.3:

image

@nicolas-grekas
Copy link
Member

I like @luispabon's proposal - anyone up for doing the PRs?

@luispabon
Copy link
Contributor Author

I can do it.

@luispabon
Copy link
Contributor Author

What would be the procedure to do this? Open separate PRs per branch? Please bear with me as my contribs to symfony can be counted in the 1s

@nicolas-grekas
Copy link
Member

One PR for 3.4 for the revert, one for 4.4 for the deprecation. The one for master after they're both merged.

@luispabon
Copy link
Contributor Author
luispabon commented Aug 13, 2019 •
edited
Loading

Thank you. There would be no PR for master would it? We're keeping the current behaviour as is for 5.0+

@luispabon luispabon mentioned this issue Aug 13, 2019
Closed
@xabbuh xabbuh mentioned this issue Aug 15, 2019
Closed
@nicolas-grekas
Copy link
Member
7528 nicolas-grekas commented Aug 22, 2019

Offending commit revert in 1d71149

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BC Break Bug FrameworkBundle Status: Needs Review
Projects
None yet
Milestone
3.4
Development

No branches or pull requests
7 participants
@javiereguiluz @nicolas-grekas @gnat42 @xabbuh @yceruto @luispabon @carsonbot
0