From: Timo Tijhof <krinklemail@gmail.com>
Date: Fri, 19 Apr 2019 23:41:56 +0000 (+0100)
Subject: WebRequest: Change getFullRequestURL() to use local getProtocol()
X-Git-Tag: 1.34.0-rc.0~1759^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/pie.php?a=commitdiff_plain;h=7929d190b467bb9ecaec02e581c8c548b63ea8ce;p=lhc%2Fweb%2Fwiklou.git

WebRequest: Change getFullRequestURL() to use local getProtocol()

Previously it relied on wfGetServerUrl to decide the url,
which passed PROTO_CURRENT on to wfExpandUrl(), when then uses
global $wgRequest and checks getProtocol() on that.

For typical web requests, these would be the same object by
reference, but the needless indirection makes the code harder
to reason about and harder to test.

Instead, check this locally and pass on an explicit HTTP or HTTPS
to wfGetServerUrl/wfExpandUrl.

Change-Id: I797bd2f909625536c3af36f015fb2e94bf922ba9
---

diff --git a/includes/FauxRequest.php b/includes/FauxRequest.php
index 906e5e242d..ecbc6e3373 100644
--- a/includes/FauxRequest.php
+++ b/includes/FauxRequest.php
@@ -170,17 +170,6 @@ class FauxRequest extends WebRequest {
 		return $this->requestUrl;
 	}
 
-	public function getFullRequestURL() {
-		// Pass an explicit PROTO constant instead of PROTO_CURRENT so that we
-		// do not rely on state from the global $wgRequest object (which it would,
-		// via wfGetServerUrl/wfExpandUrl/$wgRequest->protocol).
-		if ( $this->protocol === 'http' ) {
-			return wfGetServerUrl( PROTO_HTTP ) . $this->getRequestURL();
-		} else {
-			return wfGetServerUrl( PROTO_HTTPS ) . $this->getRequestURL();
-		}
-	}
-
 	public function getProtocol() {
 		return $this->protocol;
 	}
diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 2a03d2dc7d..7da092f5dc 100644
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -849,12 +849,19 @@ class WebRequest {
 	 * in HTML or other output.
 	 *
 	 * If $wgServer is protocol-relative, this will return a fully
-	 * qualified URL with the protocol that was used for this request.
+	 * qualified URL with the protocol of this request object.
 	 *
 	 * @return string
 	 */
 	public function getFullRequestURL() {
-		return wfGetServerUrl( PROTO_CURRENT ) . $this->getRequestURL();
+		// Pass an explicit PROTO constant instead of PROTO_CURRENT so that we
+		// do not rely on state from the global $wgRequest object (which it would,
+		// via wfGetServerUrl/wfExpandUrl/$wgRequest->protocol).
+		if ( $this->getProtocol() === 'http' ) {
+			return wfGetServerUrl( PROTO_HTTP ) . $this->getRequestURL();
+		} else {
+			return wfGetServerUrl( PROTO_HTTPS ) . $this->getRequestURL();
+		}
 	}
 
 	/**
diff --git a/tests/phpunit/includes/WebRequestTest.php b/tests/phpunit/includes/WebRequestTest.php
index 07c307e64e..cda56603f9 100644
--- a/tests/phpunit/includes/WebRequestTest.php
+++ b/tests/phpunit/includes/WebRequestTest.php
@@ -4,16 +4,19 @@
  * @group WebRequest
  */
 class WebRequestTest extends MediaWikiTestCase {
-	protected $oldServer;
 
 	protected function setUp() {
 		parent::setUp();
 
 		$this->oldServer = $_SERVER;
+		$this->oldWgRequest = $GLOBALS['wgRequest'];
+		$this->oldWgServer = $GLOBALS['wgServer'];
 	}
 
 	protected function tearDown() {
 		$_SERVER = $this->oldServer;
+		$GLOBALS['wgRequest'] = $this->oldWgRequest;
+		$GLOBALS['wgServer'] = $this->oldWgServer;
 
 		parent::tearDown();
 	}
@@ -368,6 +371,22 @@ class WebRequestTest extends MediaWikiTestCase {
 		$this->assertSame( [ 'x' ], $req->getValueNames( [ 'y' ] ), 'Exclude keys' );
 	}
 
+	/**
+	 * @covers WebRequest
+	 */
+	public function testGetFullRequestURL() {
+		// Stub this for wfGetServerUrl()
+		$GLOBALS['wgServer'] = '//wiki.test';
+		$req = $this->getMock( WebRequest::class, [ 'getRequestURL', 'getProtocol' ] );
+		$req->method( 'getRequestURL' )->willReturn( '/path' );
+		$req->method( 'getProtocol' )->willReturn( 'https' );
+
+		$this->assertSame(
+			'https://wiki.test/path',
+			$req->getFullRequestURL()
+		);
+	}
+
 	/**
 	 * @dataProvider provideGetIP
 	 * @covers WebRequest::getIP