Hi Yasuo,

> -----Original Message-----
> From: yohg...@gmail.com [mailto:yohg...@gmail.com] On Behalf Of Yasuo
> Ohgaki
> Sent: Friday, November 13, 2015 2:31 AM
> To: Anatol Belski <a...@php.net>
> Cc: php-b...@lists.php.net
> Subject: Re: Bug #70871 [Asn]: session_regenerate_id(): Failed to create 
> session
> ID: user (path: )
> 
> Hi Anatol,
> 
> A user has problem with session_regenerate_id()
> 
> https://bugs.php.net/bug.php?id=70871&edit=1
> 
> 
> On Fri, Nov 13, 2015 at 10:20 AM,  <yohg...@php.net> wrote:
> >
> > session_regenerate_id(): Failed to create session ID: user (path:
> > <value of session.save_path>)
> >
> > This error is raised by multiple reasons. The error messages are
> > better to modify so that users can distinguish the cause. Anyway, it
> > may be raised if
> >
> >  - new session ID creation is failed. (Very unlikely)
> >  - opening save handler is failed.
> >  - session ID collision is detected and failed. (Very unlikely)
> >  - session read with new session ID is failed.
> >
> > I'll modify error messages to distinguish the cause of your problem.
> 
> I didn't expect problem like that working save handler, i.e. works with normal
> session start/commit, has problem with session_regenerate_id().
> 
> Therefore, the same error messages are used for verious errors. It seems we 
> are
> better to use distingishable error messages for above errors.
> 
> The patch would be following. It only changes error messages.
> May I commit this patch to 7.0 branch?
> 
> Regards,
> 
> [yohgaki@dev PHP-7.0]$ git diff
> diff --git a/ext/session/session.c b/ext/session/session.c index
> dd86518..5e4831c 100644
> --- a/ext/session/session.c
> +++ b/ext/session/session.c
> @@ -2074,12 +2074,12 @@ static PHP_FUNCTION(session_regenerate_id)
>         PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
>         if (!PS(id)) {
>                 PS(session_status) = php_session_none;
> -               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
> +               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create new session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
>                 RETURN_FALSE;
>         }
>         if (PS(mod)->s_open(&PS(mod_data), PS(save_path),
> PS(session_name)) == FAILURE) {
>                 PS(session_status) = php_session_none;
> -               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
> +               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create(open) session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
>                 RETURN_FALSE;
>         }
>         if (PS(use_strict_mode) && PS(mod)->s_validate_sid && @@ -2088,14
> +2088,14 @@ static PHP_FUNCTION(session_regenerate_id)
>                 PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
>                 if (!PS(id)) {
>                         PS(session_status) = php_session_none;
> -                       php_error_docref(NULL, E_RECOVERABLE_ERROR,
> "Failed to create session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
> +                       php_error_docref(NULL, E_RECOVERABLE_ERROR,
> "Failed to create session ID by collision: %s (path: %s)", PS(mod)->s_name,
> PS(save_path));
>                         RETURN_FALSE;
>                 }
>         }
>         /* Read is required to make new session data at this point. */
>         if (PS(mod)->s_read(&PS(mod_data), PS(id), &data,
> PS(gc_maxlifetime)) == FAILURE) {
>                 PS(session_status) = php_session_none;
> -               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
> +               php_error_docref(NULL, E_RECOVERABLE_ERROR, "Failed to
> create(read) session ID: %s (path: %s)", PS(mod)->s_name, PS(save_path));
>                 RETURN_FALSE;
>         }
>         if (data) {
> 
The ticket itself doesn't look like a bug at the current point. The behavior 
looks logic so far and there is no prove reproduce case for the opposite. 
Nevertheless, changing the error texts seems suitable to me. This can help to 
figure out errors on the user side with tricky cases like custom session 
handlers. I'd suggest to do that, please care about the tests as well, though. 

Thanks

Anatol


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to