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

Reply via email to