psr7-sessions / storageless

:mailbox_with_mail: storage-less PSR-7 session support
MIT License
649 stars 38 forks source link

[RFC] Make session handlers implements `SessionHandlerInterface` #57

Closed malukenho closed 1 year ago

malukenho commented 8 years ago

My proposal is to make PSR7-Session implements the php's SessionHandlerInterface. By doing that we can use $_SESSION directly, or whatever other library that manage the session.

Here's an exemple of what would be the set up for each handler:

$handler = new PSR7Session\StorageLessHandler();
session_set_save_handler($handler, true);

You should be able to change the handler easy and with the bare minimum modification on your application.

Yes. I'm saying that we should get rid of the most of OOP user API, and May be write a separated session manage, or just leave the user to user whatever they want.

/cc @Ocramius @lcobucci

Ocramius commented 8 years ago

@malukenho I suggest two separate repositories:

Ocramius commented 8 years ago

Note: relying on $_SESSION directly in the code is a bad idea. We'd be building it as backport/compatibility layer, not as a real feature.

SvenRtbg commented 8 years ago

I'd oppose this proposal as long as you cannot prove that the resulting software will behave exactly identical compared to the classic files save handler, i.e. including locking and not losing any data on concurrent requests.

If it behaves differently, it should not inherit from/extend a mechanism that people have certain expectations on how it works.

malukenho commented 8 years ago

@Ocramius relying on session directly or not is up to the user IMO.

@SvenRtbg look at the interface:

interface SessionHandlerInterface
{
    /* Methods */
    public bool close();
    public bool destroy(string $session_id);
    public bool gc(int $maxlifetime);
    public bool open(string $save_path , string $name);
    public string read(string $session_id);
    public bool write(string $session_id, string $session_data);
}

That's basically what we got now. My main point in here is that it should be the most simple to use we can, and the most important thing, should be most ease to get rid of it when you want.

Ocramius commented 8 years ago

If it behaves differently, it should not inherit from/extend a mechanism that people have certain expectations on how it works.

This is perfectly OK for apps that only need auth

@Ocramius relying on session directly or not is up to the user IMO.

Relying on a superglobal should be for legacy code, not for new code.

SvenRtbg commented 8 years ago

@malukenho I known that interface. It's most overlooked requirement is that open() should aquire a lock on the storage for the session id, and that close() must release that lock again. If this is not implemented (at all or correctly), things will go wrong when people assume that it is just another session save handler that can be used just like any other old school PHP session.

Various projects including Symfony and Laravel get this wrong, and it is constantly causing pain. Let's not add to this another bad implementation of SessionHandlerInterface.

See https://github.com/symfony/symfony/issues/4976 or https://github.com/laravel/framework/issues/7549 - there even is a dedicated wrapper-fix for Laravel: https://github.com/rairlie/laravel-locking-session

allflame commented 8 years ago

@SvenRtbg I'm pretty sure locks without (shared) "storage" are impossible. There are just much more limitations to the realization of that interface that are "under the hood" (mainly the amount of data you can set in the cookie) hence there will be at most basic compatibility just "for apps that only need auth" and nothing besides that.

SvenRtbg commented 8 years ago

@allflame Are you supporting this proposal, or is your statement against it? From my view, I don't see a benefit, and your comment underlined it:

  1. Cookies are not able to store large amount of data.
  2. Locking is impossible.
  3. $_SESSION access is a bad idea.
  4. The use case seems to be a very limited "authentication data only" one.

If this is all true: Why implement SessionHandlerInterface if you cannot use the result just like a real PHP native session.

What is the use case that should be supported here? Who would use that interface implementation, and in what kind of situation?

allflame commented 8 years ago

@SvenRtbg honestly just wanted to share an opinion, as it's not up to me to decide. I see your point and probably I tend to lean more towards it, but I can understand people who want flexible on/off switch and don't want to write another copy of wrapper code just for themselves.

Ocramius commented 8 years ago

@SvenRtbg the limitations of this package are described in full in docs/limitations.md. If someone wishes to use $_SESSION in a highly concurrent write scenario, they already know what they are going to hit when using this package for session management.

Other than that, writing both the session storage adapter and the legacy layer that provides the $_SESSION super-global are trivial concerns that would still widen the scope of this project to ~90% of the PHP projects out there.