FauxResponse: Honor cookie options, and other cleanup
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 29 Sep 2015 02:09:07 +0000 (22:09 -0400)
committerBryanDavis <bdavis@wikimedia.org>
Tue, 29 Sep 2015 22:45:56 +0000 (22:45 +0000)
We have this nice class for unit testing cookie-setting, but the
cookie-setting method ignores all the parameters! Fix that. Also provide
accessors to check the entire set of cookie data, and the set cookies as
a whole.

While this does change the semantics of FauxRequest::getcookie() in that
the name now needs to be prefixed, no extension in Gerrit uses this
method so we should be fine.

Also clean up the case of the setCookie and getCookie methods while
we're at it. Since PHP method names are case-insensitive, this doesn't
even break compatibility with extensions.

Change-Id: Ib44a074bf9796bc0b470d557e39465792f399d30

includes/EditPage.php
includes/User.php
includes/WebResponse.php
tests/phpunit/includes/FauxResponseTest.php

index 9059987..dac482c 100644 (file)
@@ -1312,7 +1312,7 @@ class EditPage {
                }
 
                $response = RequestContext::getMain()->getRequest()->response();
-               $response->setcookie( $postEditKey, $val, time() + self::POST_EDIT_COOKIE_DURATION, array(
+               $response->setCookie( $postEditKey, $val, time() + self::POST_EDIT_COOKIE_DURATION, array(
                        'httpOnly' => false,
                ) );
        }
index 3dcd480..4123eb2 100644 (file)
@@ -3508,7 +3508,7 @@ class User implements IDBAccessObject {
                        $request = $this->getRequest();
                }
                $params['secure'] = $secure;
-               $request->response()->setcookie( $name, $value, $exp, $params );
+               $request->response()->setCookie( $name, $value, $exp, $params );
        }
 
        /**
index 1b6947c..bb7682d 100644 (file)
@@ -81,7 +81,7 @@ class WebResponse {
         *   'prefix', 'domain', and 'secure'
         * @since 1.22 Replaced $prefix, $domain, and $forceSecure with $options
         */
-       public function setcookie( $name, $value, $expire = 0, $options = array() ) {
+       public function setCookie( $name, $value, $expire = 0, $options = array() ) {
                global $wgCookiePath, $wgCookiePrefix, $wgCookieDomain;
                global $wgCookieSecure, $wgCookieExpiration, $wgCookieHttpOnly;
 
@@ -207,18 +207,74 @@ class FauxResponse extends WebResponse {
         * @param int|null $expire Ignored in this faux subclass.
         * @param array $options Ignored in this faux subclass.
         */
-       public function setcookie( $name, $value, $expire = 0, $options = array() ) {
-               $this->cookies[$name] = $value;
+       public function setCookie( $name, $value, $expire = 0, $options = array() ) {
+               global $wgCookiePath, $wgCookiePrefix, $wgCookieDomain;
+               global $wgCookieSecure, $wgCookieExpiration, $wgCookieHttpOnly;
+
+               if ( !is_array( $options ) ) {
+                       // Backwards compatibility
+                       $options = array( 'prefix' => $options );
+                       if ( func_num_args() >= 5 ) {
+                               $options['domain'] = func_get_arg( 4 );
+                       }
+                       if ( func_num_args() >= 6 ) {
+                               $options['secure'] = func_get_arg( 5 );
+                       }
+               }
+               $options = array_filter( $options, function ( $a ) {
+                       return $a !== null;
+               } ) + array(
+                       'prefix' => $wgCookiePrefix,
+                       'domain' => $wgCookieDomain,
+                       'path' => $wgCookiePath,
+                       'secure' => $wgCookieSecure,
+                       'httpOnly' => $wgCookieHttpOnly,
+                       'raw' => false,
+               );
+
+               if ( $expire === null ) {
+                       $expire = 0; // Session cookie
+               } elseif ( $expire == 0 && $wgCookieExpiration != 0 ) {
+                       $expire = time() + $wgCookieExpiration;
+               }
+
+               $this->cookies[$options['prefix'] . $name] = array(
+                       'value' => (string)$value,
+                       'expire' => (int)$expire,
+                       'path' => (string)$options['path'],
+                       'domain' => (string)$options['domain'],
+                       'secure' => (bool)$options['secure'],
+                       'httpOnly' => (bool)$options['httpOnly'],
+                       'raw' => (bool)$options['raw'],
+               );
        }
 
        /**
         * @param string $name
         * @return string|null
         */
-       public function getcookie( $name ) {
+       public function getCookie( $name ) {
+               if ( isset( $this->cookies[$name] ) ) {
+                       return $this->cookies[$name]['value'];
+               }
+               return null;
+       }
+
+       /**
+        * @param string $name
+        * @return array|null
+        */
+       public function getCookieData( $name ) {
                if ( isset( $this->cookies[$name] ) ) {
                        return $this->cookies[$name];
                }
                return null;
        }
+
+       /**
+        * @return array
+        */
+       public function getCookies() {
+               return $this->cookies;
+       }
 }
index 39a0eff..8c104d9 100644 (file)
@@ -32,13 +32,36 @@ class FauxResponseTest extends MediaWikiTestCase {
        }
 
        /**
-        * @covers FauxResponse::getcookie
-        * @covers FauxResponse::setcookie
+        * @covers FauxResponse::setCookie
+        * @covers FauxResponse::getCookie
+        * @covers FauxResponse::getCookieData
+        * @covers FauxResponse::getCookies
         */
        public function testCookie() {
-               $this->assertEquals( null, $this->response->getcookie( 'key' ), 'Non-existing cookie' );
-               $this->response->setcookie( 'key', 'val' );
-               $this->assertEquals( 'val', $this->response->getcookie( 'key' ), 'Existing cookie' );
+               $expire = time() + 100;
+               $cookie = array(
+                       'value' => 'val',
+                       'path' => '/path',
+                       'domain' => 'domain',
+                       'secure' => true,
+                       'httpOnly' => false,
+                       'raw' => false,
+                       'expire' => $expire,
+               );
+
+               $this->assertEquals( null, $this->response->getCookie( 'xkey' ), 'Non-existing cookie' );
+               $this->response->setCookie( 'key', 'val', $expire, array(
+                       'prefix' => 'x',
+                       'path' => '/path',
+                       'domain' => 'domain',
+                       'secure' => 1,
+                       'httpOnly' => 0,
+               ) );
+               $this->assertEquals( 'val', $this->response->getCookie( 'xkey' ), 'Existing cookie' );
+               $this->assertEquals( $cookie, $this->response->getCookieData( 'xkey' ),
+                       'Existing cookie (data)' );
+               $this->assertEquals( array( 'xkey' => $cookie ), $this->response->getCookies(),
+                       'Existing cookies' );
        }
 
        /**