Re: Revert session_serializer_name(), session_gc()

From: Date: Wed, 12 Mar 2014 11:30:03 +0000
Subject: Re: Revert session_serializer_name(), session_gc()
References: 1 2 3 4 5  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
Hi Andrey,

On Wed, Mar 12, 2014 at 8:04 PM, Andrey Andreev <[email protected]> wrote:

> On Wed, Mar 12, 2014 at 2:19 AM, Yasuo Ohgaki <[email protected]> wrote:
> > Hi Andrey,
> >
> > On Wed, Mar 12, 2014 at 8:54 AM, Andrey Andreev <[email protected]>
> wrote:
> >>
> >> > Users want to have contorl *when* gc is performed rather than luck.
> This
> >> > is
> >> > reasonable. IMO.
> >>
> >> I like control, it's nice and *I already have it*:
> >>
> >>     ini_set('session.gc_probability', 1);
> >>     ini_set('session.gc_divisor', 1);
> >
> >
> > Of course you can.
> > It does not look nice as API. API should have good name for it. IMO.
> >
> > session_gc() is useful for both low traffic site and high traffic site.
> > I don't see reason not to have explicit gc API. User may simply call
> >
> > session_gc();
> >
> > periodically by cron, etc.
>
> There's a very good reason not to have it, you just ignored most of my
> previous email which explained exactly that.
>

Users may abuse INI or API.
If you worry about abuse, it would be better to have API and warn users.

Users should purge deleted session data for security reasons. It is
especially important for low traffic sites. High traffic sites may need
periodic purge due
varying request rate. Not having GC API is API design bug.

If there should not be convenient API, file_get/put_contents() aren't
needed neither.

 >> >> And while session_serializer_name() is just redundant, session_gc()
> >> >> could cause performance issues.
> >> >
> >> > There is session_module_name(), why not session_serializer_name()?
> >>
> >> Why not session_cookie_lifetime()?
> >> Why not session_cache_limiter()?
> >> Why not session_entropy_file()?
> >> etc.
> >
> >
> > Those aren't changed by programs often.
>
> Oh, trust me - cookie lifetime is changed way more often than the
> serializer.
>

It's bad practice, indeed.
User should not change default 0 (session cookie - i.e. cookie only lives
in browser's memory and destroyed by browser termination)

If users need auto login or like, they should implement it properly by
using setcookie(). (i.e. Never use session ID cookie for long life session)

And how do you define often anyway? Sessions are something that are
> setup once, it's rarely the case that a programmer needs to change any
> of the session INI settings more than once in the same program.
>
> > As I wrote, 3rd party save handler implements their own serializers.
> > Not like above example, it is likely called with session_module_name().
> e.g.
> >
> > session_module_name('memcache');
> > session_serializer_name('igbinary');
>
> It's your opinion that this is "likely" done that way. I for one have
> *never* used session_module_name().
> And again, this is not something that people would write more often
> than once in the whole application.
>

It does not matter much if you use often or not.
As an API design, not having session_serializer_name() is strange.


>
> > It looks better than
> >
> > session_module_name('memcached');
> > ini_set('session.serializer_handler', 'igbinary');
>
> It doesn't look better than:
>
> ini_set('session.save_handler', 'memcached');
> ini_set('session.serialize_handler', 'igbinary');
>

I don't mind having discussion deprecating all API that modifying its INI.
I'm some where between +1 and 0 for this.

IMO, this is more explicit and more understandable for people who
> haven't read the manuals for the "custom" functions.
>
> > It's reasonable to have session_serializer_name(), since it would be used
> > with session_module_name(). API should have good name here, too.
>
> I'm again questioning whether session_module_name() is
> useful/necessary in the first place.
>

Now I think I understand your point of view.

 > We may discuss what we should do with module functions that modify it's
> INI.
> > There are number of them already. I think you need RFC for this.
>
> On this, I agree.
>

Consistency matters. I'm trying to be consistent with current way.

I'm tends to agree deprecating all INI modifying APIs ;)
Let's have the RFC, I'll vote +1 for it.

Regards,

--
Yasuo Ohgaki
[email protected]


Thread (41 messages)

« previous php.internals (#73080) next »