From 7f8d016f5a1bdb05355198a1e0dcf76a5d4b378c Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 19 May 2016 20:31:19 -0700
Subject: [PATCH] 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
---
 includes/MediaWiki.php   |  6 +++---
 includes/WebRequest.php  | 34 ++++++++++++++++++++++++----------
 includes/api/ApiMain.php | 14 ++++++--------
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index ff469e4e39..55f9e9ea8d 100644
--- 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 2333c78559..152a3d2d19 100644
--- 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 9c54eaced9..93a5b60440 100644
--- 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__ );
 		}
 	}
 
-- 
2.20.1