Split $objectDefinition into $schema and $properties

Object validation attempts to use a single variable to store both the object definition, and its properties. This causes validation to be incomplete where "properties" is not set, but "additionalProperties" is. This commit fixes both bugs in issue #353.
jsonrainbow · Apr 11, 2017 · 7fc4bf4 · 7fc4bf4
1 parent 2553ebd
commit 7fc4bf4
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 24 deletions.
diff --git a/src/JsonSchema/Constraints/Constraint.php b/src/JsonSchema/Constraints/Constraint.php
@@ -78,13 +78,15 @@ protected function checkArray(&$value, $schema = null, JsonPointer $path = null,
      * @param mixed            $value
      * @param mixed            $schema
      * @param JsonPointer|null $path
-     * @param mixed            $i
+     * @param mixed            $properties
+     * @param mixed            $additionalProperties
      * @param mixed            $patternProperties
      */
-    protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $i = null, $patternProperties = null, $appliedDefaults = array())
+    protected function checkObject(&$value, $schema = null, JsonPointer $path = null, $properties = null,
+        $additionalProperties = null, $patternProperties = null, $appliedDefaults = array())
     {
         $validator = $this->factory->createInstanceFor('object');
-        $validator->check($value, $schema, $path, $i, $patternProperties, $appliedDefaults);
+        $validator->check($value, $schema, $path, $properties, $additionalProperties, $patternProperties, $appliedDefaults);
 
         $this->addErrors($validator->getErrors());
     }

diff --git a/src/JsonSchema/Constraints/ObjectConstraint.php b/src/JsonSchema/Constraints/ObjectConstraint.php
@@ -28,7 +28,8 @@ class ObjectConstraint extends Constraint
     /**
      * {@inheritdoc}
      */
-    public function check(&$element, $definition = null, JsonPointer $path = null, $additionalProp = null, $patternProperties = null, $appliedDefaults = array())
+    public function check(&$element, $schema = null, JsonPointer $path = null, $properties = null,
+        $additionalProp = null, $patternProperties = null, $appliedDefaults = array())
     {
         if ($element instanceof UndefinedConstraint) {
             return;
@@ -38,16 +39,17 @@ public function check(&$element, $definition = null, JsonPointer $path = null, $
 
         $matches = array();
         if ($patternProperties) {
+            // validate the element pattern properties
             $matches = $this->validatePatternProperties($element, $path, $patternProperties);
         }
 
-        if ($definition) {
-            // validate the definition properties
-            $this->validateDefinition($element, $definition, $path);
+        if ($properties) {
+            // validate the element properties
+            $this->validateProperties($element, $properties, $path);
         }
 
-        // additional the element properties
-        $this->validateElement($element, $matches, $definition, $path, $additionalProp);
+        // validate additional element properties & constraints
+        $this->validateElement($element, $matches, $schema, $path, $properties, $additionalProp);
     }
 
     public function validatePatternProperties($element, JsonPointer $path = null, $patternProperties)
@@ -82,18 +84,20 @@ public function validatePatternProperties($element, JsonPointer $path = null, $p
     /**
      * Validates the element properties
      *
-     * @param \stdClass        $element          Element to validate
-     * @param array            $matches          Matches from patternProperties (if any)
-     * @param \stdClass        $objectDefinition ObjectConstraint definition
-     * @param JsonPointer|null $path             Path to test?
-     * @param mixed            $additionalProp   Additional properties
+     * @param \StdClass        $element        Element to validate
+     * @param array            $matches        Matches from patternProperties (if any)
+     * @param \StdClass        $schema         ObjectConstraint definition
+     * @param JsonPointer|null $path           Current test path
+     * @param \StdClass        $properties     Properties
+     * @param mixed            $additionalProp Additional properties
      */
-    public function validateElement($element, $matches, $objectDefinition = null, JsonPointer $path = null, $additionalProp = null)
+    public function validateElement($element, $matches, $schema = null, JsonPointer $path = null,
+        $properties = null, $additionalProp = null)
     {
-        $this->validateMinMaxConstraint($element, $objectDefinition, $path);
+        $this->validateMinMaxConstraint($element, $schema, $path);
 
         foreach ($element as $i => $value) {
-            $definition = $this->getProperty($objectDefinition, $i);
+            $definition = $this->getProperty($properties, $i);
 
             // no additional properties allowed
             if (!in_array($i, $matches) && $additionalProp === false && $this->inlineSchemaProperty !== $i && !$definition) {
@@ -128,17 +132,17 @@ public function validateElement($element, $matches, $objectDefinition = null, Js
     /**
      * Validates the definition properties
      *
-     * @param \stdClass        $element          Element to validate
-     * @param \stdClass        $objectDefinition ObjectConstraint definition
-     * @param JsonPointer|null $path             Path?
+     * @param \stdClass        $element    Element to validate
+     * @param \stdClass        $properties Property definitions
+     * @param JsonPointer|null $path       Path?
      */
-    public function validateDefinition(&$element, $objectDefinition = null, JsonPointer $path = null)
+    public function validateProperties(&$element, $properties = null, JsonPointer $path = null)
     {
         $undefinedConstraint = $this->factory->createInstanceFor('undefined');
 
-        foreach ($objectDefinition as $i => $value) {
+        foreach ($properties as $i => $value) {
             $property = &$this->getProperty($element, $i, $undefinedConstraint);
-            $definition = $this->getProperty($objectDefinition, $i);
+            $definition = $this->getProperty($properties, $i);
 
             if (is_object($definition)) {
                 // Undefined constraint will check for is_object() and quit if is not - so why pass it?

diff --git a/src/JsonSchema/Constraints/UndefinedConstraint.php b/src/JsonSchema/Constraints/UndefinedConstraint.php
@@ -73,8 +73,9 @@ public function validateTypes(&$value, $schema = null, JsonPointer $path, $i = n
                                                 // is not set (i.e. don't use $this->getTypeCheck() here).
             $this->checkObject(
                 $value,
-                isset($schema->properties) ? $this->factory->getSchemaStorage()->resolveRefSchema($schema->properties) : $schema,
+                $schema,
                 $path,
+                isset($schema->properties) ? $schema->properties : null,
                 isset($schema->additionalProperties) ? $schema->additionalProperties : null,
                 isset($schema->patternProperties) ? $schema->patternProperties : null,
                 $this->appliedDefaults