Yurik has uploaded a new change for review.
https://gerrit.wikimedia.org/r/65970
Change subject: Logic revision, config "wikipedia" setting
......................................................................
Logic revision, config "wikipedia" setting
* Added more cases for suspicious logging
* More consistent "site" config param uses "wikipedia" instead of "wiki"
* Began consolidating isZero*() methods, pending logic check
Change-Id: Ifb9ea8e4912d97e558c3e4ff255ff6a0fbb9d787
---
M includes/CarrierConfig.php
M includes/PageRenderingHooks.php
2 files changed, 122 insertions(+), 81 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ZeroRatedMobileAccess
refs/changes/70/65970/1
diff --git a/includes/CarrierConfig.php b/includes/CarrierConfig.php
index 14a5de1..24f2206 100644
--- a/includes/CarrierConfig.php
+++ b/includes/CarrierConfig.php
@@ -135,9 +135,9 @@
$v = $value;
if ( is_string( $v ) ) {
$oldValues = array(
- 'zero' => 'zero.wiki',
- 'm' => 'm.wiki',
- 'both' => array( 'm.wiki',
'zero.wiki' ) );
+ 'zero' => 'zero.wikipedia',
+ 'm' => 'm.wikipedia',
+ 'both' => array( 'm.wikipedia',
'zero.wikipedia' ) );
$v = strtolower( $v );
if ( array_key_exists( $v, $oldValues )
) {
$v = $oldValues[$v];
@@ -147,12 +147,20 @@
}
}
$validValues = array(
- 'zero.wiki',
- 'm.wiki',
+ 'zero.wikipedia',
+ 'm.wikipedia',
);
if ( is_array( $v ) ) {
$v = array_map( 'strtolower', $v );
- if ( count( array_intersect( $v,
$validValues ) ) !== count($v) ) {
+ // FIXME: remove this after refreshing
all meta configs
+ foreach ( $v as &$item ) {
+ if ( $item === 'zero.wiki' ) {
+ $item =
'zero.wikipedia';
+ } elseif ( $item === 'm.wiki' )
{
+ $item = 'm.wikipedia';
+ }
+ }
+ if ( count( array_intersect( $v,
$validValues ) ) !== count( $v ) ) {
$v = false;
} else {
$v = array_unique( $v );
diff --git a/includes/PageRenderingHooks.php b/includes/PageRenderingHooks.php
index b391239..ab4b372 100644
--- a/includes/PageRenderingHooks.php
+++ b/includes/PageRenderingHooks.php
@@ -35,7 +35,7 @@
*/
public static function onGetMobileUrl( &$subdomainTokenReplacement ) {
// TODO: logic check
- if ( self::isZeroRequest() && self::disableImages() ) {
+ if ( self::isZeroSubdomain() && self::disableImages() ) {
$subdomainTokenReplacement = 'zero.';
}
return true;
@@ -81,7 +81,6 @@
}
return true;
-
}
/**
@@ -94,30 +93,35 @@
public static function onBeforePageDisplay( &$out, &$options ) {
global $wgRequest;
- $config = self::getConfig();
- if ( $config === null ) {
+ if ( !self::isZeroSite() ) {
return true;
}
wfProfileIn( __METHOD__ );
+ $config = self::getConfig();
+
+ // @FIXME: Debug checks - should be remove once all issues are
gone
+ if ( $config === null && (
+ $wgRequest->getCheck( 'renderZeroRatedBanner' ) ||
+ $wgRequest->getCheck( 'acceptbilling' ) ||
+ $wgRequest->getCheck( 'renderwarning' ) ||
+ $wgRequest->getCheck( 'renderZeroRatedRedirect' )
+ ) ) {
+ self::logDebug( "!config && zero params in query" );
+ }
+
+ // @FIXME: Should not be added for non-zero sites
$out->addModuleStyles( 'mobile.zero.styles' );
$out->addModules( 'mobile.zero.scripts' );
$isFilePage = $out->getTitle()->inNamespace( NS_FILE );
$showBanner = $config !== null || $wgRequest->getFuzzyBool(
'renderZeroRatedBanner' );
- if ( !$showBanner && self::isZeroRequest() ) {
+ if ( !$showBanner && self::isZeroSubdomain() ) {
$out->clearHTML();
$out->setPageTitle( null );
$out->addHTML( self::renderUnknownCarrier() );
wfProfileOut( __METHOD__ );
return true;
- }
-
- if ( $showBanner && $config === null ) {
- // This should never happen - attempt to debug it
- $dbg = "showBanner && !config\t" . $wgRequest->getIP()
. "\t" . $wgRequest->getFullRequestURL();
- $dbg .= "\nHeaders: " . print_r(
$wgRequest->getAllHeaders(), true );
- wfDebugLog( 'zero', $dbg );
}
$acceptBilling = $wgRequest->getVal( 'acceptbilling' );
@@ -146,7 +150,7 @@
}
if ( $showBanner && $showWarning && $acceptBilling !== 'yes' ) {
- $domainReplacement = self::isZeroRequest() ?
self::$zerodotParent : self::$mdotParent;
+ $domainReplacement = self::isZeroSubdomain() ?
self::$zerodotParent : self::$mdotParent;
$output = self::renderQuestion(
str_replace(
self::$originParent,
@@ -199,8 +203,8 @@
*/
private static function renderUnknownCarrier() {
global $wgRequest;
- $ip = $wgRequest->getVal( 'ip', $wgRequest->getIP() );
- // @todo FIXME: Unescaped UI text output as HTML in next 3
lines.
+ $ip = $wgRequest->getIP();
+ // @FIXME: Unescaped UI text output as HTML in next 3 lines.
$bannerText = wfMessage( 'zero-rated-mobile-access-sorry'
)->text();
$bannerText .= '<br />' . wfMessage(
'zero-rated-mobile-access-sorry-ip', $ip )->text();
$bannerText .= '<br />' . wfMessage(
@@ -322,7 +326,7 @@
if ( $config ) {
$overrides = $config['langNameOverrides'];
- // Allow language name overrides, as requested by
Orange Congo ('kg' => 'Kikongo)
+ // Allow language name overrides, as requested by
Orange Congo ('kg' => 'Kikongo')
// Do it one at a time to keep the original order of
the array
foreach ( $overrides as $l => $n ) {
$languageNames[$l] = $n;
@@ -432,24 +436,19 @@
ResourceLoader &$resourceLoader
) {
$testModules['qunit']['ext.zeroratedmobileaccess.tests'] =
array(
- 'dependencies' => array(
- 'mobile.zero.scripts'
- ),
+ 'dependencies' => array( 'mobile.zero.scripts' ),
'targets' => array( 'mobile' ),
'localBasePath' => dirname( dirname( __FILE__ ) ),
'remoteExtPath' => 'ZeroRatedMobileAccess',
- 'scripts' => array(
- 'tests/js/test_banner.js',
- ),
+ 'scripts' => array( 'tests/js/test_banner.js' ),
);
return true;
}
-
/**
- * Returns cached carrier configuration based on X-CS header or query
parameter
- * @return array|null Carrier configuration or null if it doesn't exist
or is disabled
- */
+ * Returns cached carrier configuration based on X-CS header or query
parameter
+ * @return array|null Carrier configuration or null if it doesn't exist
or is disabled
+ */
private static function getConfig() {
// Cached configuration array or null if missing or disabled.
@@ -459,37 +458,25 @@
}
wfProfileIn( __METHOD__ );
- global $wgRequest, $wgConf, $wgDBname,
$wgEnableZeroRatedMobileAccessTesting;
-
+ global $wgRequest;
$config = null;
- list( $site, $langCode ) = $wgConf->siteFromDB( $wgDBname );
-
- // Determine if this is a Zero-related site shown with a mobile
device
- $isZero = ( $site === 'wikipedia' ||
$wgEnableZeroRatedMobileAccessTesting );
- if ( $isZero ) {
- $useFormat = $wgRequest->getText( 'useformat' );
- $isZero = !empty( $_SERVER['HTTP_X_DEVICE'] ) ||
$useFormat === 'mobile' || $useFormat === 'mobile-wap';
+ // Allow URL override of the X-CS parameter for testing purposes
+ $id = $wgRequest->getVal( 'X-CS' );
+ if ( $id === null ) {
+ $id = $wgRequest->getHeader( 'X-CS' );
}
- if ( $isZero ) {
- // Allow URL override of the X-CS parameter for testing
purposes
- $id = $wgRequest->getVal( 'X-CS' );
- if ( $id === null ) {
- $id = $wgRequest->getHeader( 'X-CS' );
- }
- if ( $id === '(null)' || !$id ) {
- $id = null;
- }
- if ( $id !== null ) {
- $store = new CarrierConfigStore( $id );
- $text = $store->get();
- if ( $text !== false ) {
- $conf = new CarrierConfig( $text );
- if ( !$conf->isError() ) {
- $config = $conf->getData();
- $subdomain =
$wgRequest->getHeader( 'X-SUBDOMAIN' );
- if ( !self::isApplicable(
$config, $subdomain, $langCode ) ) {
- $config = null;
- }
+ if ( $id === '(null)' || !$id ) {
+ $id = null;
+ }
+ if ( $id !== null ) {
+ $store = new CarrierConfigStore( $id );
+ $text = $store->get();
+ if ( $text !== false ) {
+ $conf = new CarrierConfig( $text );
+ if ( !$conf->isError() ) {
+ $config = $conf->getData();
+ if ( !self::isZeroEnabled( $config ) ) {
+ $config = null;
}
}
}
@@ -498,20 +485,30 @@
return $config;
}
-
- static function isApplicable( $config, $subdomain, $langCode ) {
+ /**
+ * As part of self::getConfig(), determine if zero should be enabled
+ * for this site based on configuration
+ * @param array $config
+ * @return bool
+ */
+ private static function isZeroEnabled( $config ) {
if ( !$config['enabled'] ) {
// If the configuration is disabled, pretend like it
doesn't exist
return false;
}
// Check if the wiki is in the allowed list
- // @TODO: expand to allow non-wikipedia sites
- $site = strtolower( $subdomain ) . '.wiki';
+ // @FIXME: expand to allow non-wikipedia sites
+ global $wgRequest;
+ $subdomain = $wgRequest->getHeader( 'X-SUBDOMAIN' );
+ $site = strtolower( $subdomain ) . '.wikipedia';
if ( !in_array( $site, $config['sites'] ) ) {
return false;
}
$freeLangs = $config['whitelistedLangs'];
if ( 0 !== count( $freeLangs ) ) {
+ global $wgConf, $wgDBname;
+ /** @noinspection PhpUnusedLocalVariableInspection */
+ list( $site, $langCode ) = $wgConf->siteFromDB(
$wgDBname );
if ( $langCode === '' || $langCode === 'test' ) {
$langCode = 'en'; // Useful for debugging,
should never be the case in production
}
@@ -520,6 +517,41 @@
}
}
return true;
+ }
+
+ /**
+ * Returns true if X-SUBDOMAIN header is set to 'ZERO'
+ * @return bool
+ */
+ private static function isZeroSubdomain() {
+ static $isZero = null;
+ if ( $isZero === null ) {
+ global $wgRequest;
+ $isZero = $wgRequest->getHeader( 'X-SUBDOMAIN' ) ===
'ZERO';
+ }
+ return $isZero;
+ }
+
+ /**
+ * Determine if this could be a Zero-related site shown with a mobile
device
+ * Must be wikipedia, with either the X-Device header or have a mobile
useformat
+ * @return bool
+ */
+ private static function isZeroSite() {
+ static $isZero = null;
+ if ( $isZero === null ) {
+ global $wgConf, $wgDBname, $wgRequest,
$wgEnableZeroRatedMobileAccessTesting;
+ /** @noinspection PhpUnusedLocalVariableInspection */
+ list( $site, $langCode ) = $wgConf->siteFromDB(
$wgDBname );
+ // @FIXME: expand to allow non-wikipedia sites
+ $isZero = ( $site === 'wikipedia' ||
$wgEnableZeroRatedMobileAccessTesting );
+ // To be zero must have either X-Device or useformat
+ if ( $isZero && $wgRequest->getHeader( 'X-DEVICE' ) ===
false ) {
+ $useFormat = $wgRequest->getText( 'useformat' );
+ $isZero = $useFormat === 'mobile' || $useFormat
=== 'mobile-wap';
+ }
+ }
+ return $isZero;
}
/**
@@ -632,25 +664,12 @@
/**
- * Returns true if HTTP_X_SUBDOMAIN header is set to 'ZERO'
- * @return bool
- */
- private static function isZeroRequest() {
- static $isZero = null;
- if ( $isZero === null ) {
- $isZero = isset( $_SERVER['HTTP_X_SUBDOMAIN'] ) &&
$_SERVER['HTTP_X_SUBDOMAIN'] === 'ZERO';
- }
- return $isZero;
- }
-
-
- /**
* This is a placeholder function to replace $wgZeroDisableImages which
seems to always be 1 for Zero
* @return bool
*/
private static function disableImages() {
- // TODO? FIXME? Probably need to check the config as well
- return self::isZeroRequest();
+ // @FIXME: Probably need to check the config as well
+ return self::isZeroSubdomain();
}
/**
@@ -672,9 +691,23 @@
return $link;
}
+ /**
+ * Output debug info into Zero log
+ * @param string $dbg Debug message
+ */
+ private static function logDebug( $dbg ) {
+ static $printedHeaders = false;
+ global $wgRequest;
+ $dbg .= "\t" . $wgRequest->getIP() . "\t" .
$wgRequest->getFullRequestURL();
+ if ( !$printedHeaders ) {
+ $dbg .= "\nHeaders: " . print_r(
$wgRequest->getAllHeaders(), true );
+ $printedHeaders = true;
+ }
+ wfDebugLog( 'zero', $dbg );
+ }
public function getVersion() {
- // TODO? Delete this?
+ // @FIXME: Delete this?
return __CLASS__ . ': $Id$';
}
}
--
To view, visit https://gerrit.wikimedia.org/r/65970
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb9ea8e4912d97e558c3e4ff255ff6a0fbb9d787
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ZeroRatedMobileAccess
Gerrit-Branch: master
Gerrit-Owner: Yurik <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
