Skip to content

oneOf never validates when one item is array type the other is scalar or null when checkMode = Constraint::CHECK_MODE_COERCE_TYPES #770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jozefbriss opened this issue Dec 6, 2024 · 6 comments

Comments

@jozefbriss
Copy link

It seems that coercing scalar to array causes than oneOf never validates when one item is array type and the other is scalar or null.

The reason is that any scalar or null can be coerced to array.

Maybe there should not be toArray coercion.

$jsonSchema = <<<'JSON'
{
    "type": "object",
    "properties": {
        "data": {
            "oneOf": [{ "type": "string" }, { "type": "array" }]
        }
    },
    "required": ["data"]
}
JSON;

$jsonSchemaObject = json_decode($jsonSchema);

$schemaStorage = new SchemaStorage();
$schemaStorage->addSchema('file://mySchema', $jsonSchemaObject);

$jsonValidator = new Validator(new Factory($schemaStorage));

$jsonToValidateObject = json_decode('{"data":"ABC"}');

$jsonValidator->validate($jsonToValidateObject, $jsonSchemaObject, Constraint::CHECK_MODE_COERCE_TYPES);

print_r($jsonValidator->getErrors());

results in:

Array
(
    [0] => Array
        (
            [property] => data
            [pointer] => /data
            [message] => Failed to match exactly one schema
            [constraint] => Array
                (
                    [name] => oneOf
                    [params] => Array
                        (
                        )

                )

            [context] => 1
        )

)

version: 6.0.0

Maybe the issues is more generic than only related to array and "oneOf" and Constraint::CHECK_MODE_COERCE_TYPES may not work together.

@DannyvdSluijs
Copy link
Collaborator

Thanks for the report I'll try and find time to take a look soon. Do you perhaps also know if this occurs with earlier versions as well?

@jozefbriss
Copy link
Author

I have migrated from "5.2.13" to "6.0.0".

I've been using 5.2.13 with custom TypeConstraint class:

$factory->setConstraintClass('type', TypeConstraint::class);

Mime TypeConstraint was almost the same as there is now in the project except mine version did not have array coercion.

@madman-81
Copy link

madman-81 commented Dec 10, 2024

I think I'm running into this same issue. The following unit test passes with version 5.3 but fails with version 6.0:

<?php

namespace Tests\Unit\Validation;

use JsonSchema\Constraints\Constraint;
use JsonSchema\Validator;
use PHPUnit\Framework\TestCase;

class JsonValidateV6Test extends TestCase
{
    public function testJsonCompare()
    {
        $schema = <<<JSON
            {
                "title": "Location",
                "type": "object",
                "properties": {
                    "id": {
                        "type": "string"
                    },
                    "related_locations": {
                        "oneOf": [
                            {
                                "type": "null"
                            },
                            {
                                "type": "array",
                                "items": {
                                    "type": "object",
                                    "properties": {
                                        "latitude": {
                                            "type": "string"
                                        },
                                        "longitude": {
                                            "type": "string"
                                        }
                                    }
                                }
                            }
                        ]
                    }
                }
            }
        JSON;

        $json = <<<JSON
            {
                "id": "LOC1",
                "related_locations": [
                    {
                        "latitude": "51.047598",
                        "longitude": "3.729943"
                    }
                ]
            }
        JSON;

        $jsonObj = json_decode($json);
        $schemaObj = json_decode($schema);

        $jsonSchemaValidation = new Validator();
        $jsonSchemaValidation->validate($jsonObj, $schemaObj, Constraint::CHECK_MODE_COERCE_TYPES);

        $this->assertTrue($jsonSchemaValidation->isValid());
    }
}

@DannyvdSluijs
Copy link
Collaborator

DannyvdSluijs commented Feb 25, 2025

@jozefbriss I've been looking into this report and quickly realised that your schema combined with the type coercion is working against each other.

Simple put, your schema defines data as a oneOf with either a string or an array (without specifying an inner type). This results in a valid match on the string subschema in the oneOf as well as a valid match for the array subschema once the string data is coerced into an array.

This makes the schema invalidate because for the oneOf only a single subschema should match. Perhaps you wanted to use the anyOf which is allowed to match multiple subschemas.

In earlier version your case was valid but with the improvements made in #384 the type coercion was extended to also include arrays. This is part of 5.3.0 and 6.x This also explains why you're getting into this error you're describing here.


@madman-81 something similar applies for you as well. Combining oneOf with type coercion for you is resulting in the null subschema causing the inner value (array of objects with lat/long properties) to a single object with lat/long properties. See here. So during the check for a null value a change to your data is done (as described in the README.md)

Please note that using CHECK_MODE_COERCE_TYPES or CHECK_MODE_APPLY_DEFAULTS will modify your
original data.

After the change the second subschema is checked which is now no longer valid due to the changes made by the first subschema. This might be actually a true problem as the second subschema should be checked against the original data not the version altered by the first subschema. Would you mind opening an separate issue for this as it clearly something different from the original report. Edit: scratch that I've reported #790

Perhaps the following could already resolve the issue of the first sub schema altering the value causing the second sub schema to be invalid for the changed data

diff --git a/src/JsonSchema/Constraints/UndefinedConstraint.php b/src/JsonSchema/Constraints/UndefinedConstraint.php
index 4d955cf..caa0341 100644
--- a/src/JsonSchema/Constraints/UndefinedConstraint.php
+++ b/src/JsonSchema/Constraints/UndefinedConstraint.php
@@ -352,14 +352,17 @@ class UndefinedConstraint extends Constraint
 
         if (isset($schema->oneOf)) {
             $allErrors = [];
-            $matchedSchemas = 0;
+            $matchedSchemas = [];
             $startErrors = $this->getErrors();
+            $originalValue = $value;
+
             foreach ($schema->oneOf as $oneOf) {
+                $originalValue = $value;
                 try {
                     $this->errors = [];
-                    $this->checkUndefined($value, $oneOf, $path, $i);
+                    $this->checkUndefined($originalValue, $oneOf, $path, $i);
                     if (count($this->getErrors()) == 0) {
-                        $matchedSchemas++;
+                        $matchedSchemas[] = ['schema' => $oneOf, 'value' => $originalValue];
                     }
                     $allErrors = array_merge($allErrors, array_values($this->getErrors()));
                 } catch (ValidationException $e) {
@@ -367,10 +370,12 @@ class UndefinedConstraint extends Constraint
                     // other schema options in the OneOf field.
                 }
             }
-            if ($matchedSchemas !== 1) {
+
+            if (count($matchedSchemas) !== 1) {
                 $this->addErrors(array_merge($allErrors, $startErrors));
                 $this->addError(ConstraintError::ONE_OF(), $path);
             } else {
+                $value = $originalValue;
                 $this->errors = $startErrors;
             }
         }

@jozefbriss
Copy link
Author

@DannyvdSluijs Thank You. I have switched to anyOf.

Further, specification of inner type for array from the example did not fix the problem. Simply "ABC" is a string and can be coerced to an array of strings ["ABC"].

It seems type coercion and oneOf will never play well together as noted in my initial comment.

A possible improvement could be to do two passes through oneOf when the type coercion is turned on. The first pass without the type coercion, if the first pass fails then the second pass with the type coercion. Though, I am not familiar enough with the project to say if this is feasible or not.

@DannyvdSluijs
Copy link
Collaborator

A possible improvement could be to do two passes through oneOf when the type coercion is turned on. The first pass without the type coercion, if the first pass fails then the second pass with the type coercion. Though, I am not familiar enough with the project to say if this is feasible or not.

That could be done however that would have some negative impact on the performance. I think for now using the anyOf is best.
Closing the issue for now, feel free to reopen when more information or related cases are discovered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants