Issues

ZF2-169: Zend\Http\Header\SetCookie::fromString fails to parse cookie when contains unknown attributes

Issue Type: Bug Created: 2012-02-20T16:41:20.000+0000 Last Updated: 2012-03-15T00:44:57.000+0000 Status: Resolved Fix version(s): Reporter: Zoltan Toth-Czifra (gphilip) Assignee: Enrico Zimuel (zimuel) Tags: - Zend\Http

Related issues: Attachments:

Description

I have found an issue when processing HTTP responses, not all Set-Cookie headers are parsed correctly.

h2. Symptoms

The header:

<pre class="highlight">
Set-Cookie: leo_auth_token="GST:xxxxxxxxxxxxxxxxx:xxxxxxxxxxxx:xxxxxxxxxxxxxxxxx"; Version=1; Max-Age=1799; Expires=Mon, 20-Feb-2012 02:49:57 GMT; Path=/

Value after parsing:

<pre class="highlight">
Max-Age=1799; Expires=Mon, 20-Feb-2012 02:49:57 GMT; Path=/

h2. The reason

The problem is in the Zend\Http\Header\SetCookie::fromString factory method, more specifically with this piece of code:

<pre class="highlight">

                $header = new $setCookieClass;
                $keyValuePairs = preg_split('#;\s*#', $headerLine);
                foreach ($keyValuePairs as $keyValue) {
                    if (strpos($keyValue, '=')) {
                        list($headerKey, $headerValue) = preg_split('#=\s*#', $keyValue, 2);
                    } else {
                        $headerKey = $keyValue;
                        $headerValue = null;
                    }

                    switch (str_replace(array('-', '_'), '', strtolower($headerKey))) {
                        case 'expires':  $header->setExpires($headerValue); break;
                        case 'domain':   $header->setDomain($headerValue); break;
                        case 'path':     $header->setPath($headerValue); break;
                        case 'secure':   $header->setSecure(true); break;
                        case 'httponly': $header->setHttponly(true); break;
                        default:
                            $header->setName($headerKey);
                            $header->setValue($headerValue);
                    }
                }

In the switch when an unknown header is found, it replaces the original name-value pair from the beginning of the cookie. So, for

<pre class="highlight">
Set-Cookie: leo_auth_token="GST:xxxxxxxxxxxxxxxxx:xxxxxxxxxxxx:xxxxxxxxxxxxxxxxx"; Version=1; Max-Age=1799; Expires=Mon, 20-Feb-2012 02:49:57 GMT; Path=/

...first leo_auth_token is considered (correctly) as the name of the cookie. But then, in the next iterations Version is not caught by the switches, the default branch executes and overwrites the name. Then, in the next iteration the same happens with Max-Age.

Please note the both Version and Max-Age are valid attributes according to the www.ietf.org/rfc/rfc2109.txt" rel="nofollow">RFC.

As a quick fix, I propose this:

<pre class="highlight">
default:
    if (null !== $header->getName()) {
        break;
    }
    $header->setName($headerKey);
    $header->setValue($headerValue);

However, probably adding Version and Max-Age would also be necessary.

Comments

Posted by Enrico Zimuel (zimuel) on 2012-02-29T18:31:59.000+0000

I fixed the bug with this PR https://github.com/zendframework/zf2/pull/861

Posted by Adam Lundrigan (adamlundrigan) on 2012-03-14T22:35:37.000+0000

I think this issue still exists. The RFC linked to above specifies that the name/value pair must be the first K=V pair encountered:

<pre class="highlight">
   The syntax for the Set-Cookie response header is

   set-cookie      =       "Set-Cookie:" cookies
   cookies         =       1#cookie
   cookie          =       NAME "=" VALUE *(";" cookie-av)
   NAME            =       attr
   VALUE           =       value
   cookie-av       =       "Comment" "=" value
                   |       "Domain" "=" value
                   |       "Max-Age" "=" value
                   |       "Path" "=" value
                   |       "Secure"
                   |       "Version" "=" 1*DIGIT

Zend\Http\Header\SetCookie will accept name/value pairs anywhere in the header string:

<pre class="highlight">

diff --git a/tests/Zend/Http/Header/SetCookieTest.php b/tests/Zend/Http/Header/SetCookieTest.php
index b7dc7a9..9b87a3c 100644
--- a/tests/Zend/Http/Header/SetCookieTest.php
+++ b/tests/Zend/Http/Header/SetCookieTest.php
@@ -129,8 +129,7 @@ class SetCookieTest extends \PHPUnit_Framework_TestCase
     /** Implmentation specific tests here */

     /**
-     * ZF2-169
-     *
+     * @group ZF2-169
      * @see <a href="http://framework.zend.com/issues/browse/ZF2-169">http://framework.zend.com/issues/browse/ZF2-169</a>
      */
     public function testZF2_169()
@@ -139,5 +138,15 @@ class SetCookieTest extends \PHPUnit_Framework_TestCase
         $setCookieHeader = SetCookie::fromString($cookie);
         $this->assertEquals($cookie, $setCookieHeader->toString());
     }
+
+    /**
+     * @group ZF2-169
+     */
+    public function testDoesNotAcceptCookieNameFromArbitraryLocationInHeaderValue()
+    {
+        $cookie = 'Set-Cookie: Version=1; Max-Age=1799; Expires=Mon, 20-Feb-2012 02:49:57 GMT; Path=/; leo_auth_token="example"';
+        $setCookieHeader = SetCookie::fromString($cookie);
+        $this->assertNotEquals('leo_auth_token', $setCookieHeader->getName());
+    }
 }

The above test fails. I've fixed this in ZF-4520, and can issue a PR against ZF2 if necessary.

Posted by Adam Lundrigan (adamlundrigan) on 2012-03-15T00:44:57.000+0000

Pull Request: https://github.com/zendframework/zf2/pull/929

Have you found an issue?

See the Overview section for more details.

Copyright

© 2006-2016 by Zend, a Rogue Wave Company. Made with by awesome contributors.

This website is built using zend-expressive and it runs on PHP 7.

Contacts