Merge "Stop mangling $_GET and provide WebRequest::getQueryValuesOnly()"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 6 Sep 2019 17:38:26 +0000 (17:38 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 6 Sep 2019 17:38:26 +0000 (17:38 +0000)
1  2 
RELEASE-NOTES-1.34
includes/WebRequest.php
includes/api/ApiBase.php

diff --combined RELEASE-NOTES-1.34
@@@ -26,13 -26,6 +26,13 @@@ For notes on 1.33.x and older releases
  
  === Configuration changes for system administrators in 1.34 ===
  
 +In an effort to enforce best practices for passwords, MediaWiki will now warn
 +users, and suggest that they change their password, if it is in the list of
 +100,000 commonly used passwords that are considered bad passwords. If you want
 +to disable this for your users, please add the following to your local settings:
 +
 +$wgPasswordPolicy['policies']['default']['PasswordNotInLargeBlacklist'] = false;
 +
  ==== New configuration ====
  * $wgAllowExternalReqID (T201409) - This configuration setting controls whether
    Mediawiki accepts the request ID set by the incoming request via the
@@@ -73,7 -66,6 +73,7 @@@
    which was deprecated in 1.30, no longer works. Instead, $wgProxyList should be
    an array with IP addresses as the values, or a string path to a file
    containing one IP address per line.
 +* $wgCookieSetOnAutoblock and $wgCookieSetOnIpBlock are now enabled by default.
  * …
  
  ==== Removed configuration ====
  
  ==== Changed external libraries ====
  * Updated Mustache from 1.0.0 to v3.0.1.
 -* Updated OOUI from v0.31.3 to v0.33.4.
 +* Updated OOUI from v0.31.3 to v0.34.0.
 +* Updated OOjs from v2.2.2 to v3.0.0.
  * Updated composer/semver from 1.4.2 to 1.5.0.
  * Updated composer/spdx-licenses from 1.4.0 to 1.5.1 (dev-only).
  * Updated mediawiki/codesniffer from 25.0.0 to 26.0.0 (dev-only).
@@@ -369,6 -360,8 +369,8 @@@ because of Phabricator reports
    initialized after calling SearchResult::initFromTitle().
  * The UserIsBlockedFrom hook is only called if a block is found first, and
    should only be used to unblock a blocked user.
+ * Parameters for index.php from PATH_INFO, such as the title, are no longer
+   written to $_GET.
  * …
  
  === Deprecations in 1.34 ===
  * ResourceLoaderContext::getConfig and ResourceLoaderContext::getLogger have
    been deprecated. Inside ResourceLoaderModule subclasses, use the local methods
    instead. Elsewhere, use the methods from the ResourceLoader class.
 +* The 'jquery.accessKeyLabel' module has been deprecated. This jQuery
 +  plugin is now ships as part of the 'mediawiki.util' module bundle.
  * The Profiler::setTemplated and Profiler::getTemplated methods have been
    deprecated. Use Profiler::setAllowOutput and Profiler::getAllowOutput
    instead.
diff --combined includes/WebRequest.php
@@@ -27,7 -27,6 +27,7 @@@ use MediaWiki\MediaWikiServices
  use MediaWiki\Session\Session;
  use MediaWiki\Session\SessionId;
  use MediaWiki\Session\SessionManager;
 +use Wikimedia\AtEase\AtEase;
  
  // The point of this class is to be a wrapper around super globals
  // phpcs:disable MediaWiki.Usage.SuperGlobalsUsage.SuperGlobals
   * @ingroup HTTP
   */
  class WebRequest {
-       /** @var array */
+       /**
+        * The parameters from $_GET, $_POST and the path router
+        * @var array
+        */
        protected $data;
-       /** @var array */
+       /**
+        * The parameters from $_GET. The parameters from the path router are
+        * added by interpolateTitle() during Setup.php.
+        * @var array
+        */
+       protected $queryAndPathParams;
+       /**
+        * The parameters from $_GET only.
+        */
+       protected $queryParams;
+       /**
+        * Lazy-initialized request headers indexed by upper-case header name
+        * @var array
+        */
        protected $headers = [];
  
        /**
                // POST overrides GET data
                // We don't use $_REQUEST here to avoid interference from cookies...
                $this->data = $_POST + $_GET;
+               $this->queryAndPathParams = $this->queryParams = $_GET;
        }
  
        /**
         * @return array Any query arguments found in path matches.
         */
        public static function getPathInfo( $want = 'all' ) {
 -              global $wgUsePathInfo;
                // PATH_INFO is mangled due to https://bugs.php.net/bug.php?id=31892
                // And also by Apache 2.x, double slashes are converted to single slashes.
                // So we will use REQUEST_URI if possible.
 -              $matches = [];
 -              if ( !empty( $_SERVER['REQUEST_URI'] ) ) {
 +              if ( isset( $_SERVER['REQUEST_URI'] ) ) {
                        // Slurp out the path portion to examine...
                        $url = $_SERVER['REQUEST_URI'];
                        if ( !preg_match( '!^https?://!', $url ) ) {
                                $url = 'http://unused' . $url;
                        }
 -                      Wikimedia\suppressWarnings();
 +                      AtEase::suppressWarnings();
                        $a = parse_url( $url );
 -                      Wikimedia\restoreWarnings();
 -                      if ( $a ) {
 -                              $path = $a['path'] ?? '';
 -
 -                              global $wgScript;
 -                              if ( $path == $wgScript && $want !== 'all' ) {
 -                                      // Script inside a rewrite path?
 -                                      // Abort to keep from breaking...
 -                                      return $matches;
 -                              }
 +                      AtEase::restoreWarnings();
 +                      if ( !$a ) {
 +                              return [];
 +                      }
 +                      $path = $a['path'] ?? '';
  
 -                              $router = new PathRouter;
 +                      global $wgScript;
 +                      if ( $path == $wgScript && $want !== 'all' ) {
 +                              // Script inside a rewrite path?
 +                              // Abort to keep from breaking...
 +                              return [];
 +                      }
  
 -                              // Raw PATH_INFO style
 -                              $router->add( "$wgScript/$1" );
 +                      $router = new PathRouter;
  
 -                              if ( isset( $_SERVER['SCRIPT_NAME'] )
 -                                      && preg_match( '/\.php/', $_SERVER['SCRIPT_NAME'] )
 -                              ) {
 -                                      # Check for SCRIPT_NAME, we handle index.php explicitly
 -                                      # But we do have some other .php files such as img_auth.php
 -                                      # Don't let root article paths clober the parsing for them
 -                                      $router->add( $_SERVER['SCRIPT_NAME'] . "/$1" );
 -                              }
 +                      // Raw PATH_INFO style
 +                      $router->add( "$wgScript/$1" );
  
 -                              global $wgArticlePath;
 -                              if ( $wgArticlePath ) {
 -                                      $router->add( $wgArticlePath );
 -                              }
 -
 -                              global $wgActionPaths;
 -                              if ( $wgActionPaths ) {
 -                                      $router->add( $wgActionPaths, [ 'action' => '$key' ] );
 -                              }
 +                      if ( isset( $_SERVER['SCRIPT_NAME'] )
 +                              && strpos( $_SERVER['SCRIPT_NAME'], '.php' ) !== false
 +                      ) {
 +                              // Check for SCRIPT_NAME, we handle index.php explicitly
 +                              // But we do have some other .php files such as img_auth.php
 +                              // Don't let root article paths clober the parsing for them
 +                              $router->add( $_SERVER['SCRIPT_NAME'] . "/$1" );
 +                      }
  
 -                              global $wgVariantArticlePath;
 -                              if ( $wgVariantArticlePath ) {
 -                                      $router->add( $wgVariantArticlePath,
 -                                              [ 'variant' => '$2' ],
 -                                              [ '$2' => MediaWikiServices::getInstance()->getContentLanguage()->
 -                                              getVariants() ]
 -                                      );
 -                              }
 +                      global $wgArticlePath;
 +                      if ( $wgArticlePath ) {
 +                              $router->add( $wgArticlePath );
 +                      }
  
 -                              Hooks::run( 'WebRequestPathInfoRouter', [ $router ] );
 +                      global $wgActionPaths;
 +                      $articlePaths = PathRouter::getActionPaths( $wgActionPaths, $wgArticlePath );
 +                      if ( $articlePaths ) {
 +                              $router->add( $articlePaths, [ 'action' => '$key' ] );
 +                      }
  
 -                              $matches = $router->parse( $path );
 +                      global $wgVariantArticlePath;
 +                      if ( $wgVariantArticlePath ) {
 +                              $router->add( $wgVariantArticlePath,
 +                                      [ 'variant' => '$2' ],
 +                                      [ '$2' => MediaWikiServices::getInstance()->getContentLanguage()->
 +                                      getVariants() ]
 +                              );
                        }
 -              } elseif ( $wgUsePathInfo ) {
 -                      if ( isset( $_SERVER['ORIG_PATH_INFO'] ) && $_SERVER['ORIG_PATH_INFO'] != '' ) {
 -                              // Mangled PATH_INFO
 -                              // https://bugs.php.net/bug.php?id=31892
 -                              // Also reported when ini_get('cgi.fix_pathinfo')==false
 -                              $matches['title'] = substr( $_SERVER['ORIG_PATH_INFO'], 1 );
 -
 -                      } elseif ( isset( $_SERVER['PATH_INFO'] ) && $_SERVER['PATH_INFO'] != '' ) {
 -                              // Regular old PATH_INFO yay
 -                              $matches['title'] = substr( $_SERVER['PATH_INFO'], 1 );
 +
 +                      Hooks::run( 'WebRequestPathInfoRouter', [ $router ] );
 +
 +                      $matches = $router->parse( $path );
 +              } else {
 +                      global $wgUsePathInfo;
 +                      $matches = [];
 +                      if ( $wgUsePathInfo ) {
 +                              if ( !empty( $_SERVER['ORIG_PATH_INFO'] ) ) {
 +                                      // Mangled PATH_INFO
 +                                      // https://bugs.php.net/bug.php?id=31892
 +                                      // Also reported when ini_get('cgi.fix_pathinfo')==false
 +                                      $matches['title'] = substr( $_SERVER['ORIG_PATH_INFO'], 1 );
 +                              } elseif ( !empty( $_SERVER['PATH_INFO'] ) ) {
 +                                      // Regular old PATH_INFO yay
 +                                      $matches['title'] = substr( $_SERVER['PATH_INFO'], 1 );
 +                              }
                        }
                }
  
  
                $matches = self::getPathInfo( 'title' );
                foreach ( $matches as $key => $val ) {
-                       $this->data[$key] = $_GET[$key] = $_REQUEST[$key] = $val;
+                       $this->data[$key] = $this->queryAndPathParams[$key] = $val;
                }
        }
  
        }
  
        /**
-        * Get the values passed in the query string.
+        * Get the values passed in the query string and the path router parameters.
         * No transformation is performed on the values.
         *
         * @codeCoverageIgnore
         * @return array
         */
        public function getQueryValues() {
-               return $_GET;
+               return $this->queryAndPathParams;
+       }
+       /**
+        * Get the values passed in the query string only, not including the path
+        * router parameters. This is less suitable for self-links to index.php but
+        * useful for other entry points. No transformation is performed on the
+        * values.
+        *
+        * @since 1.34
+        * @return array
+        */
+       public function getQueryValuesOnly() {
+               return $this->queryParams;
        }
  
        /**
diff --combined includes/api/ApiBase.php
@@@ -992,7 -992,7 +992,7 @@@ abstract class ApiBase extends ContextS
                        return;
                }
  
-               $queryValues = $this->getRequest()->getQueryValues();
+               $queryValues = $this->getRequest()->getQueryValuesOnly();
                $badParams = [];
                foreach ( $params as $param ) {
                        if ( $prefix !== 'noprefix' ) {
                                                }
                                                break;
                                        case 'limit':
 +                                              // Must be a number or 'max'
 +                                              if ( $value !== 'max' ) {
 +                                                      $value = (int)$value;
 +                                              }
 +                                              if ( $multi ) {
 +                                                      self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
 +                                              }
                                                if ( !$parseLimit ) {
 -                                                      // Don't do any validation whatsoever
 +                                                      // Don't do min/max validation and don't parse 'max'
                                                        break;
                                                }
                                                if ( !isset( $paramSettings[self::PARAM_MAX] )
                                                                "MAX1 or MAX2 are not defined for the limit $encParamName"
                                                        );
                                                }
 -                                              if ( $multi ) {
 -                                                      self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
 -                                              }
 -                                              $min = $paramSettings[self::PARAM_MIN] ?? 0;
 -                                              if ( $value == 'max' ) {
 +                                              if ( $value === 'max' ) {
                                                        $value = $this->getMain()->canApiHighLimits()
                                                                ? $paramSettings[self::PARAM_MAX2]
                                                                : $paramSettings[self::PARAM_MAX];
                                                        $this->getResult()->addParsedLimit( $this->getModuleName(), $value );
                                                } else {
 -                                                      $value = (int)$value;
                                                        $this->validateLimit(
                                                                $paramName,
                                                                $value,
 -                                                              $min,
 +                                                              $paramSettings[self::PARAM_MIN] ?? 0,
                                                                $paramSettings[self::PARAM_MAX],
                                                                $paramSettings[self::PARAM_MAX2]
                                                        );