jenkins-bot has submitted this change and it was merged.
Change subject: Fix MobileContext::getBaseDomain() method
......................................................................
Fix MobileContext::getBaseDomain() method
Improvements around getting base domain for optin and stopMobileRedirect
cookies
Changes:
- extract logic to separate BaseDomainExtractor class
- added beta cluster handling
- improved handling WMF projects
- when no match return full domain instead of only two parts
Bug: T148975
Bug: T150768
Change-Id: Ieab107cdc32ff9d4911f3794d1662bdf80c98a2e
---
M extension.json
A includes/BaseDomainExtractorInterface.php
M includes/MobileContext.php
A includes/WMFBaseDomainExtractor.php
M tests/phpunit/MobileContextTest.php
A tests/phpunit/WMFBaseDomainExtractorTest.php
6 files changed, 204 insertions(+), 46 deletions(-)
Approvals:
Bmansurov: Looks good to me, but someone else must approve
jenkins-bot: Verified
Phuedx: Looks good to me, approved
diff --git a/extension.json b/extension.json
index 5682dbb..7bbe133 100644
--- a/extension.json
+++ b/extension.json
@@ -52,6 +52,8 @@
"ExtMobileFrontend": "includes/MobileFrontend.body.php",
"MobileFrontendHooks": "includes/MobileFrontend.hooks.php",
"MobileFrontendSkinHooks":
"includes/MobileFrontend.skin.hooks.php",
+ "MobileFrontend\\BaseDomainExtractorInterface":
"includes/BaseDomainExtractorInterface.php",
+ "MobileFrontend\\WMFBaseDomainExtractor":
"includes/WMFBaseDomainExtractor.php",
"MobileContext": "includes/MobileContext.php",
"MobileFormatter": "includes/MobileFormatter.php",
"MobileCollection": "includes/models/MobileCollection.php",
diff --git a/includes/BaseDomainExtractorInterface.php
b/includes/BaseDomainExtractorInterface.php
new file mode 100644
index 0000000..3b323e6
--- /dev/null
+++ b/includes/BaseDomainExtractorInterface.php
@@ -0,0 +1,22 @@
+<?php
+/**
+ * BaseDomainExtractorInterface.php
+ */
+namespace MobileFrontend;
+
+/**
+ * Helper for operations on domain names
+ *
+ * Interface DomainExtractorInterface
+ */
+interface BaseDomainExtractorInterface {
+
+ /**
+ * Try to extract the base domain from $server
+ * Returns $server if no base domain is found.
+ *
+ * @param string $server URL
+ * @return string Hostname
+ */
+ public function getCookieDomain( $server );
+}
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 5556cb3..cc21c88 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -5,11 +5,13 @@
use MediaWiki\MediaWikiServices;
use MobileFrontend\Devices\DeviceDetectorService;
+use MobileFrontend\WMFBaseDomainExtractor;
/**
* Provide various request-dependant methods to use in mobile context
*/
class MobileContext extends ContextSource {
+
const USEFORMAT_COOKIE_NAME = 'mf_useformat';
const USER_MODE_PREFERENCE_NAME = 'mfMode';
const LAZY_LOAD_IMAGES_COOKIE_NAME = 'mfLazyLoadImages';
@@ -397,14 +399,10 @@
$user->setOption( self::USER_MODE_PREFERENCE_NAME, $mode );
$user->saveSettings();
- $host = $this->getBaseDomain();
- // Deal with people running off localhost. see
http://curl.haxx.se/rfc/cookie_spec.html
- if ( strpos( $host, '.' ) === false ) {
- $host = false;
- }
- $this->getRequest()->response()->setcookie( 'optin', $mode, 0,
- [ 'prefix' => '', 'domain' => $host ]
- );
+ $this->getRequest()->response()->setCookie( 'optin', $mode, 0, [
+ 'prefix' => '',
+ 'domain' => $this->getCookieDomain()
+ ] );
}
/**
@@ -666,38 +664,28 @@
}
/**
- * Return the basic second level domain or just IP adress
+ * Return the base level domain or IP address
+ *
* @return string
*/
- public function getBaseDomain() {
- $server = $this->getConfig()->get( 'Server' );
-
- $parsedUrl = wfParseUrl( $server );
- $host = $parsedUrl['host'];
- // Validates value as IP address
- if ( !IP::isValid( $host ) ) {
- $domainParts = explode( '.', $host );
- $domainParts = array_reverse( $domainParts );
- // Although some browsers will accept cookies without
the initial .,
- // ยป RFC 2109 requires it to be included.
- $host = count( $domainParts ) >= 2 ? '.' .
$domainParts[1] . '.' . $domainParts[0] : $host;
- }
-
- return $host;
+ public function getCookieDomain() {
+ $helper = new WMFBaseDomainExtractor();
+ return $helper->getCookieDomain( $this->getMFConfig()->get(
'Server' ) );
}
/**
* Determine the correct domain to use for the stopMobileRedirect cookie
*
* Will use $wgMFStopRedirectCookieHost if it's set, otherwise will use
- * result of getBaseDomain()
+ * result of getCookieDomain()
* @return string
*/
public function getStopMobileRedirectCookieDomain() {
$mfStopRedirectCookieHost = $this->getMFConfig()->get(
'MFStopRedirectCookieHost' );
if ( !$mfStopRedirectCookieHost ) {
- self::$mfStopRedirectCookieHost =
$this->getBaseDomain();
+ self::$mfStopRedirectCookieHost =
$this->getCookieDomain();
+
} else {
self::$mfStopRedirectCookieHost =
$mfStopRedirectCookieHost;
}
diff --git a/includes/WMFBaseDomainExtractor.php
b/includes/WMFBaseDomainExtractor.php
new file mode 100644
index 0000000..832b852
--- /dev/null
+++ b/includes/WMFBaseDomainExtractor.php
@@ -0,0 +1,121 @@
+<?php
+/**
+ * WMFBaseDomainExtractor.php
+ */
+namespace MobileFrontend;
+
+/**
+ * Utility class to find base domain for given host.
+ *
+ * This class contains a hardcoded list of all WMF hosts and WMF specific
domain logic. As we never
+ * experienced any bug requests from users and we do not change domains too
often there is no need
+ * to put that hosts list into settings.
+ *
+ * @see T148975
+ */
+class WMFBaseDomainExtractor implements BaseDomainExtractorInterface {
+ /**
+ * @var string[]
+ */
+ private $wmfWikiHosts = [
+ 'wikipedia.org',
+ 'wikibooks.org',
+ 'wikiversity.org',
+ 'wikinews.org',
+ 'wiktionary.org',
+ 'wikisource.org',
+ 'wikiquote.org',
+ 'wikivoyage.org',
+ 'wikidata.org',
+ 'mediawiki.org',
+ 'wikimediafoundation.org',
+ 'local.wmftest.net' // local vagrant instances
+ ];
+ /**
+ * @var string[]
+ */
+ private $wmfMultiDomainWikiHosts = [
+ '.wikimedia.org', // commons, office, meta, outreach,
wikimania, incubator, etc...
+ '.beta.wmflabs.org', // beta cluster
+ '.wmflabs.org' // all other labs
+ ];
+
+ /**
+ * Try to extract the WMF base domain from $server
+ * Returns $server if no WMF base domain is found.
+ *
+ * Although some browsers will accept cookies without the initial . in
domain
+ * RFC 2109 requires it to be included.
+ *
+ * @param string $server URL
+ * @return string Hostname
+ */
+ public function getCookieDomain( $server ) {
+ $parsedUrl = wfParseUrl( $server );
+ $host = $parsedUrl['host'];
+
+ $wikiHost = $this->matchBaseHostname( $host,
$this->wmfWikiHosts );
+ if ( $wikiHost !== false ) {
+ return '.' . $wikiHost;
+ }
+
+ $multiWikiHost = $this->matchBaseHostname( $host,
$this->wmfMultiDomainWikiHosts );
+ if ( $multiWikiHost !== false ) {
+ return '.' . $this->extractSubdomain( $host,
$multiWikiHost );
+ }
+ return $host;
+ }
+
+ /**
+ * Find out whether $hostname matches or is subdomain of any host from
$hosts array
+ *
+ * @param string $hostname Visited host
+ * @param string[] $hosts Array of all wikimedia hosts
+ * @return bool|string Returns wikimedia host basedomain, false when
not found
+ */
+ private function matchBaseHostname( $hostname, array $hosts ) {
+ foreach ( $hosts as $wmfHost ) {
+ if ( $this->endsWith( $hostname, $wmfHost ) ) {
+ return $wmfHost;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Parse $host and return $baseDomain with first subdomain
+ * ex: extractSubdomain('en.commons.wikimedia.org', '.wikimedia.org')
=> 'commons.wikimedia.org'
+ *
+ * This function assumes that $fullHostname is a subdomain of
$basedomain. Please
+ * do the endsWith() check first before calling this function
+ *
+ * @param string $fullHostname
+ * @param string $baseDomain
+ * @return string
+ */
+ private function extractSubdomain( $fullHostname, $baseDomain ) {
+ $length = strlen( $baseDomain );
+ $subdomains = explode( '.', substr( $fullHostname, 0, -$length
) );
+ if ( count( $subdomains ) == 0 ) {
+ return $baseDomain;
+ }
+ $subdomain = array_pop( $subdomains );
+ return $subdomain . $baseDomain;
+
+ }
+
+ /**
+ * Check that $haystack ends with $needle
+ *
+ * @param string $haystack
+ * @param string $needle
+ * @return bool
+ */
+ private function endsWith( $haystack, $needle ) {
+ $length = strlen( $needle );
+ if ( $length === 0 ) {
+ return true;
+ }
+ return substr( $haystack, -$length ) === $needle;
+ }
+}
diff --git a/tests/phpunit/MobileContextTest.php
b/tests/phpunit/MobileContextTest.php
index ff0571f..5208208 100644
--- a/tests/phpunit/MobileContextTest.php
+++ b/tests/phpunit/MobileContextTest.php
@@ -52,26 +52,6 @@
}
/**
- * @dataProvider getBaseDomainProvider
- * @covers MobileContext::getBaseDomain
- */
- public function testGetBaseDomain( $server, $baseDomain ) {
- $this->setMwGlobals( 'wgServer', $server );
- $this->assertEquals( $baseDomain,
$this->makeContext()->getBaseDomain() );
- }
-
- public function getBaseDomainProvider() {
- return [
- [ 'https://en.wikipedia.org', '.wikipedia.org' ],
- [ 'http://en.m.wikipedia.org', '.wikipedia.org' ],
- [ '//en.m.wikipedia.org', '.wikipedia.org' ],
- [ 'http://127.0.0.1', '127.0.0.1' ],
- [ 'http://127.0.0.1:8080', '127.0.0.1' ],
- [ 'http://localhost', 'localhost' ],
- ];
- }
-
- /**
* @covers MobileContext::getMobileUrl
*/
public function testGetMobileUrl() {
diff --git a/tests/phpunit/WMFBaseDomainExtractorTest.php
b/tests/phpunit/WMFBaseDomainExtractorTest.php
new file mode 100644
index 0000000..943c20f
--- /dev/null
+++ b/tests/phpunit/WMFBaseDomainExtractorTest.php
@@ -0,0 +1,45 @@
+<?php
+
+/**
+ * @group MobileFrontend
+ */
+class WMFBaseDomainExtractorTest extends \PHPUnit_Framework_TestCase {
+ /**
+ * @dataProvider getBaseDomainProvider
+ * @covers MobileContext::getBaseDomain
+ */
+ public function testGetBaseDomain( $server, $baseDomain ) {
+ $extractor = new \MobileFrontend\WMFBaseDomainExtractor();
+ $this->assertEquals( $baseDomain, $extractor->getCookieDomain(
$server ) );
+ }
+
+ public function getBaseDomainProvider() {
+ return [
+ // Production wikis
+ [ 'http://wikipedia.org', '.wikipedia.org' ],
+ [ 'https://en.wikipedia.org', '.wikipedia.org' ],
+ [ 'http://en.m.wikipedia.org', '.wikipedia.org' ],
+ [ '//en.m.wikipedia.org', '.wikipedia.org' ],
+ [ 'http://wikimediafoundation.org',
'.wikimediafoundation.org' ],
+ [ 'http://wikiversity.org', '.wikiversity.org' ],
+ [ 'https://office.wikimedia.org',
'.office.wikimedia.org' ],
+ [ 'https://commons.wikimedia.org',
'.commons.wikimedia.org' ],
+ // Beta cluster
+ [ 'http://en.wikipedia.beta.wmflabs.org',
'.wikipedia.beta.wmflabs.org' ],
+ [ 'https://en.m.wikipedia.beta.wmflabs.org',
'.wikipedia.beta.wmflabs.org' ],
+ [ 'http://en.wikiversity.beta.wmflabs.org',
'.wikiversity.beta.wmflabs.org' ],
+ [ 'http://en.m.wikiversity.beta.wmflabs.org',
'.wikiversity.beta.wmflabs.org' ],
+ // IP address
+ [ 'http://127.0.0.1', '127.0.0.1' ],
+ [ 'http://127.0.0.1:8080', '127.0.0.1' ],
+ // Other possible domains/client instances
+ [ 'http://localhost', 'localhost' ],
+ [ 'http://mediawiki.dev', 'mediawiki.dev' ],
+ [ 'http://test.co.uk', 'test.co.uk' ],
+ [ 'http://wiki.test.com.pl', 'wiki.test.com.pl' ],
+ // Vagrant instances
+ [ 'http://php5.local.wmftest.net:8080/',
'.local.wmftest.net' ],
+ [ 'https://wiki.local.wmftest.net:8080/',
'.local.wmftest.net' ]
+ ];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/321796
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieab107cdc32ff9d4911f3794d1662bdf80c98a2e
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Pmiazga <[email protected]>
Gerrit-Reviewer: Bmansurov <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: Pmiazga <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
