Merge "Set context when using UserrightsPage"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 25 May 2013 07:59:47 +0000 (07:59 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 25 May 2013 07:59:47 +0000 (07:59 +0000)
13 files changed:
includes/Article.php
includes/DefaultSettings.php
includes/Preferences.php
includes/Sanitizer.php
includes/db/LoadBalancer.php
includes/job/JobQueueRedis.php
includes/parser/CoreParserFunctions.php
includes/profiler/Profiler.php
maintenance/Maintenance.php
maintenance/fileOpPerfTest.php
maintenance/runJobs.php
tests/parser/parserTests.txt
tests/phpunit/includes/SanitizerTest.php

index a0d4438..dc57911 100644 (file)
@@ -855,11 +855,11 @@ class Article implements Page {
        /**
         * Get the robot policy to be used for the current view
         * @param string $action the action= GET parameter
-        * @param $pOutput ParserOutput
+        * @param $pOutput ParserOutput|null
         * @return Array the policy that should be set
         * TODO: actions other than 'view'
         */
-       public function getRobotPolicy( $action, $pOutput ) {
+       public function getRobotPolicy( $action, $pOutput = null ) {
                global $wgArticleRobotPolicies, $wgNamespaceRobotPolicies, $wgDefaultRobotPolicy;
 
                $ns = $this->getTitle()->getNamespace();
@@ -1142,6 +1142,13 @@ class Article implements Page {
                        $this->getContext()->getRequest()->response()->header( "HTTP/1.1 404 Not Found" );
                }
 
+               if ( $validUserPage ) {
+                       // Also apply the robot policy for nonexisting user pages (as those aren't served as 404)
+                       $policy = $this->getRobotPolicy( 'view' );
+                       $outputPage->setIndexPolicy( $policy['index'] );
+                       $outputPage->setFollowPolicy( $policy['follow'] );
+               }
+
                $hookResult = wfRunHooks( 'BeforeDisplayNoArticleText', array( $this ) );
 
                if ( ! $hookResult ) {
index 9221784..173605c 100644 (file)
@@ -3497,8 +3497,9 @@ $wgNoFollowDomainExceptions = array();
 $wgAllowDisplayTitle = true;
 
 /**
- * For consistency, restrict DISPLAYTITLE to titles that normalize to the same
- * canonical DB key.
+ * For consistency, restrict DISPLAYTITLE to text that normalizes to the same
+ * canonical DB key. Also disallow some inline CSS rules like display: none;
+ * which can cause the text to be hidden or unselectable.
  */
 $wgRestrictDisplayTitle = true;
 
index a6d530f..848cd32 100644 (file)
@@ -229,10 +229,14 @@ class Preferences {
                        'section' => 'personal/info',
                );
 
+               $editCount = Linker::link( SpecialPage::getTitleFor( "Contributions", $userName ),
+                       $lang->formatNum( $user->getEditCount() ) );
+
                $defaultPreferences['editcount'] = array(
                        'type' => 'info',
+                       'raw' => true,
                        'label-message' => 'prefs-edits',
-                       'default' => $lang->formatNum( $user->getEditCount() ),
+                       'default' => $editCount,
                        'section' => 'personal/info',
                );
 
index ed01235..b4a1c62 100644 (file)
@@ -813,9 +813,10 @@ class Sanitizer {
        /**
         * Pick apart some CSS and check it for forbidden or unsafe structures.
         * Returns a sanitized string. This sanitized string will have
-        * character references and escape sequences decoded, and comments
-        * stripped. If the input is just too evil, only a comment complaining
-        * about evilness will be returned.
+        * character references and escape sequences decoded and comments
+        * stripped (unless it is itself one valid comment, in which case the value
+        * will be passed through). If the input is just too evil, only a comment
+        * complaining about evilness will be returned.
         *
         * Currently URL references, 'expression', 'tps' are forbidden.
         *
@@ -856,19 +857,24 @@ class Sanitizer {
                $value = preg_replace_callback( $decodeRegex,
                        array( __CLASS__, 'cssDecodeCallback' ), $value );
 
-               // Remove any comments; IE gets token splitting wrong
-               // This must be done AFTER decoding character references and
-               // escape sequences, because those steps can introduce comments
-               // This step cannot introduce character references or escape
-               // sequences, because it replaces comments with spaces rather
-               // than removing them completely.
-               $value = StringUtils::delimiterReplace( '/*', '*/', ' ', $value );
-
-               // Remove anything after a comment-start token, to guard against
-               // incorrect client implementations.
-               $commentPos = strpos( $value, '/*' );
-               if ( $commentPos !== false ) {
-                       $value = substr( $value, 0, $commentPos );
+               // Let the value through if it's nothing but a single comment, to
+               // allow other functions which may reject it to pass some error
+               // message through.
+               if ( !preg_match( '! ^ \s* /\* [^*\\/]* \*/ \s* $ !x', $value ) ) {
+                       // Remove any comments; IE gets token splitting wrong
+                       // This must be done AFTER decoding character references and
+                       // escape sequences, because those steps can introduce comments
+                       // This step cannot introduce character references or escape
+                       // sequences, because it replaces comments with spaces rather
+                       // than removing them completely.
+                       $value = StringUtils::delimiterReplace( '/*', '*/', ' ', $value );
+
+                       // Remove anything after a comment-start token, to guard against
+                       // incorrect client implementations.
+                       $commentPos = strpos( $value, '/*' );
+                       if ( $commentPos !== false ) {
+                               $value = substr( $value, 0, $commentPos );
+                       }
                }
 
                // Reject problematic keywords and control characters
@@ -932,14 +938,7 @@ class Sanitizer {
                $decoded = Sanitizer::decodeTagAttributes( $text );
                $stripped = Sanitizer::validateTagAttributes( $decoded, $element );
 
-               $attribs = array();
-               foreach ( $stripped as $attribute => $value ) {
-                       $encAttribute = htmlspecialchars( $attribute );
-                       $encValue = Sanitizer::safeEncodeAttribute( $value );
-
-                       $attribs[] = "$encAttribute=\"$encValue\"";
-               }
-               return count( $attribs ) ? ' ' . implode( ' ', $attribs ) : '';
+               return Sanitizer::safeEncodeTagAttributes( $stripped );
        }
 
        /**
@@ -1139,6 +1138,24 @@ class Sanitizer {
                return $attribs;
        }
 
+       /**
+        * Build a partial tag string from an associative array of attribute
+        * names and values as returned by decodeTagAttributes.
+        *
+        * @param $assoc_array Array
+        * @return String
+        */
+       public static function safeEncodeTagAttributes( $assoc_array ) {
+               $attribs = array();
+               foreach ( $assoc_array as $attribute => $value ) {
+                       $encAttribute = htmlspecialchars( $attribute );
+                       $encValue = Sanitizer::safeEncodeAttribute( $value );
+
+                       $attribs[] = "$encAttribute=\"$encValue\"";
+               }
+               return count( $attribs ) ? ' ' . implode( ' ', $attribs ) : '';
+       }
+
        /**
         * Pick the appropriate attribute value from a match set from the
         * attribs regex matches.
index 12e493a..db709b5 100644 (file)
@@ -479,6 +479,7 @@ class LoadBalancer {
 
                # Operation-based index
                if ( $i == DB_SLAVE ) {
+                       $this->mLastError = 'Unknown error'; // reset error string
                        $i = $this->getReaderIndex( false, $wiki );
                        # Couldn't find a working server in getReaderIndex()?
                        if ( $i === false ) {
index 8250d2b..a3f847f 100644 (file)
@@ -54,7 +54,7 @@
  *
  * @ingroup JobQueue
  * @ingroup Redis
- * @since 1.21
+ * @since 1.22
  */
 class JobQueueRedis extends JobQueue {
        /** @var RedisConnectionPool */
index be945f7..375ff2b 100644 (file)
@@ -363,22 +363,43 @@ class CoreParserFunctions {
        static function displaytitle( $parser, $text = '' ) {
                global $wgRestrictDisplayTitle;
 
-               #parse a limited subset of wiki markup (just the single quote items)
+               // parse a limited subset of wiki markup (just the single quote items)
                $text = $parser->doQuotes( $text );
 
-               #remove stripped text (e.g. the UNIQ-QINU stuff) that was generated by tag extensions/whatever
+               // remove stripped text (e.g. the UNIQ-QINU stuff) that was generated by tag extensions/whatever
                $text = preg_replace( '/' . preg_quote( $parser->uniqPrefix(), '/' ) . '.*?'
                        . preg_quote( Parser::MARKER_SUFFIX, '/' ) . '/', '', $text );
 
-               #list of disallowed tags for DISPLAYTITLE
-               #these will be escaped even though they are allowed in normal wiki text
+               // list of disallowed tags for DISPLAYTITLE
+               // these will be escaped even though they are allowed in normal wiki text
                $bad = array( 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'div', 'blockquote', 'ol', 'ul', 'li', 'hr',
                        'table', 'tr', 'th', 'td', 'dl', 'dd', 'caption', 'p', 'ruby', 'rb', 'rt', 'rp', 'br' );
 
-               #only requested titles that normalize to the actual title are allowed through
-               #if $wgRestrictDisplayTitle is true (it is by default)
-               #mimic the escaping process that occurs in OutputPage::setPageTitle
-               $text = Sanitizer::normalizeCharReferences( Sanitizer::removeHTMLtags( $text, null, array(), array(), $bad ) );
+               // disallow some styles that could be used to bypass $wgRestrictDisplayTitle
+               if ( $wgRestrictDisplayTitle ) {
+                       $htmlTagsCallback = function ( $params ) {
+                               $decoded = Sanitizer::decodeTagAttributes( $params );
+
+                               if ( isset( $decoded['style'] ) ) {
+                                       // this is called later anyway, but we need it right now for the regexes below to be safe
+                                       // calling it twice doesn't hurt
+                                       $decoded['style'] = Sanitizer::checkCss( $decoded['style'] );
+
+                                       if ( preg_match( '/(display|user-select|visibility)\s*:/i', $decoded['style'] ) ) {
+                                               $decoded['style'] = '/* attempt to bypass $wgRestrictDisplayTitle */';
+                                       }
+                               }
+
+                               $params = Sanitizer::safeEncodeTagAttributes( $decoded );
+                       };
+               } else {
+                       $htmlTagsCallback = null;
+               }
+
+               // only requested titles that normalize to the actual title are allowed through
+               // if $wgRestrictDisplayTitle is true (it is by default)
+               // mimic the escaping process that occurs in OutputPage::setPageTitle
+               $text = Sanitizer::normalizeCharReferences( Sanitizer::removeHTMLtags( $text, $htmlTagsCallback, array(), array(), $bad ) );
                $title = Title::newFromText( Sanitizer::stripAllTags( $text ) );
 
                if ( !$wgRestrictDisplayTitle ) {
index 194bafe..7ca4c2d 100644 (file)
  * @param string $functionname name of the function we will profile
  */
 function wfProfileIn( $functionname ) {
-       global $wgProfiler;
-       if ( $wgProfiler instanceof Profiler || isset( $wgProfiler['class'] ) ) {
+       if ( Profiler::$__instance === null ) { // use this directly to reduce overhead
+               Profiler::instance();
+       }
+       if ( Profiler::$__instance && !( Profiler::$__instance instanceof ProfilerStub ) ) {
                Profiler::instance()->profileIn( $functionname );
        }
 }
@@ -42,8 +44,10 @@ function wfProfileIn( $functionname ) {
  * @param string $functionname name of the function we have profiled
  */
 function wfProfileOut( $functionname = 'missing' ) {
-       global $wgProfiler;
-       if ( $wgProfiler instanceof Profiler || isset( $wgProfiler['class'] ) ) {
+       if ( Profiler::$__instance === null ) { // use this directly to reduce overhead
+               Profiler::instance();
+       }
+       if ( Profiler::$__instance && !( Profiler::$__instance instanceof ProfilerStub ) ) {
                Profiler::instance()->profileOut( $functionname );
        }
 }
@@ -115,12 +119,10 @@ class Profiler {
         * @return Profiler
         */
        public static function instance() {
-               if ( is_null( self::$__instance ) ) {
+               if ( self::$__instance === null ) {
                        global $wgProfiler;
                        if ( is_array( $wgProfiler ) ) {
                                if ( !isset( $wgProfiler['class'] ) ) {
-                                       wfDebug( __METHOD__ . " called without \$wgProfiler['class']"
-                                               . " set, falling back to ProfilerStub for safety\n" );
                                        $class = 'ProfilerStub';
                                } else {
                                        $class = $wgProfiler['class'];
@@ -129,8 +131,6 @@ class Profiler {
                        } elseif ( $wgProfiler instanceof Profiler ) {
                                self::$__instance = $wgProfiler; // back-compat
                        } else {
-                               wfDebug( __METHOD__ . ' called with bogus $wgProfiler setting,'
-                                               . " falling back to ProfilerStub for safety\n" );
                                self::$__instance = new ProfilerStub( $wgProfiler );
                        }
                }
index 98b7d47..b4df328 100644 (file)
@@ -426,6 +426,7 @@ abstract class Maintenance {
                $this->addOption( 'server', "The protocol and server name to use in URLs, e.g. " .
                                "http://en.wikipedia.org. This is sometimes necessary because " .
                                "server name detection may fail in command line scripts.", false, true );
+               $this->addOption( 'profiler', 'Set to "text" or "trace" show profiling output', false, true );
 
                # Save generic options to display them separately in help
                $this->mGenericParameters = $this->mParams;
@@ -877,6 +878,16 @@ abstract class Maintenance {
                $wgShowSQLErrors = true;
                @set_time_limit( 0 );
                $this->adjustMemoryLimit();
+
+               // Per-script profiling; useful for debugging
+               $forcedProfiler = $this->getOption( 'profiler' );
+               if ( $forcedProfiler === 'text' ) {
+                       Profiler::setInstance( new ProfilerSimpleText( array() ) );
+                       Profiler::instance()->setTemplated( true );
+               } elseif ( $forcedProfiler === 'trace' ) {
+                       Profiler::setInstance( new ProfilerSimpleTrace( array() ) );
+                       Profiler::instance()->setTemplated( true );
+               }
        }
 
        /**
index ea4be82..9dba818 100644 (file)
@@ -21,9 +21,7 @@
  * @ingroup Maintenance
  */
 
-$wgProfiler = array( 'class' => 'ProfilerSimpleText' );
 error_reporting( E_ALL );
-
 require_once __DIR__ . '/Maintenance.php';
 
 /**
@@ -44,6 +42,8 @@ class TestFileOpPerformance extends Maintenance {
        }
 
        public function execute() {
+               Profiler::setInstance( new ProfilerSimpleText( array() ) ); // clear
+
                $backend = FileBackendGroup::singleton()->get( $this->getOption( 'b1' ) );
                $this->doPerfTest( $backend );
 
@@ -52,10 +52,8 @@ class TestFileOpPerformance extends Maintenance {
                        $this->doPerfTest( $backend );
                }
 
-               $profiler = Profiler::instance();
-               $profiler->setTemplated( true );
-
-               //NOTE: as of MW1.21, $profiler->logData() is called implicitly by doMaintenance.php.
+               Profiler::instance()->setTemplated( true );
+               // NOTE: as of MW1.21, $profiler->logData() is called implicitly by doMaintenance.php.
        }
 
        protected function doPerfTest( FileBackend $backend ) {
index 0f4c184..6a6f9d2 100644 (file)
@@ -84,7 +84,7 @@ class RunJobs extends Maintenance {
                }
 
                $flags = JobQueueGroup::USE_CACHE | JobQueueGroup::USE_PRIORITY;
-               $lastTime = time();
+               $lastTime = time(); // time since last slave check
                do {
                        $job = ( $type === false )
                                ? $group->pop( JobQueueGroup::TYPE_DEFAULT, $flags )
@@ -129,6 +129,7 @@ class RunJobs extends Maintenance {
                                $timePassed = time() - $lastTime;
                                if ( $timePassed >= 5 || $timePassed < 0 ) {
                                        wfWaitForSlaves();
+                                       $lastTime = time();
                                }
                                // Don't let any queue slaves/backups fall behind
                                if ( $jobsRun > 0 && ( $jobsRun % 100 ) == 0 ) {
index d5e221a..8995593 100644 (file)
@@ -13289,6 +13289,40 @@ Screen
 </p>
 !! end
 
+!! test
+Verify that displaytitle handles inline CSS styles (bug 26547) - rejected value
+!! options
+showtitle
+title=[[Screen]]
+!! config
+wgAllowDisplayTitle=true
+wgRestrictDisplayTitle=true
+!! input
+this is not the the title
+{{DISPLAYTITLE:<span style="display: none;">s</span>creen}}
+!! result
+<span style="/* attempt to bypass $wgRestrictDisplayTitle */">s</span>creen
+<p>this is not the the title
+</p>
+!! end
+
+!! test
+Verify that displaytitle handles inline CSS styles (bug 26547) - accepted value
+!! options
+showtitle
+title=[[Screen]]
+!! config
+wgAllowDisplayTitle=true
+wgRestrictDisplayTitle=true
+!! input
+this is not the the title
+{{DISPLAYTITLE:<span style="color: red;">s</span>creen}}
+!! result
+<span style="color: red;">s</span>creen
+<p>this is not the the title
+</p>
+!! end
+
 !! test
 preload: check <noinclude> and <includeonly>
 !! options
index b745423..38c15ee 100644 (file)
@@ -227,10 +227,14 @@ class SanitizerTest extends MediaWikiTestCase {
        public static function provideCssCommentsFixtures() {
                /** array( <expected>, <css>, [message] ) */
                return array(
-                       array( ' ', '/**/' ),
+                       // Valid comments spanning entire input
+                       array( '/**/', '/**/' ),
+                       array( '/* comment */', '/* comment */' ),
+                       // Weird stuff
                        array( ' ', '/****/' ),
-                       array( ' ', '/* comment */' ),
-                       array( ' ', "\\2f\\2a foo \\2a\\2f",
+                       array( ' ', '/* /* */' ),
+                       array( 'display: block;', "display:/* foo */block;" ),
+                       array( 'display: block;', "display:\\2f\\2a foo \\2a\\2f block;",
                                'Backslash-escaped comments must be stripped (bug 28450)' ),
                        array( '', '/* unfinished comment structure',
                                'Remove anything after a comment-start token' ),