Skip to content

[HttpFoundation] Move Session Support Away #12839

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

Closed
timglabisch opened this issue Dec 3, 2014 · 15 comments · Fixed by #38616
Closed

[HttpFoundation] Move Session Support Away #12839

timglabisch opened this issue Dec 3, 2014 · 15 comments · Fixed by #38616

Comments

@timglabisch
Copy link

most issues in the HttpFoundation Component are related to Sessions.

The HttpFoundation is described as:
The HttpFoundation component defines an object-oriented layer for the HTTP specification.

if you read the http specification there is nothing related to sessions, even more sessions are violating the idea of a stateless protocol.

Sessions are just a concept how to use cookies in combination with a server side key value store.
If you take a look at the HttpFoundation, now it knows a lot about our server side key value stores like the filesystem memcached, pdo, mongo, memcache. this has nothing todo with "an object-oriented layer for the HTTP specification".

i don't want to say that we should not support the concept of sessions. the idea is just to accept that sessions are not related to http.

consider the difference:

 $request->getSession()->get('foo')

vs

$sessionStorage->byRequest($request)->get('foo')

so it would be up to any kind of session storage to know about rfc6265

@fabpot
Copy link
Member

fabpot commented Dec 3, 2014

I like this idea. I think we have another issue about splitting the component and create a new one for sessions.

@Tobion
Copy link
Contributor

Tobion commented Dec 4, 2014

I proposed that already in #6840. The idea that sessions are not part of the HTTP specification was exactly the argument I used when discussing symfony sessions with @znerol from drupal a few weeks ago. So I'm 👍 for making a new component.

@fabpot I think it would also be best to start this new session component from scratch and deprecate the old one. This way we can fix all the design issues. Fixing all these issues with the current architecture is almost impossible without breaking bc. I would be willing to take the lead for that. It's just a matter of finding the time/resources.

@fabpot
Copy link
Member

fabpot commented Dec 4, 2014

@Tobion I agree with you and if you can take the lead, that would be wonderful. Creating a new component also mean that it can be part of 2.7, no need to wait for 3.0.

@znerol
Copy link
Contributor

znerol commented Dec 4, 2014

When rewriting the session component from scratch, it would be great if it drop support for PHP < 5.4.

@mvrhov
Copy link

mvrhov commented Dec 4, 2014

http://scache.nanona.fi/

@yguedidi
Copy link
Contributor

yguedidi commented Dec 5, 2014

👍

@kingcrunch
Copy link
Contributor

@znerol Drop support for PHP<5.4 would imply, that it cannot go into 2.7

@stof
Copy link
Member

stof commented Dec 5, 2014

@mvrhov we cannot force the usage of scache (it would severely limit the possibility to use Symfony if we introduce a hard dependency to this extension). However, it might be a good idea to support scache as a storage in the new component.

@mvrhov
Copy link

mvrhov commented Dec 6, 2014

@stof: I know that addidg scache as a default mechanism would limit the new symfony adoption. I've added the link because I find the ideas behind interesting and well thought and it solves many propblems people are having today with sessions. This is Something that should be supported by php out of the box. @jpauli

@znerol
Copy link
Contributor

znerol commented Dec 6, 2014

@kingcrunch even if it is implemented as a new component and the existing session support would be left in place (marked as deprecated)? Projects updating to 2.7 would still be free to continue using the old one (compatible to ancient PHP) without suffering BC issues. New projects starting with 2.7 simply could go with the new component which is free of the compatibility shims and legacy cruft.

@timglabisch
Copy link
Author

i am just thinking about the interface of the sessionHandler, would you prefer using optimiticLocking or try to implement a global locking? - and why?

interface SessionHandlerInterface {

  public function supports($sessionId);

  public function read($key, $default = null);

  public function writeWithOptimisticLock($key, $currentValue, $newValue);

}

@fabpot fabpot added this to the 3.0 milestone Jan 2, 2015
@ghost
Copy link

ghost commented May 13, 2015

👍 I think a new component for sessions would be the best way.

@javiereguiluz javiereguiluz changed the title [Enhancement][HttpFoundation] Move Session Support Away [HttpFoundation] Move Session Support Away Feb 7, 2016
@TomasVotruba
Copy link
Contributor

@JHGitty I agree. E.g. Laravel already did it.

What do you think about it? I might try to decouple it.

@ostrolucky
Copy link
Contributor

Related: #10557

@mfrieswyk
Copy link

@fabpot Status Bump? This issue seems stale.

nicolas-grekas added a commit that referenced this issue Jan 28, 2021
…service "session" (jderusse)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[FrameworkBundle][HttpFoundation][Security] Deprecate service "session"

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | Fix #10557 and Fix #12839
| License       | MIT
| Doc PR        | TODO

This is a attempt to deprecate service `session` and `SessionInterface`.

This PR replaces the `session` service by a `.session.do-not-use` service (used internally by Symfony) and make `session` a deprecated alias.
In Symfony 6.0 we can remove the `session` service and replace the `SessionListener` by a Factory that build the session (instead of fetching it from container)

This PR also add a short cut `RequestStack::getSession(): ?SessionInterface`

For backward compatibility the `SessionListener` is replaced by `FactorySessionListener` **only when** the user don't override the service `session` (ping @wouterj )

TODO:
- [x] Test many configuration and dependencies (ie. session disabled + csrf)
- [x] ChangeLog and Upgrade
- [x] fix tests

Commits
-------

54acc00 Deprecat service "session"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.