r80372 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r80371‎ | r80372 | r80373 >
Date:00:54, 15 January 2011
Author:kaldari
Status:ok (Comments)
Tags:
Comment:
more future-proof method of retrieving domain parameter (and less-likely to interfere with CentralAuth)
Modified paths:
  • /trunk/extensions/CentralNotice/CentralNotice.php (modified) (history)
  • /trunk/extensions/CentralNotice/SpecialHideBanners.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/CentralNotice.php
——@@ -61,10 +61,7 @@
6262
6363 // Domain to set global cookies for.
6464 // Example: '.wikipedia.org'
65 -// This setting is currently shared with CentralAuth extension.
66 -if ( !isset( $wgCentralAuthCookieDomain )) {
67 - $wgCentralAuthCookieDomain = '';
68 -}
 65+$wgNoticeCookieDomain = '';
6966
7067 $wgExtensionFunctions[] = 'efCentralNoticeSetup';
7168
Index: trunk/extensions/CentralNotice/SpecialHideBanners.php
——@@ -5,7 +5,7 @@
66 }
77
88 /**
9 - * Unlisted Special Page to set cookies for hiding banners across all wikis.
 9+ * Unlisted Special Page which sets a cookie for hiding banners across all languages of a project.
1010 */
1111 class SpecialHideBanners extends UnlistedSpecialPage {
1212 function __construct() {
——@@ -27,9 +27,14 @@
2828 }
2929
3030 function setHideCookie() {
31 - global $wgCentralAuthCookieDomain, $wgCookieSecure, $wgCookieHttpOnly;
 31+ global $wgNoticeCookieDomain, $wgCookieSecure, $wgCookieHttpOnly;
3232 $exp = time() + 86400 * 14; // Cookie expires after 2 weeks
 33+ if ( is_callable( 'CentralAuthUser', 'getCookieDomain' ) ) {
 34+ $cookieDomain = CentralAuthUser::getCookieDomain();
 35+ } else {
 36+ $cookieDomain = $wgNoticeCookieDomain;
 37+ }
3338 // Hide banners for this domain
34 - setcookie( 'hidesnmessage', '1', $exp, '/', $wgCentralAuthCookieDomain, $wgCookieSecure, $wgCookieHttpOnly );
 39+ setcookie( 'hidesnmessage', '1', $exp, '/', $cookieDomain, $wgCookieSecure, $wgCookieHttpOnly );
3540 }
3641 }

Comments

#Comment by 😂 (talk | contribs)   21:40, 19 January 2011

Should probably put the $wgCookieHttpOnly logic behind wfHttpOnlySafe() like WebResponse::setcookie().

I'd say to use the wrapper entirely and skip the manual setcookie() call, but our wrapper doesn't allow for overriding the cookie domain.

#Comment by Kaldari (talk | contribs)   22:05, 19 January 2011

Problem is I need to always override the cookie domain, and never use HTTP-only - since the cookie needs to be accessible in Javascript. Basically, I'm trying to do exactly what HTTP-only is designed to prevent - mucking with things cross-site :)

#Comment by 😂 (talk | contribs)   22:21, 19 January 2011

Then you shouldn't pass the last parameter at all (it defaults to false, unless session.cookie_httponly is true in php.ini).

The default for $wgCookieHttpOnly in 5.2 and above is true.

#Comment by Kaldari (talk | contribs)   22:37, 19 January 2011

Already ahead of you (r80584) :)

Status & tagging log