Improve WebResponse::setcookie

Various bits of code are not using this because it doesn't support
various use cases, e.g. session cookies, httpOnly, custom paths, etc.

Refactor it to add all those options. Also add a hook so extensions can
override the setting of the cookie.

Change-Id: Ia0c424a48d9455a8574d91631cde0f00c9882288

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 7be25fe..b888cab 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -174,6 +174,9 @@ production.
 * (bug 25931) Add Special:RandomInCategory.
 * mediawiki.util: addPortletLink now supports passing a jQuery object as nextnode.
 * <wbr> can now be used inside WikiText.
+* WebResponse::setcookie is much more featureful. Callers using PHP's
+  setcookie() or setrawcookie() should begin using this instead.
+* New hook WebResponseSetCookie, called from WebResponse::setcookie().
 
 === Bug fixes in 1.22 ===
 * Disable Special:PasswordReset when $wgEnableEmail is false. Previously one

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 99c0839..878823b 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -2711,6 +2711,13 @@ $skin: Skin object
 REQUEST_URI.
 $router: The PathRouter instance
 
+'WebResponseSetCookie': when setting a cookie in WebResponse::setcookie().
+Return false to prevent setting of the cookie.
+&$name: Cookie name passed to WebResponse::setcookie()
+&$value: Cookie value passed to WebResponse::setcookie()
+&$expire: Cookie expiration, as for PHP's setcookie()
+$options: Options passed to WebResponse::setcookie()
+
 'WikiExporter::dumpStableQuery': Get the SELECT query for "stable" revisions
 dumps. One, and only one hook should set this, and return false.
 &$tables: Database tables to use in the SELECT query

diff --git a/includes/EditPage.php b/includes/EditPage.php
index 1b59430..81c4f93 100644 (file)
--- a/includes/EditPage.php
+++ b/includes/EditPage.php
@@ -1178,20 +1178,20 @@ class EditPage {
         * marked HttpOnly. The JavaScript code converts the cookie to a wgPostEdit config
         * variable.
         *
-        * Since WebResponse::setcookie does not allow forcing HttpOnly for a single
-        * cookie, we have to use PHP's setcookie() directly.
-        *
         * We use a path of '/' since wgCookiePath is not exposed to JS
         *
         * If the variable were set on the server, it would be cached, which is unwanted
         * since the post-edit state should only apply to the load right after the save.
         */
        protected function setPostEditCookie() {
-               global $wgCookiePrefix, $wgCookieDomain;
                $revisionId = $this->mArticle->getLatest();
                $postEditKey = self::POST_EDIT_COOKIE_KEY_PREFIX . $revisionId;
 
-               setcookie( $wgCookiePrefix . $postEditKey, '1', time() + self::POST_EDIT_COOKIE_DURATION, '/', $wgCookieDomain );
+               $response = RequestContext::getMain()->getRequest()->response();
+               $response->setcookie( $postEditKey, '1', time() + self::POST_EDIT_COOKIE_DURATION, array(
+                       'path' => '/',
+                       'httpOnly' => false,
+               ) );
        }
 
        /**

diff --git a/includes/User.php b/includes/User.php
index cca0458..14a22d2 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -3113,7 +3113,9 @@ class User {
         *  null (default): Use the default ($wgCookieSecure) to set the secure attribute
         */
        protected function setCookie( $name, $value, $exp = 0, $secure = null ) {
-               $this->getRequest()->response()->setcookie( $name, $value, $exp, null, null, $secure );
+               $this->getRequest()->response()->setcookie( $name, $value, $exp, array(
+                       'secure' => $secure,
+               ) );
        }
 
        /**

diff --git a/includes/WebResponse.php b/includes/WebResponse.php
index c2193fb..ab7524c 100644 (file)
--- a/includes/WebResponse.php
+++ b/includes/WebResponse.php
@@ -42,56 +42,81 @@ class WebResponse {
         * Set the browser cookie
         * @param string $name name of cookie
         * @param string $value value to give cookie
-        * @param int $expire Unix timestamp (in seconds) when the cookie should expire.
+        * @param int|null $expire Unix timestamp (in seconds) when the cookie should expire.
         *        0 (the default) causes it to expire $wgCookieExpiration seconds from now.
-        * @param string $prefix Prefix to use, if not $wgCookiePrefix (use '' for no prefix)
-        * @param string $domain Cookie domain to use, if not $wgCookieDomain
-        * @param $forceSecure Bool:
-        *   true: force the cookie to be set with the secure attribute
-        *   false: force the cookie to be set without the secure attribute
-        *   null: use the value from $wgCookieSecure
+        *        null causes it to be a session cookie.
+        * @param array $options Assoc of additional cookie options:
+        *     prefix: string, name prefix ($wgCookiePrefix)
+        *     domain: string, cookie domain ($wgCookieDomain)
+        *     path: string, cookie path ($wgCookiePath)
+        *     secure: bool, secure attribute ($wgCookieSecure)
+        *     httpOnly: bool, httpOnly attribute ($wgCookieHttpOnly)
+        *     raw: bool, if true uses PHP's setrawcookie() instead of setcookie()
+        *   For backwards compatability, if $options is not an array then it and
+        *   the following two parameters will be interpreted as values for
+        *   'prefix', 'domain', and 'secure'
+        * @since 1.22 Replaced $prefix, $domain, and $forceSecure with $options
         */
-       public function setcookie( $name, $value, $expire = 0, $prefix = null, $domain = null, $forceSecure = null ) {
+       public function setcookie( $name, $value, $expire = 0, $options = null ) {
                global $wgCookiePath, $wgCookiePrefix, $wgCookieDomain;
                global $wgCookieSecure, $wgCookieExpiration, $wgCookieHttpOnly;
-               if ( $expire == 0 && $wgCookieExpiration != 0 ) {
-                       $expire = time() + $wgCookieExpiration;
-               }
-               if ( $prefix === null ) {
-                       $prefix = $wgCookiePrefix;
+
+               if ( !is_array( $options ) ) {
+                       // Backwards compatability
+                       $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 );
+                       }
                }
-               if ( $domain === null ) {
-                       $domain = $wgCookieDomain;
+               $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;
                }
 
-               if ( is_null( $forceSecure ) ) {
-                       $secureCookie = $wgCookieSecure;
-               } else {
-                       $secureCookie = $forceSecure;
+               // Don't mark the cookie as httpOnly if the requesting user-agent is
+               // known to have trouble with httpOnly cookies.
+               if ( !wfHttpOnlySafe() ) {
+                       $options['httpOnly'] = false;
                }
 
-               // Mark the cookie as httpOnly if $wgCookieHttpOnly is true,
-               // unless the requesting user-agent is known to have trouble with
-               // httpOnly cookies.
-               $httpOnlySafe = $wgCookieHttpOnly && wfHttpOnlySafe();
-
-               wfDebugLog( 'cookie',
-                       'setcookie: "' . implode( '", "',
-                               array(
-                                       $prefix . $name,
-                                       $value,
-                                       $expire,
-                                       $wgCookiePath,
-                                       $domain,
-                                       $secureCookie,
-                                       $httpOnlySafe ) ) . '"' );
-               setcookie( $prefix . $name,
-                       $value,
-                       $expire,
-                       $wgCookiePath,
-                       $domain,
-                       $secureCookie,
-                       $httpOnlySafe );
+               $func = $options['raw'] ? 'setrawcookie' : 'setcookie';
+
+               if ( wfRunHooks( 'WebResponseSetCookie', array( &$name, &$value, &$expire, $options ) ) ) {
+                       wfDebugLog( 'cookie',
+                               $func . ': "' . implode( '", "',
+                                       array(
+                                               $options['prefix'] . $name,
+                                               $value,
+                                               $expire,
+                                               $options['path'],
+                                               $options['domain'],
+                                               $options['secure'],
+                                               $options['httpOnly'] ) ) . '"' );
+
+                       call_user_func( $func,
+                               $options['prefix'] . $name,
+                               $value,
+                               $expire,
+                               $options['path'],
+                               $options['domain'],
+                               $options['secure'],
+                               $options['httpOnly'] );
+               }
        }
 }
 
@@ -156,11 +181,9 @@ class FauxResponse extends WebResponse {
         * @param string $name name of cookie
         * @param string $value value to give cookie
         * @param int $expire number of seconds til cookie expires (Default: 0)
-        * @param $prefix TODO DOCUMENT (Default: null)
-        * @param $domain TODO DOCUMENT (Default: null)
-        * @param $forceSecure TODO DOCUMENT (Default: null)
+        * @param array $options ignored
         */
-       public function setcookie( $name, $value, $expire = 0, $prefix = null, $domain = null, $forceSecure = null ) {
+       public function setcookie( $name, $value, $expire = 0, $options = null ) {
                $this->cookies[$name] = $value;
        }