Issues

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

Description

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

h2. Symptoms

The header:


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:


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:



                $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


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:


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

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

Comments

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

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:


   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:



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 http://framework.zend.com/issues/browse/ZF2-169
      */
     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.