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

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 3dd7420..ff469e4 100644 (file)
--- 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 b159f79..2333c78 100644 (file)
--- 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 07642c4..5157109 100644 (file)
--- 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 725c4fc..f529720 100644 (file)
--- 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();
                        }
                }