From 549af8bf9158b3fff3a1b5f219094acf9bf67608 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 6 May 2016 15:25:36 -0700
Subject: [PATCH] Add WebRequest methods for determining "safe" requests

* This is useful for logic that decides what DB (master/slave) to use
  based on the nature of the web request. It could also be used to
  enforce clear read/write distinctions via exceptions if DB_MASTER
  is misused.
* Also fixed two IDEA errors while editing this class.

Bug: T134608
Change-Id: I43f4bc06c19d823d7d1ffd9cee8bbe60563c7f82
---
 includes/MediaWiki.php                      |  1 +
 includes/WebRequest.php                     | 53 ++++++++++++++++++++-
 includes/api/ApiMain.php                    |  1 +
 includes/specialpage/SpecialPageFactory.php |  1 +
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 3dd7420b88..ff469e4e39 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -487,6 +487,7 @@ class MediaWiki {
 			$trxProfiler = Profiler::instance()->getTransactionProfiler();
 			if ( $request->wasPosted() && !$action->doesWrites() ) {
 				$trxProfiler->setExpectations( $trxLimits['POST-nonwrite'], __METHOD__ );
+				$request->markAsSafeRequest();
 			}
 
 			# Let CDN cache things if we can purge them.
diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index b159f79526..2333c78559 100644
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -80,6 +80,9 @@ class WebRequest {
 	 */
 	protected $sessionId = null;
 
+	/** @var bool Whether this HTTP request is "safe" (even if it is an HTTP post) */
+	protected $markedAsSafe = false;
+
 	public function __construct() {
 		$this->requestTime = isset( $_SERVER['REQUEST_TIME_FLOAT'] )
 			? $_SERVER['REQUEST_TIME_FLOAT'] : microtime( true );
@@ -318,7 +321,7 @@ class WebRequest {
 	 *
 	 * @param string $path The URL path given from the client
 	 * @param array $bases One or more URLs, optionally with $1 at the end
-	 * @param string $key If provided, the matching key in $bases will be
+	 * @param string|bool $key If provided, the matching key in $bases will be
 	 *    passed on as the value of this URL parameter
 	 * @return array Array of URL variables to interpolate; empty if no match
 	 */
@@ -1022,7 +1025,7 @@ class WebRequest {
 	 * @param mixed $data
 	 */
 	public function setSessionData( $key, $data ) {
-		return $this->getSession()->set( $key, $data );
+		$this->getSession()->set( $key, $data );
 	}
 
 	/**
@@ -1245,4 +1248,50 @@ HTML;
 	public function setIP( $ip ) {
 		$this->ip = $ip;
 	}
+
+	/**
+	 * Whether this request should be identified as being "safe"
+	 *
+	 * This means that the client is not requesting any state changes and that database writes
+	 * are not inherently required. Ideally, no visible updates would happen at all. If they
+	 * must, then they should not be publically attributed to the end user.
+	 *
+	 * In more detail:
+	 *   - Cache populations and refreshes MAY occur.
+	 *   - Private user session updates and private server logging MAY occur.
+	 *   - Updates to private viewing activity data MAY occur via DeferredUpdates.
+	 *   - Other updates SHOULD NOT occur (e.g. modifying content assets).
+	 *
+	 * @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 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"
+		}
+
+		return false; // PUT/DELETE
+	}
+
+	/**
+	 * Mark this request is 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.
+	 *
+	 * @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 markAsSafeRequest() {
+		$this->markedAsSafe = true;
+	}
 }
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 07642c48ed..51571092a2 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -1355,6 +1355,7 @@ class ApiMain extends ApiBase {
 				$trxProfiler->setExpectations( $limits['POST'], __METHOD__ );
 			} else {
 				$trxProfiler->setExpectations( $limits['POST-nonwrite'], __METHOD__ );
+				$this->getRequest()->markAsSafeRequest();
 			}
 		} else {
 			$trxProfiler->setExpectations( $limits['GET'], __METHOD__ );
diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php
index 725c4fc581..f529720fc7 100644
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -539,6 +539,7 @@ class SpecialPageFactory {
 			$trxProfiler = Profiler::instance()->getTransactionProfiler();
 			if ( $context->getRequest()->wasPosted() && !$page->doesWrites() ) {
 				$trxProfiler->setExpectations( $trxLimits['POST-nonwrite'], __METHOD__ );
+				$context->getRequest()->markAsSafeRequest();
 			}
 		}
 
-- 
2.20.1