r40530 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r40529‎ | r40530 | r40531 >
Date:12:38, 6 September 2008
Author:demon
Status:old
Tags:
Comment:
* Add getCookie() method to WebRequest as a wrapper for $_COOKIE. Updated all instances of $_COOKIE to use this.
* Switch from running fix_magic_quotes() on $_COOKIE and $_GET/$_POST to running it on $this->cookies and $this->data. Should keep us from interfering with other programs that might do the same (and/or trying to start up a second WebRequest object). This partially fixes bug 11558.
* Todo: Do similar things with $_SERVER/$_ENV and switch to a lazy-load style, rather than on every new WebRequest.
Modified paths:
  • /trunk/phase3/includes/Setup.php (modified) (history)
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/includes/WebRequest.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/User.php
——@@ -747,7 +747,7 @@
748748 function loadDefaults( $name = false ) {
749749 wfProfileIn( __METHOD__ );
750750
751 - global $wgCookiePrefix;
 751+ global $wgRequest;
752752
753753 $this->mId = 0;
754754 $this->mName = $name;
——@@ -757,8 +757,8 @@
758758 $this->mEmail = '';
759759 $this->mOptions = null; # Defer init
760760
761 - if ( isset( $_COOKIE[$wgCookiePrefix.'LoggedOut'] ) ) {
762 - $this->mTouched = wfTimestamp( TS_MW, $_COOKIE[$wgCookiePrefix.'LoggedOut'] );
 761+ if ( !is_null( $wgRequest->getCookie('LoggedOut') ) ) {
 762+ $this->mTouched = wfTimestamp( TS_MW, $wgRequest->getCookie('LoggedOut') );
763763 } else {
764764 $this->mTouched = '0'; # Allow any pages to be cached
765765 }
——@@ -789,7 +789,7 @@
790790 * @return \type{\bool} True if the user is logged in, false otherwise.
791791 */
792792 private function loadFromSession() {
793 - global $wgMemc, $wgCookiePrefix;
 793+ global $wgMemc, $wgRequest;
794794
795795 $result = null;
796796 wfRunHooks( 'UserLoadFromSession', array( $this, &$result ) );
——@@ -804,8 +804,8 @@
805805 $this->loadDefaults();
806806 return false;
807807 }
808 - } else if ( isset( $_COOKIE["{$wgCookiePrefix}UserID"] ) ) {
809 - $sId = intval( $_COOKIE["{$wgCookiePrefix}UserID"] );
 808+ } else if ( !is_null( $wgRequest->getCookie( 'UserID' ) ) ) {
 809+ $sId = intval( $wgRequest->getCookie( 'UserID' ) );
810810 $_SESSION['wsUserID'] = $sId;
811811 } else {
812812 $this->loadDefaults();
——@@ -813,8 +813,8 @@
814814 }
815815 if ( isset( $_SESSION['wsUserName'] ) ) {
816816 $sName = $_SESSION['wsUserName'];
817 - } else if ( isset( $_COOKIE["{$wgCookiePrefix}UserName"] ) ) {
818 - $sName = $_COOKIE["{$wgCookiePrefix}UserName"];
 817+ } else if ( !is_null( $wgRequest->getCookie( 'UserName' ) ) ) {
 818+ $sName = $wgRequest->getCookie( 'UserName' );
819819 $_SESSION['wsUserName'] = $sName;
820820 } else {
821821 $this->loadDefaults();
——@@ -831,8 +831,8 @@
832832 if ( isset( $_SESSION['wsToken'] ) ) {
833833 $passwordCorrect = $_SESSION['wsToken'] == $this->mToken;
834834 $from = 'session';
835 - } else if ( isset( $_COOKIE["{$wgCookiePrefix}Token"] ) ) {
836 - $passwordCorrect = $this->mToken == $_COOKIE["{$wgCookiePrefix}Token"];
 835+ } else if ( !is_null( $wgRequest->getCookie( 'Token' ) ) ) {
 836+ $passwordCorrect = $this->mToken == $wgRequest->getCookie( 'Token' );
837837 $from = 'cookie';
838838 } else {
839839 # No session or persistent login cookie
Index: trunk/phase3/includes/Setup.php
——@@ -238,7 +238,7 @@
239239 if( !wfIniGetBool( 'session.auto_start' ) )
240240 session_name( $wgSessionName ? $wgSessionName : $wgCookiePrefix . '_session' );
241241
242 -if( !$wgCommandLineMode && ( $wgRequest->checkSessionCookie() || isset( $_COOKIE[$wgCookiePrefix.'Token'] ) ) ) {
 242+if( !$wgCommandLineMode && ( $wgRequest->checkSessionCookie() || !is_null( $wgRequest->getCookie('Token') ) ) ) {
243243 wfIncrStats( 'request_with_session' );
244244 wfSetupSession();
245245 $wgSessionStarted = true;
Index: trunk/phase3/includes/WebRequest.php
——@@ -46,16 +46,18 @@
4747 var $data = array();
4848 var $headers;
4949 private $_response;
 50+ private $cookies = array();
5051
5152 function __construct() {
52 - /// @fixme This preemptive de-quoting can interfere with other web libraries
53 - /// and increases our memory footprint. It would be cleaner to do on
54 - /// demand; but currently we have no wrapper for $_SERVER etc.
55 - $this->checkMagicQuotes();
56 -
5753 // POST overrides GET data
5854 // We don't use $_REQUEST here to avoid interference from cookies...
5955 $this->data = wfArrayMerge( $_GET, $_POST );
 56+ $this->cookies = $_COOKIE;
 57+
 58+ /// @fixme This preemptive de-quoting increases our memory footprint.
 59+ /// It would be cleaner to do on demand; but currently we have no
 60+ /// wrapper for $_SERVER etc.
 61+ $this->checkMagicQuotes();
6062 }
6163
6264 /**
——@@ -183,10 +185,9 @@
184186 */
185187 function checkMagicQuotes() {
186188 if ( function_exists( 'get_magic_quotes_gpc' ) && get_magic_quotes_gpc() ) {
187 - $this->fix_magic_quotes( $_COOKIE );
 189+ $this->fix_magic_quotes( $this->cookies );
188190 $this->fix_magic_quotes( $_ENV );
189 - $this->fix_magic_quotes( $_GET );
190 - $this->fix_magic_quotes( $_POST );
 191+ $this->fix_magic_quotes( $this->data );
191192 $this->fix_magic_quotes( $_REQUEST );
192193 $this->fix_magic_quotes( $_SERVER );
193194 }
——@@ -399,6 +400,23 @@
400401 }
401402
402403 /**
 404+ * Get a cookie that has been sent through fix_magic_quotes().
 405+ * $wgCookiePrefix added before requesting, so no need to do
 406+ * it yourself.
 407+ *
 408+ * @param string $key Key of the cookie name
 409+ * @param bool $addPrefix Whether to append $wgCookiePrefix (ie: most of the time)
 410+ * @return mixed (value or null if not found)
 411+ */
 412+ function getCookie( $key, $addPrefix = true ) {
 413+ if ( $addPrefix ) {
 414+ global $wgCookiePrefix;
 415+ $key = $wgCookiePrefix . $key;
 416+ }
 417+ return isset( $this->cookies[$key] ) ? $this->cookies[$key] : null;
 418+ }
 419+
 420+ /**
403421 * Returns true if there is a session cookie set.
404422 * This does not necessarily mean that the user is logged in!
405423 *
——@@ -410,7 +428,7 @@
411429 * @return bool
412430 */
413431 function checkSessionCookie() {
414 - return isset( $_COOKIE[session_name()] );
 432+ return !is_null( $this->getCookie( session_name(), false ) );
415433 }
416434
417435 /**
Index: trunk/phase3/includes/specials/SpecialUserlogin.php
——@@ -742,7 +742,7 @@
743743 */
744744 function mainLoginForm( $msg, $msgtype = 'error' ) {
745745 global $wgUser, $wgOut, $wgAllowRealName, $wgEnableEmail;
746 - global $wgCookiePrefix, $wgAuth, $wgLoginLanguageSelector;
 746+ global $wgRequest, $wgAuth, $wgLoginLanguageSelector;
747747 global $wgAuth, $wgEmailConfirmToEdit, $wgCookieExpiration;
748748
749749 $titleObj = SpecialPage::getTitleFor( 'Userlogin' );
——@@ -767,7 +767,7 @@
768768 if ( $wgUser->isLoggedIn() ) {
769769 $this->mName = $wgUser->getName();
770770 } else {
771 - $this->mName = isset( $_COOKIE[$wgCookiePrefix.'UserName'] ) ? $_COOKIE[$wgCookiePrefix.'UserName'] : null;
 771+ $this->mName = isset( $wgRequest->getCookie('UserName') ) ? $wgRequest->getCookie('UserName') : null;
772772 }
773773 }
774774

Status & tagging log