php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Doc Bug #72997 Set-Cookie header lost if we do session_regenerate_id() without setcookie call
Submitted: 2016-09-01 20:35 UTC Modified: 2016-10-19 05:25 UTC
Votes:5
Avg. Score:5.0 ± 0.0
Reproduced:4 of 4 (100.0%)
Same Version:3 (75.0%)
Same OS:3 (75.0%)
From: rmpic30 at gmail dot com Assigned: yohgaki (profile)
Status: Assigned Package: Session related
PHP Version: Irrelevant OS: Irrelevant
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: rmpic30 at gmail dot com
New email:
PHP Version: OS:

 

 [2016-09-01 20:35 UTC] rmpic30 at gmail dot com
Description:
------------
PHP does not send the `Set-Cookie` header, if we call `session_regenerate_id()` AND do not call `setcookie()` function for adding additional cookies.

Here is plain PHP version and Symfony3 & ZF Diactoros versions. Both do not work as expected.

Plain PHP version: https://gist.github.com/anonymous/6b4a906273f489e95e2dfac3c247c68c

Symfony version: https://gist.github.com/anonymous/88e52bd7876378b0f490ed15d30b43fe

How to reproduce (for plain PHP version):

1) Run this script on any web-server. It should return one `Set-Cookie` header with a new session.
2) Repeat your request with issued session. No extra `Set-Cookie` headers should be.
3) Run this script with parameter `set_cookie` with value of your current domain. You will see only ONE `Set-Cookie` header, header with the new PHPSESSID is lost.

Now comment out line 18, and uncomment line 21 in this https://gist.github.com/anonymous/6b4a906273f489e95e2dfac3c247c68c version

Repeat all steps again. On step #3 you will see TWO `Set-Cookie` headers as expected.

$ php -v
PHP 7.0.10-2+deb.sury.org~xenial+1 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.10-2+deb.sury.org~xenial+1, Copyright (c) 1999-2016, by Zend Technologies



Test script:
---------------
Plain PHP version: https://gist.github.com/anonymous/6b4a906273f489e95e2dfac3c247c68c

Symfony version: https://gist.github.com/anonymous/88e52bd7876378b0f490ed15d30b43fe


Expected result:
----------------
I should see two `Set-Cookie` headers on step 3.

Actual result:
--------------
I see only one `Set-Cookie` header on step 3.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-10-17 11:47 UTC] yohgaki@php.net
-Status: Open +Status: Assigned -Operating System: Ubuntu Xenial Xerus 16.04 +Operating System: Irrelevant -PHP Version: 7.0.10 +PHP Version: Irrelevant -Assigned To: +Assigned To: yohgaki
 [2016-10-17 11:47 UTC] yohgaki@php.net
A little surprised by this bug. Verified.
I suppose there is something wrong in header buffer handling.
 [2016-10-18 05:05 UTC] yohgaki@php.net
-Status: Assigned +Status: Analyzed
 [2016-10-18 05:05 UTC] yohgaki@php.net
I narrowed down what's wrong.

(Use CGI binary to check HTTP response headers)
https://gist.github.com/yohgaki/295434c924aeaaa898689b92086378e5

Problem is that header() function removes all of previously defined 'Set-Cookie' headers blindly. header() removes any 'Set-Cookie' headers always. 

header() removes any 'Header' previously stored and it is not limited to 'Set-Cookie' header.

I checked with git PHP 5.6. Header list operation is broken in a way to remove previous 'Header' always with header().

To work around this bug, use header() function first, then use session functions and/or setcookie(). Or do not use set HTTP header by header() that has the same name.

Fix would be easy. It seems someone wrote code that remove 'Header' always with header().
 [2016-10-18 06:31 UTC] yohgaki@php.net
Found commit efd671f242e87e3301a1b3
Date:   Wed Apr 4 16:14:28 2012 +0800

It's 4 years old. I'll ask the committer for intention.
 [2016-10-18 06:45 UTC] yohgaki@php.net
There was phpt commit also. I understand why this is made. There could be multiple headers for the same name, and there are cases that we would like to remove them all.

Set-Cookie is a special cookie. I wrote cookie removal code in session module for session ID cookie. It searches not only 'Set-Cookie', but also search cookie name to avoid removing other Set-Cookie header.

It seems I need to ask internal list for resolution.
For the time being, do not user header() for 'Set-Cookie'.
 [2016-10-19 05:25 UTC] yohgaki@php.net
-Status: Analyzed +Status: Assigned -Type: Bug +Type: Documentation Problem
 [2016-10-19 05:25 UTC] yohgaki@php.net
I realized that there is bug in my test script. I updated gist.
header() remove flag is Ok.

You should not call header('Set-Cookie: something') unless you really want to remove session ID cookie also. In stead, specify remove flag to false.

header('Set-Cookie: something', false);

I think this would be better to be documented in header()/session_start()/session_regenerate_id() reference.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 05:01:29 2024 UTC