Hi Marco,
On Mon, Dec 5, 2016 at 10:52 PM, Marco Pivetta <ocramius@gmail.com> wrote:
> On Mon, Dec 5, 2016 at 4:31 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>> Firstly, current OO custom save handler design (use of previously used
>> internal save handler as its base class) is not good. Overriding
>> open()/close()/etc are useless, moreover harmful. Number of bugs
>> proved it is not good.
>
>
> Number of bugs? In which adapters? There are well tested session
> savehandlers out there, and I'd trade a userland bug for an internals bug
> anytime.
If you search bug db by "session crash"
https://bugs.php.net/search.php?search_for=session+crash&boolean=0&limit=30&order_by=&direction=DESC&cmd=display&status=Closed&bug_type=All&project=All&php_os=&phpver=&cve_id=&assign=&author_email=&bug_age=0&bug_updated=0
You'll find many save handler bug reports.
>> Secondly, the only use case, that current OO save handler design is
>> useful, is read()/write() override to encrypt/validate session data.
>
>
> What does this RFC do that a custom session savehandler *CANNOT*?
Currently, users cannot use arbitrary serialization.
Dirty hack (which is inefficient also) is required to do this.
>> Thirdly, I fixed _many_ of current OO API bugs, but fixing them all is
>> impossible by design.
>
>
> It would need a deprecation of the current session module, and a replacement
> with a newer, cleaner API.
Although some APIs need improvements, but basic module design is good.
If you have a design improvement idea, I appreciate it.
>> Current OO save handler API is wrong in many ways and should be
>> deprecated and replaced by clean OO API as mentioned in the RFC.
>>
>> Is there any good reasons to keep problematic API for good?
>
>
> There is a problem with adding a half-feature to fix half the problem, and
> then having to deprecate this half-feature when the new full solution is
> actually replacing it in a correct way.
Correct way would be
- deprecate problematic current OO API which uses previously defined
save handler as base object.
- implement new OO API which simply uses user defined handler.
BTW, this is not a topic of this RFC, but it's future scope.
Please don't mixed up.
This RFC provides mandatory feature that customize session data
representation. i.e. serialization format, encrypt/decrypt,
validation, etc.
>> > This is just confusing, in my opinion, and the API can be replicated in
>> > userland with a custom session save handler decorator instead.
>>
>> I may be too familiar to session module. I don't understand what's
>> confusing. Session module has 2 distinct submodules for saving and
>> serializing data. These have different tasks.
>
>
> It is confusing that we now have two ways of passing down serializers for
> sessions.
> The Redis session savehandlers already use igbinary where applicable, for
> example, so why do I need to have a second equivalent API? Providing a
> userland base session savehandler with pluggable serializers (written in
> PHP, not in C, please!) is a much better idea.
C module could do anything, but user script cannot.
Please don't mixed up C code and user script.
This RFC generalizes "serialization" and provide interface to user
script which C module cannot.
(Since C submodule can do anything, so it's possible that save
handlers to provide their own serializer interface/API. However,
what's good about this? Session module defines serializer as submodule
already. We don't need module A provide A serializer interface, module
B provides B serializer interface. It's just a mess and confusing.)
>> The RFC allows to save session data in whatever format. e.g.
>> XML/JSON/BSON/etc. This is _impossible_ by save handler submodule
>
>
> Savehandlers even save to flat DB tables, so I'm not understanding what is
> being explained here...
>
> Even in the docs, the first example shows how to add encryption to a custom
> savehandler: https://secure.php.net/manual/en/class.sessionhandler.php
>
> Adding a custom serialization is as simple as an `unserialize()` + custom
> serializer call (Yes, I can already hear all those microseconds screaming!
> The horror!)
It seems you misunderstood this RFC.
I have to say, you are wrong and your statement is FUD.
Try to write your own serializer with current API by yourself.
If you could write clean one like this,
session_set_serializer(
function ($input_array) {
return json_encode($input_array);
},
function ($input_string) {
return json_decode($input_string);
}
);
// User may use "interface" alternatively.
// Optionally, encryption/validation can be with these functions.
please show us.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
