Jhernandez has uploaded a new change for review.
https://gerrit.wikimedia.org/r/195970
Change subject: Make JSON importing more robust to avoid runtime errors
......................................................................
Make JSON importing more robust to avoid runtime errors
Validate json structures before shoving them into an object (because if there
is type mismatches there will be breakage).
If JSONs aren't valid then they will be ignored being effectively invisible in
the site.
Bug: T92347
Change-Id: I50443c8e9806e93e838298cc7f80c04aa76335cc
---
M includes/stores/UserPageCollection.php
M includes/stores/UserPageCollectionsList.php
2 files changed, 60 insertions(+), 37 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Gather
refs/changes/70/195970/1
diff --git a/includes/stores/UserPageCollection.php
b/includes/stores/UserPageCollection.php
index b741382..98498f8 100644
--- a/includes/stores/UserPageCollection.php
+++ b/includes/stores/UserPageCollection.php
@@ -23,11 +23,7 @@
public static function newFromUserAndId( $owner, $id ) {
if ( $id !== 0 ) {
$collectionData = JSONPage::get( self::getStorageTitle(
$owner, $id ) );
- if ( isset( $collectionData['id'] ) ) {
- return self::collectionFromJSON(
$collectionData );
- } else {
- return null;
- }
+ return self::collectionFromJSON( $collectionData );
} else {
// id 0 is the watchlist. Which loads differently
return WatchlistCollection::newFromUser( $owner );
@@ -48,28 +44,42 @@
/**
* Fill a collection object from json data
+ * Returns null if there is not enough information to fill it up.
* @param array $json data to pull information from
*
- * @return models\Collection
+ * @return models\Collection|null
*/
public static function collectionFromJSON( $json ) {
- $collection = new models\Collection(
- $json['id'],
- User::newFromName( $json['owner'] ),
- $json['title'],
- $json['description'],
- $json['public'],
- wfFindFile( $json['image'] )
- );
- if ( isset( $json['items'] ) ) {
- // Make titles
- $titles = array();
- foreach ( $json['items'] as $title ) {
- $titles[] = Title::newFromText( $title );
+ try {
+ if ( !isset($json['id']) ||
+ !isset($json['owner']) ||
+ !isset($json['title']) ) {
+ return null;
}
- $collection->batch( self::getItemsFromTitles( $titles )
);
+
+ $collection = new models\Collection(
+ $json['id'],
+ User::newFromName( $json['owner'] ),
+ $json['title'],
+ $json['description'],
+ $json['public'],
+ wfFindFile( $json['image'] )
+ );
+ if ( isset( $json['items'] ) ) {
+ // Make titles
+ $titles = array();
+ foreach ( $json['items'] as $title ) {
+ if ( is_string( $title ) && isset(
$title ) ) {
+ $titles[] = Title::newFromText(
$title );
+ }
+ }
+ $collection->batch( self::getItemsFromTitles(
$titles ) );
+ }
+ return $collection;
+ } catch (Exception $e) {
+ // Invalid json
+ return null;
}
- return $collection;
}
}
diff --git a/includes/stores/UserPageCollectionsList.php
b/includes/stores/UserPageCollectionsList.php
index fecf8ac..3dd3ea9 100644
--- a/includes/stores/UserPageCollectionsList.php
+++ b/includes/stores/UserPageCollectionsList.php
@@ -29,11 +29,12 @@
$collectionsData = JSONPage::get( self::getStorageTitle( $user
) );
foreach ( $collectionsData as $collectionData ) {
$collection = self::collectionFromJSON( $collectionData
);
- $collectionsList->add( $collection );
-
- // If the added collection is a watchlist make a record
of it
- if ( $collection->getId() === 0 ) {
- $includesWatchlist = true;
+ if ( $collection ) {
+ $collectionsList->add( $collection );
+ // If the added collection is a watchlist make
a record of it
+ if ( $collection->getId() === 0 ) {
+ $includesWatchlist = true;
+ }
}
}
@@ -67,21 +68,33 @@
/**
* Get a basic collection object with the metadata from json data in
the manifest
+ * Returns null if there is not enough info to create the object.
* @param array $json data to pull information from
*
- * @return models\CollectionInfo
+ * @return models\CollectionInfo|null
*/
public static function collectionFromJSON( $json ) {
- $collection = new models\CollectionInfo(
- $json['id'],
- User::newFromName( $json['owner'] ),
- $json['title'],
- $json['description'],
- $json['public'],
- wfFindFile( $json['image'] )
- );
- $collection->setCount( $json['count'] );
- return $collection;
+ try {
+ if ( !isset( $json['id'] ) ||
+ !isset( $json['owner'] ) ||
+ !isset( $json['title'] ) ) {
+ return null;
+ }
+
+ $collection = new models\CollectionInfo(
+ $json['id'],
+ User::newFromName( $json['owner'] ),
+ $json['title'],
+ $json['description'],
+ $json['public'],
+ wfFindFile( $json['image'] )
+ );
+ $collection->setCount( $json['count'] );
+ return $collection;
+ } catch (Exception $e) {
+ // Invalid json
+ return null;
+ }
}
}
--
To view, visit https://gerrit.wikimedia.org/r/195970
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I50443c8e9806e93e838298cc7f80c04aa76335cc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Gather
Gerrit-Branch: master
Gerrit-Owner: Jhernandez <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
