jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/334046 )
Change subject: Leave data validation in EditEntity to ChangeOp deserializers
......................................................................
Leave data validation in EditEntity to ChangeOp deserializers
EditEntity should not be whitelisting fields in "data" parameter
as fields may vary depending on what entity types are enabled.
This change makes EditEntity contain only single assumption:
"remove" key should not be part of top-level structure,
ie. "remove" key should always be part of the object related
to the specific field.
Change-Id: I9654640b2fdd47cb02afbbcae5bb17d447723555
---
M repo/includes/Api/EditEntity.php
M repo/tests/phpunit/includes/Api/EditEntityTest.php
2 files changed, 31 insertions(+), 31 deletions(-)
Approvals:
Ladsgroup: Looks good to me, approved
jenkins-bot: Verified
Thiemo Mättig (WMDE): Looks good to me, but someone else must approve
diff --git a/repo/includes/Api/EditEntity.php b/repo/includes/Api/EditEntity.php
index 5da710d..730955d 100644
--- a/repo/includes/Api/EditEntity.php
+++ b/repo/includes/Api/EditEntity.php
@@ -629,29 +629,7 @@
$entityId = $entity->getId();
$title = $entityId === null ? null :
$this->getTitleLookup()->getTitleForId( $entityId );
- $allowedProps = array(
- // ignored props
- 'length',
- 'count',
- 'touched',
- 'modified',
- // checked props
- 'id',
- 'type',
- 'pageid',
- 'ns',
- 'title',
- 'lastrevid',
- // useful props
- 'labels',
- 'descriptions',
- 'aliases',
- 'sitelinks',
- 'claims',
- 'datatype'
- );
-
- $this->checkValidJson( $data, $allowedProps );
+ $this->checkValidJson( $data );
$this->checkEntityId( $data, $entityId );
$this->checkEntityType( $data, $entity );
$this->checkPageIdProp( $data, $title );
@@ -662,9 +640,8 @@
/**
* @param mixed $data
- * @param array $allowedProps
*/
- private function checkValidJson( $data, array $allowedProps ) {
+ private function checkValidJson( $data ) {
if ( is_null( $data ) ) {
$this->errorReporter->dieError( 'Invalid json: The
supplied JSON structure could not be parsed or '
. 'recreated as a valid structure',
'invalid-json' );
@@ -677,8 +654,11 @@
// Catch json_decode returning an indexed array (list).
$this->assertString( $prop, 'Top level structure must
be a JSON object (no keys found)' );
- if ( !in_array( $prop, $allowedProps ) ) {
- $this->errorReporter->dieError( "Unknown key in
json: $prop", 'not-recognized' );
+ if ( $prop === 'remove' ) {
+ $this->errorReporter->dieError(
+ '"remove" should not be a top-level
key',
+ 'not-recognized'
+ );
}
}
}
diff --git a/repo/tests/phpunit/includes/Api/EditEntityTest.php
b/repo/tests/phpunit/includes/Api/EditEntityTest.php
index 591ff55..9b6e096 100644
--- a/repo/tests/phpunit/includes/Api/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/Api/EditEntityTest.php
@@ -706,8 +706,8 @@
'site' => 'enwiki',
'title' => 'Berlin',
'data' => '{
- "remove": "",
"claims": [ {
+ "remove": "",
"mainsnak": {
"snaktype":
"value",
"property":
"%P56%",
@@ -720,9 +720,8 @@
),
'e' => array( 'exception' => array(
'type' => ApiUsageException::class,
- 'code' => 'not-recognized',
- 'message' => 'Unknown key in json:
remove' )
- )
+ 'code' => 'invalid-claim',
+ ) )
),
'bad badge id' => array(
'p' => array(
@@ -875,6 +874,27 @@
) ),
'requires' => 'mediainfo' // skip if MediaInfo
is not configured
),
+ 'remove key misplaced in data' => [
+ 'p' => [
+ 'id' => '%Berlin%',
+ 'data' => json_encode( [
+ 'remove' => '',
+ 'claims' => [ [
+ 'type' => 'statement',
+ 'mainsnak' => [
+ 'snaktype' =>
'novalue',
+ 'property' =>
'%P56%',
+ ],
+ 'id' => '%BerlinP56%',
+ ] ],
+ ] )
+ ],
+ 'e' => [ 'exception' => [
+ 'type' => ApiUsageException::class,
+ 'code' => 'not-recognized',
+ 'message' => '"remove" should not be a
top-level key'
+ ] ],
+ ],
);
}
--
To view, visit https://gerrit.wikimedia.org/r/334046
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9654640b2fdd47cb02afbbcae5bb17d447723555
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Ladsgroup <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
