Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97295
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.178 as permitted sender)
MIME-Version: 1.0
Sender: morrison.levi@gmail.com
In-Reply-To: <CAGa2bXbsgQvAuLF_gdZShFP8RbJhtJ8njYi2q=Y977B8EDz=BA@mail.gmail.com>
References: <CAGa2bXbsgQvAuLF_gdZShFP8RbJhtJ8njYi2q=Y977B8EDz=BA@mail.gmail.com>
Date: Mon, 5 Dec 2016 15:50:50 -0700
Message-ID: <CAFMT4NqLQiXK0=WUQ5NW+YrFSVUXJvn5gk3FFi0dC4MNtgLf_Q@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC][VOTE] User defined session serializer
From: levim@php.net (Levi Morrison)

On Sun, Dec 4, 2016 at 6:44 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi all,
>
> This RFC exposes session serializer interface to user space. It works
> like user defined session save handler.
>
> Users are able to encrypt/validate session data transparently. e.g.
> You can save encrypted session data to database, decrypt encrypted
> data from database transparently.
>
> https://wiki.php.net/rfc/user_defined_session_serializer
> Vote starts: 2016-12-05 Vote ends: 2016-12-19 UTC 23:59:59
>
> Although we don't have consensus about number of votes to pass, I set
> this RFC to require 2/3 votes.
>
> Questions are welcomed if you have.
> Thank you for voting.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohgaki@ohgaki.net
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php

I apologize for the late review of your RFC. This RFC has a few issues
which I think need to be fixed (yes, this means I am asking you to
pull it out of voting phase).

It does not specify how to indicate an encoding or decoding error. As
these are unexpected and userland code may throw exceptions anyway I
am fine with an exception being used here (as opposed to returning
null). However in its current form it is incomplete.

The RFC says this about the return value of the encoding function:

    Returning anything other than string raises E_RECOVERABLE_ERROR

And similarly this for the decoding function:

   Returning anything other than array raises E_RECOVERABLE_ERROR

These are not consistent with return type checking. This *must* reuse
the error checking that is already in-place and not provide something
new to accomplish the same thing.