From: Gergő Tisza <tgr.huwiki@gmail.com>
Date: Fri, 22 Jan 2016 22:29:40 +0000 (-0800)
Subject: Only delete cookies which are actually set
X-Git-Tag: 1.31.0-rc.0~8242
X-Git-Url: http://git.cyclocoop.org/data/%24self?a=commitdiff_plain;h=1ce684fcef1ee69ca0921c05081cae47f90939e5;p=lhc%2Fweb%2Fwiklou.git

Only delete cookies which are actually set

Some API clients seem to be confused by cookie deletion.
Prevent cookie deletion on the first leg of the API login sequence
(for a client with an empty cookie jar) by only emitting deletion
headers for cookies which are set in the current request.

Bug: T124252
Change-Id: I180e094ea32f951e22adab2ec87d16e5de7cef97
---

diff --git a/includes/session/CookieSessionProvider.php b/includes/session/CookieSessionProvider.php
index 7854416fa1..867c1f8c3c 100644
--- a/includes/session/CookieSessionProvider.php
+++ b/includes/session/CookieSessionProvider.php
@@ -188,7 +188,7 @@ class CookieSessionProvider extends SessionProvider {
 
 		foreach ( $cookies as $key => $value ) {
 			if ( $value === false ) {
-				$response->clearCookie( $key, $options );
+				$this->clearCookie( $request, $response, $key, $options );
 			} else {
 				if ( $extendedExpiry !== null && in_array( $key, $extendedCookies ) ) {
 					$expiry = time() + (int)$extendedExpiry;
@@ -219,15 +219,14 @@ class CookieSessionProvider extends SessionProvider {
 			'Token' => false,
 		);
 
-		$response->clearCookie(
-			$this->params['sessionName'], array( 'prefix' => '' ) + $this->cookieOptions
-		);
+		$this->clearCookie( $request, $response,  $this->params['sessionName'],
+			array( 'prefix' => '' ) + $this->cookieOptions );
 
 		foreach ( $cookies as $key => $value ) {
-			$response->clearCookie( $key, $this->cookieOptions );
+			$this->clearCookie( $request, $response, $key, $this->cookieOptions );
 		}
 
-		$response->clearCookie( 'forceHTTPS',
+		$this->clearCookie( $request, $response, 'forceHTTPS',
 			array( 'prefix' => '', 'secure' => false ) + $this->cookieOptions );
 	}
 
@@ -299,6 +298,23 @@ class CookieSessionProvider extends SessionProvider {
 		return $value;
 	}
 
+	/**
+	 * Delete a cookie. Contains an auth-specific hack.
+	 * @param \WebRequest $request
+	 * @param \WebResponse $response
+	 * @param string $key
+	 * @param array $options
+	 */
+	protected function clearCookie( $request, $response, $key, $options = array() ) {
+		global $wgCookiePrefix;
+
+		$prefix = isset( $options['prefix'] ) ? $options['prefix'] : $wgCookiePrefix;
+
+		if ( $request->getCookie( $key, $prefix ) ) {
+			$response->clearCookie( $key, $options );
+		}
+	}
+
 	/**
 	 * Return the data to store in cookies
 	 * @param User $user
diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php
index a73bf7c098..46fb0ddd12 100644
--- a/tests/phpunit/includes/session/CookieSessionProviderTest.php
+++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php
@@ -379,10 +379,10 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 		$request = new \FauxRequest();
 		$provider->persistSession( $backend, $request );
 		$this->assertSame( $sessionId, $request->response()->getCookie( 'MySessionName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xUserID' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'xUserName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserID' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserName' ) );
+		$this->assertNull( $request->response()->getCookie( 'xToken' ) );
+		$this->assertNull( $request->response()->getCookie( 'forceHTTPS' ) );
 		$this->assertSame( array(), $backend->getData() );
 
 		// Logged-in user, no remember
@@ -394,8 +394,8 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 		$this->assertSame( $sessionId, $request->response()->getCookie( 'MySessionName' ) );
 		$this->assertSame( (string)$user->getId(), $request->response()->getCookie( 'xUserID' ) );
 		$this->assertSame( $user->getName(), $request->response()->getCookie( 'xUserName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+		$this->assertNull( $request->response()->getCookie( 'xToken' ) );
+		$this->assertNull( $request->response()->getCookie( 'forceHTTPS' ) );
 		$this->assertSame( array(), $backend->getData() );
 
 		// Logged-in user, remember
@@ -484,9 +484,9 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 			'xUserName' => array(
 				'value' => $user->getName(),
 			) + $defaults,
-			'xToken' => array(
-				'value' => $remember ? $user->getToken() : '',
-				'expire' => $remember ? $extendedExpiry : -31536000,
+			'xToken' => !$remember ? null : array(
+				'value' => $user->getToken(),
+				'expire' => $extendedExpiry,
 			) + $defaults,
 			'forceHTTPS' => !$secure ? null : array(
 				'value' => 'true',
@@ -568,10 +568,10 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 		$request = new \FauxRequest();
 		$provider->persistSession( $backend, $request );
 		$this->assertSame( $sessionId, $request->response()->getCookie( 'MySessionName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xUserID' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'xUserName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserID' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserName' ) );
+		$this->assertNull( $request->response()->getCookie( 'xToken' ) );
+		$this->assertNull( $request->response()->getCookie( 'forceHTTPS' ) );
 		$this->assertSame( array(), $backend->getData() );
 
 		$provider->persistSession( $backend, $this->getSentRequest() );
@@ -606,8 +606,8 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 		$this->assertSame( $sessionId, $request->response()->getCookie( 'MySessionName' ) );
 		$this->assertSame( (string)$user->getId(), $request->response()->getCookie( 'xUserID' ) );
 		$this->assertSame( $user->getName(), $request->response()->getCookie( 'xUserName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+		$this->assertNull( $request->response()->getCookie( 'xToken' ) );
+		$this->assertNull( $request->response()->getCookie( 'forceHTTPS' ) );
 		$this->assertSame( 'bar!', $request->response()->getCookie( 'xbar' ) );
 		$this->assertSame( (string)$loggedOut, $request->response()->getCookie( 'xLoggedOut' ) );
 		$this->assertEquals( array(
@@ -675,11 +675,11 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
 
 		$request = new \FauxRequest();
 		$provider->unpersistSession( $request );
-		$this->assertSame( '', $request->response()->getCookie( 'MySessionName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xUserID' ) );
-		$this->assertSame( null, $request->response()->getCookie( 'xUserName' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-		$this->assertSame( '', $request->response()->getCookie( 'forceHTTPS' ) );
+		$this->assertNull( $request->response()->getCookie( 'MySessionName' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserID' ) );
+		$this->assertNull( $request->response()->getCookie( 'xUserName' ) );
+		$this->assertNull( $request->response()->getCookie( 'xToken' ) );
+		$this->assertNull( $request->response()->getCookie( 'forceHTTPS' ) );
 
 		$provider->unpersistSession( $this->getSentRequest() );
 	}