Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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
This commit is contained in:
Timo Tijhof 2019-04-20 00:41:56 +01:00
parent 9d225ee603
commit 7929d190b4
3 changed files with 29 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
includes/FauxRequest.php
View file

@ -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;
}

11
includes/WebRequest.php
View file

@ -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();
}
}
/**

21
tests/phpunit/includes/WebRequestTest.php
View file

@ -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