Avoid DBPerformance warnings on PURGE/TRACE requests

The former sometimes show up in the logs as they were causing
CentralAuth to use the master but the expectations treated
the request as a GET request. This makes things more
consistent.

Bug: T92357
Change-Id: I55bf3139c68f5926fe67a51cf0eb1b2ffe55d17b

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index ff469e4..55f9e9e 100644 (file)
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -667,10 +667,10 @@ class MediaWiki {
                $trxLimits = $this->config->get( 'TrxProfilerLimits' );
                $trxProfiler = Profiler::instance()->getTransactionProfiler();
                $trxProfiler->setLogger( LoggerFactory::getInstance( 'DBPerformance' ) );
-               if ( $request->wasPosted() ) {
-                       $trxProfiler->setExpectations( $trxLimits['POST'], __METHOD__ );
-               } else {
+               if ( $request->hasSafeMethod() ) {
                        $trxProfiler->setExpectations( $trxLimits['GET'], __METHOD__ );
+               } else {
+                       $trxProfiler->setExpectations( $trxLimits['POST'], __METHOD__ );
                }
 
                // If the user has forceHTTPS set to true, or if the user

diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 2333c78..152a3d2 100644 (file)
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -1249,6 +1249,26 @@ HTML;
                $this->ip = $ip;
        }
 
+       /**
+        * Check if this request uses a "safe" HTTP method
+        *
+        * Safe methods are verbs (e.g. GET/HEAD/OPTIONS) used for obtaining content. Such requests
+        * are not expected to mutate content, especially in ways attributable to the client. Verbs
+        * like POST and PUT are typical of non-safe requests which often change content.
+        *
+        * @return bool
+        * @see https://tools.ietf.org/html/rfc7231#section-4.2.1
+        * @see https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html
+        * @since 1.28
+        */
+       public function hasSafeMethod() {
+               if ( !isset( $_SERVER['REQUEST_METHOD'] ) ) {
+                       return false; // CLI mode
+               }
+
+               return in_array( $_SERVER['REQUEST_METHOD'], [ 'GET', 'HEAD', 'OPTIONS', 'TRACE' ] );
+       }
+
        /**
         * Whether this request should be identified as being "safe"
         *
@@ -1268,21 +1288,15 @@ HTML;
         * @since 1.28
         */
        public function isSafeRequest() {
-               if ( !isset( $_SERVER['REQUEST_METHOD'] ) ) {
-                       return false; // CLI mode
-               }
-
-               if ( $_SERVER['REQUEST_METHOD'] === 'POST' ) {
-                       return $this->markedAsSafe;
-               } elseif ( in_array( $_SERVER['REQUEST_METHOD'], [ 'GET', 'HEAD', 'OPTIONS' ] ) ) {
-                       return true; // HTTP "safe methods"
+               if ( $this->markedAsSafe && $this->wasPosted() ) {
+                       return true; // marked as a "safe" POST
                }
 
-               return false; // PUT/DELETE
+               return $this->hasSafeMethod();
        }
 
        /**
-        * Mark this request is identified as being nullipotent even if it is a POST request
+        * Mark this request as identified as being nullipotent even if it is a POST request
         *
         * POST requests are often used due to the need for a client payload, even if the request
         * is otherwise equivalent to a "safe method" request.

diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 9c54eac..93a5b60 100644 (file)
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -1356,15 +1356,13 @@ class ApiMain extends ApiBase {
        protected function setRequestExpectations( ApiBase $module ) {
                $limits = $this->getConfig()->get( 'TrxProfilerLimits' );
                $trxProfiler = Profiler::instance()->getTransactionProfiler();
-               if ( $this->getRequest()->wasPosted() ) {
-                       if ( $module->isWriteMode() ) {
-                               $trxProfiler->setExpectations( $limits['POST'], __METHOD__ );
-                       } else {
-                               $trxProfiler->setExpectations( $limits['POST-nonwrite'], __METHOD__ );
-                               $this->getRequest()->markAsSafeRequest();
-                       }
-               } else {
+               if ( $this->getRequest()->hasSafeMethod() ) {
                        $trxProfiler->setExpectations( $limits['GET'], __METHOD__ );
+               } elseif ( $this->getRequest()->wasPosted() && !$module->isWriteMode() ) {
+                       $trxProfiler->setExpectations( $limits['POST-nonwrite'], __METHOD__ );
+                       $this->getRequest()->markAsSafeRequest();
+               } else {
+                       $trxProfiler->setExpectations( $limits['POST'], __METHOD__ );
                }
        }