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 <ladsgr...@gmail.com> Gerrit-Reviewer: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de> Gerrit-Reviewer: Ladsgroup <ladsgr...@gmail.com> Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de> Gerrit-Reviewer: WMDE-leszek <leszek.mani...@wikimedia.de> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits