Jdlrobson has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/373292 )
Change subject: Verify the existence of `url` key when parsing lang objects
......................................................................
Verify the existence of `url` key when parsing lang objects
Some langLinks retrieved from API call do not contain url property, only
code and title. If that happens do not throw PHP notice error. Log
the error and skip broken lang object.
Changes:
- extracted langObject validation to a separate function
- added url property existence check
- renamed $code to $index as $langLinks is an array without keys
- introduced MobileContext::LOGGER_CHANNEL as it's used in couple
places
Bug: T172316
Change-Id: I4ef5b1ad4a37b96407f7f758680a6177cafdf128
(cherry picked from commit 456264d8c2b67bfe665bc153af6143e8597cd3b7)
---
M includes/MobileContext.php
M includes/specials/SpecialMobileLanguages.php
M tests/phpunit/specials/SpecialMobileLanguagesTest.php
3 files changed, 67 insertions(+), 6 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/92/373292/1
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 330a39a..cd7be3d 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -21,7 +21,7 @@
const LAZY_LOAD_IMAGES_COOKIE_VALUE = 'A';
const LAZY_LOAD_REFERENCES_COOKIE_NAME = 'mfLazyLoadReferences';
const LAZY_LOAD_REFERENCES_COOKIE_VALUE = 'A';
-
+ const LOGGER_CHANNEL = 'mobile';
/**
* Saves the testing mode user has opted in: 'beta' or 'stable'
* @var string $mobileMode
diff --git a/includes/specials/SpecialMobileLanguages.php
b/includes/specials/SpecialMobileLanguages.php
index 91e9783..735645e 100644
--- a/includes/specials/SpecialMobileLanguages.php
+++ b/includes/specials/SpecialMobileLanguages.php
@@ -58,15 +58,14 @@
// Set the name of each language based on the system
list of language names
$languageMap = Language::fetchLanguageNames();
$languages = $page['langlinks'];
- foreach ( $page['langlinks'] as $code => $langObject ) {
- if ( !isset( $languageMap[$langObject['lang']]
) ) {
- // Bug T93500: DB might still have
preantiquated rows with bogus languages
- unset( $languages[$code] );
+ foreach ( $page['langlinks'] as $index => $langObject )
{
+ if ( !$this->isLanguageObjectValid(
$languageMap, $langObject ) ) {
+ unset( $languages[$index] );
continue;
}
$langObject['langname'] =
$languageMap[$langObject['lang']];
$langObject['url'] =
MobileContext::singleton()->getMobileUrl( $langObject['url'] );
- $languages[$code] = $langObject;
+ $languages[$index] = $langObject;
}
$compareLanguage = function ( $a, $b ) {
return strcasecmp( $a['langname'],
$b['langname'] );
@@ -80,6 +79,33 @@
}
/**
+ * Verify if passed language object contains all necessary information
+ *
+ * @see https://phabricator.wikimedia.org/T93500
+ * @see https://phabricator.wikimedia.org/T172316
+ * @param array $languageMap array of language names, indexed by code.
+ * @param array $langObject array of lang objects
+ * @return bool
+ */
+ private function isLanguageObjectValid( $languageMap, $langObject ) {
+ if ( !isset( $languageMap[$langObject['lang']] ) ) {
+ // Bug T93500: DB might still have preantiquated rows
with bogus languages
+ return false;
+ }
+ if ( !array_key_exists( 'url', $langObject ) ) {
+ // Bug T172316: Some lang objects do not have url. We
would like to log those instances
+ \MediaWiki\Logger\LoggerFactory::getInstance(
MobileContext::LOGGER_CHANNEL )->warning(
+ '`url` key is undefined in language object',
+ [
+ 'uri' =>
RequestContext::getMain()->getRequest()->getFullRequestURL(),
+ 'langObject' => $langObject,
+ ]
+ );
+ return false;
+ }
+ return true;
+ }
+ /**
* Returns an array of language variants that the page is available in
* @return array
*/
diff --git a/tests/phpunit/specials/SpecialMobileLanguagesTest.php
b/tests/phpunit/specials/SpecialMobileLanguagesTest.php
index 385bb48..1f8d27c 100644
--- a/tests/phpunit/specials/SpecialMobileLanguagesTest.php
+++ b/tests/phpunit/specials/SpecialMobileLanguagesTest.php
@@ -115,6 +115,7 @@
/**
* @dataProvider providerProcessLanguages
* @covers SpecialMobileLanguages::processLanguages
+ * @covers SpecialMobileLanguages::isLanguageObjectValid
*/
public function testProcessLanguages( $langlinks, $expected ) {
$apiResult = [
@@ -132,4 +133,38 @@
'Property langname should be added, URL should be
transformed, and languages should be sorted.'
);
}
+
+ /**
+ * Test an edge case when URL key is missing in langObject
+ *
+ * @covers SpecialMobileLanguages::processLanguages
+ * @covers SpecialMobileLanguages::isLanguageObjectValid
+ */
+ public function testProcessLanguagesWhenURLKeyIsMissing() {
+ $testUri = 'http://localhost/test';
+ $langObject = [
+ 'lang' => 'pl',
+ 'title' => 'Polski'
+ ];
+ $expected = [];
+
+ $loggerMock = $this->getMock( \Psr\Log\LoggerInterface::class );
+ $loggerMock->expects( $this->once() )
+ ->method( 'warning' )
+ ->with( $this->isType( 'string' ), $this->equalTo(
+ [
+ 'uri' => $testUri,
+ 'langObject' => $langObject
+ ]
+ ) );
+ $this->setLogger( MobileContext::LOGGER_CHANNEL, $loggerMock );
+
+ $requestMock = $this->getMock( FauxRequest::class );
+ $requestMock->expects( $this->once() )
+ ->method( 'getFullRequestURL' )
+ ->willReturn( $testUri );
+
+ RequestContext::getMain()->setRequest( $requestMock );
+ $this->testProcessLanguages( [ $langObject ], $expected );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/373292
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ef5b1ad4a37b96407f7f758680a6177cafdf128
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: wmf/1.30.0-wmf.14
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Pmiazga <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
