Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] session_regenerate_id() not replacing Set-Cookie header

Posted by Patrick ALLAERT 
Patrick ALLAERT
[PHP-DEV] session_regenerate_id() not replacing Set-Cookie header
November 15, 2011 11:20PM
Hello,

Calling session_regenerate_id() inside a same request will generate
multiple Set-Cookie headers

example code:
<?
session_start();
session_regenerate_id();
session_regenerate_id();
?>

will result in, e.g.:
Set-Cookie: PHPSESSID=d8afvidkqp9jd4kns8ij976o72; path=/
Set-Cookie: PHPSESSID=lkjla7kvotnfhutb43llcirj61; path=/

As per rfc6265, it seems incorrect:
"Servers SHOULD NOT include more than one Set-Cookie header field in
the same response with the same cookie-name."

And is causing errors on some Blackberry and IE8:
http://anvilstudios.co.za/blog/php/session-cookies-faulty-in-ie8/
http://supportforums.blackberry.com/t5/Web-and-WebWorks-Development/HTTPS-and-php-session-regenerate-id/m-p/125562

It looks like the culprit is in ext/session/session.c:
/* 'replace' must be 0 here, else a previous Set-Cookie
header, probably sent with setcookie() will be replaced! */
sapi_add_header_ex(ncookie.c, ncookie.len, 0, 0 TSRMLS_CC);
where 'replace' is intentionally set to 0 while everywhere else it is
called with replace = 1 (or via sapi_add_header())

Can someone explain me why we intentionally have that behavior ?

Cheers,
Patrick

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Tue, Nov 15, 2011 at 10:09 PM, Patrick ALLAERT
<[email protected]> wrote:
> Hello,
>
> Calling session_regenerate_id() inside a same request will generate
> multiple Set-Cookie headers
>
> example code:
> <?
> session_start();
> session_regenerate_id();
> session_regenerate_id();
> ?>
>
> will result in, e.g.:
> Set-Cookie: PHPSESSID=d8afvidkqp9jd4kns8ij976o72; path=/
> Set-Cookie: PHPSESSID=lkjla7kvotnfhutb43llcirj61; path=/
>
> As per rfc6265, it seems incorrect:
> "Servers SHOULD NOT include more than one Set-Cookie header field in
> the same response with the same cookie-name."
>
> And is causing errors on some Blackberry and IE8:
> http://anvilstudios.co.za/blog/php/session-cookies-faulty-in-ie8/
> http://supportforums.blackberry.com/t5/Web-and-WebWorks-Development/HTTPS-and-php-session-regenerate-id/m-p/125562
>
> It looks like the culprit is in ext/session/session.c:
> /* 'replace' must be 0 here, else a previous Set-Cookie
>  header, probably sent with setcookie() will be replaced! */
> sapi_add_header_ex(ncookie.c, ncookie.len, 0, 0 TSRMLS_CC);
> where 'replace' is intentionally set to 0 while everywhere else it is
> called with replace = 1 (or via sapi_add_header())
>
> Can someone explain me why we intentionally have that behavior ?
>

Patrick, I don't know the reason why this is, but if it's filed as a
bug then i'm happy to patch it!.
- Paul.

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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
2011/11/15 Paul Dragoonis <[email protected]>:
> On Tue, Nov 15, 2011 at 10:09 PM, Patrick ALLAERT
> <[email protected]> wrote:
>> Hello,
>>
>> Calling session_regenerate_id() inside a same request will generate
>> multiple Set-Cookie headers
>>
>> example code:
>> <?
>> session_start();
>> session_regenerate_id();
>> session_regenerate_id();
>> ?>
>>
>> will result in, e.g.:
>> Set-Cookie: PHPSESSID=d8afvidkqp9jd4kns8ij976o72; path=/
>> Set-Cookie: PHPSESSID=lkjla7kvotnfhutb43llcirj61; path=/
>>
>> As per rfc6265, it seems incorrect:
>> "Servers SHOULD NOT include more than one Set-Cookie header field in
>> the same response with the same cookie-name."
>>
>> And is causing errors on some Blackberry and IE8:
>> http://anvilstudios.co.za/blog/php/session-cookies-faulty-in-ie8/
>> http://supportforums.blackberry.com/t5/Web-and-WebWorks-Development/HTTPS-and-php-session-regenerate-id/m-p/125562
>>
>> It looks like the culprit is in ext/session/session.c:
>> /* 'replace' must be 0 here, else a previous Set-Cookie
>>  header, probably sent with setcookie() will be replaced! */
>> sapi_add_header_ex(ncookie.c, ncookie.len, 0, 0 TSRMLS_CC);
>> where 'replace' is intentionally set to 0 while everywhere else it is
>> called with replace = 1 (or via sapi_add_header())
>>
>> Can someone explain me why we intentionally have that behavior ?
>>
>
> Patrick, I don't know the reason why this is, but if it's filed as a
> bug then i'm happy to patch it!.

Well, if that's a valid bug, I could have patched it myself, the thing
is that it really looks intentional which makes me think it is not a
bug.
Hence I asked the question on internals before submitting a bug about it.

@mike

Since you are the one who introduced the comment, you might be the
best person to comment on this.

Cheers,
Patrick

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Tue, 15 Nov 2011 23:51:25 +0100, Patrick ALLAERT wrote:

>>> As per rfc6265, it seems incorrect:
>>> "Servers SHOULD NOT include more than one Set-Cookie header field in
>>> the same response with the same cookie-name."
>>>
>
> @mike
>
> Since you are the one who introduced the comment, you might be the best
> person to comment on this.
>

If you set replace to 1 it would replace any Set-Cookie header, not
necessarily the session cookie header.

Mike


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
On Wed, Nov 16, 2011 at 12:12 PM, Michael Wallner <[email protected]> wrote:

> On Tue, 15 Nov 2011 23:51:25 +0100, Patrick ALLAERT wrote:
>
> >>> As per rfc6265, it seems incorrect:
> >>> "Servers SHOULD NOT include more than one Set-Cookie header field in
> >>> the same response with the same cookie-name."
> >>>
> >
> > @mike
> >
> > Since you are the one who introduced the comment, you might be the best
> > person to comment on this.
> >
>
> If you set replace to 1 it would replace any Set-Cookie header, not
> necessarily the session cookie header.
>
> Mike
>
>
>
if we fix that, I would like to see
https://bugs.php.net/bug.php?id=38104(previously reported as
https://bugs.php.net/bug.php?id=31455) fixed also.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
On Wed, Nov 16, 2011 at 12:30 PM, Ferenc Kovacs <[email protected]> wrote:

>
>
> On Wed, Nov 16, 2011 at 12:12 PM, Michael Wallner <[email protected]> wrote:
>
>> On Tue, 15 Nov 2011 23:51:25 +0100, Patrick ALLAERT wrote:
>>
>> >>> As per rfc6265, it seems incorrect:
>> >>> "Servers SHOULD NOT include more than one Set-Cookie header field in
>> >>> the same response with the same cookie-name."
>> >>>
>> >
>> > @mike
>> >
>> > Since you are the one who introduced the comment, you might be the best
>> > person to comment on this.
>> >
>>
>> If you set replace to 1 it would replace any Set-Cookie header, not
>> necessarily the session cookie header.
>>
>> Mike
>>
>>
>>
> if we fix that, I would like to see https://bugs.php.net/bug.php?id=38104(previously reported as
> https://bugs.php.net/bug.php?id=31455) fixed also.
>
> --
> Ferenc Kovács
> @Tyr43l - http://tyrael.hu
>

bump.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Hi Stats,

Even if this bug is marked as bogus in bug DB, I think this bug needed
to be fixed for 5.4.

https://bugs.php.net/bug.php?id=38104

It seems this bug causes problem with IE that not keeping session correctly..

From RFC 6250
-------------
Servers SHOULD NOT include more than one Set-Cookie header field in
the same response with the same cookie-name. (See Section 5.2 for
how user agents handle this case.)
-------------
http://datatracker.ietf.org/doc/rfc6265/?include_text=1

It seems IE conform this standard.

According to svn log, it seems it was not fixed.
Anyone working with this issue? or already fixed?
I'm just curious.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net



2012/1/9 Ferenc Kovacs <[email protected]>:
> On Wed, Nov 16, 2011 at 12:30 PM, Ferenc Kovacs <[email protected]> wrote:
>
>>
>>
>> On Wed, Nov 16, 2011 at 12:12 PM, Michael Wallner <[email protected]> wrote:
>>
>>> On Tue, 15 Nov 2011 23:51:25 +0100, Patrick ALLAERT wrote:
>>>
>>> >>> As per rfc6265, it seems incorrect:
>>> >>> "Servers SHOULD NOT include more than one Set-Cookie header field in
>>> >>> the same response with the same cookie-name."
>>> >>>
>>> >
>>> > @mike
>>> >
>>> > Since you are the one who introduced the comment, you might be the best
>>> > person to comment on this.
>>> >
>>>
>>> If you set replace to 1 it would replace any Set-Cookie header, not
>>> necessarily the session cookie header.
>>>
>>> Mike
>>>
>>>
>>>
>> if we fix that, I would like to see https://bugs.php.net/bug.php?id=38104(previously reported as
>> https://bugs.php.net/bug.php?id=31455) fixed also.
>>
>> --
>> Ferenc Kovács
>> @Tyr43l - http://tyrael.hu
>>
>
> bump.
>
> --
> Ferenc Kovács
> @Tyr43l - http://tyrael.hu

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

> Even if this bug is marked as bogus in bug DB, I think this bug needed
> to be fixed for 5.4.
>
> https://bugs.php.net/bug.php?id=38104
>
> It seems this bug causes problem with IE that not keeping session correctly.

It looks like pretty rare scenario and doesn't seem to require any core
changes to fix, so I think we can do it in 5.4.1.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login