From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 29 Sep 2015 02:09:07 +0000 (-0400)
Subject: FauxResponse: Honor cookie options, and other cleanup
X-Git-Tag: 1.31.0-rc.0~9657^2
X-Git-Url: https://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=c3dc3985148583c6fe3eeaee682f5b8cb0c45b89;p=lhc%2Fweb%2Fwiklou.git

FauxResponse: Honor cookie options, and other cleanup

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
---

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 9059987595..dac482cbc8 100644
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -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,
 		) );
 	}
diff --git a/includes/User.php b/includes/User.php
index 3dcd4804f8..4123eb2720 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -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 );
 	}
 
 	/**
diff --git a/includes/WebResponse.php b/includes/WebResponse.php
index 1b6947cd96..bb7682d3e8 100644
--- a/includes/WebResponse.php
+++ b/includes/WebResponse.php
@@ -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;
+	}
 }
diff --git a/tests/phpunit/includes/FauxResponseTest.php b/tests/phpunit/includes/FauxResponseTest.php
index 39a0effa97..8c104d9706 100644
--- a/tests/phpunit/includes/FauxResponseTest.php
+++ b/tests/phpunit/includes/FauxResponseTest.php
@@ -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' );
 	}
 
 	/**