Bug with oneOf and catch

[vsouz4]

If you have your definition with something like:

    SomeObjectHere:
      type: object
      properties:
        some_property_here:
          oneOf:
            - type: object
            - type: string
              maxLength: 50
              minLength: 1

You're supposed to accept requests like:

{
    "some_property_here": {}
}

and

{
    "some_property_here": "Some Value Here"
}

The second requests works fine but the first one complains that { } must be a string, because on lezhnev74/openapi-psr7-validator/src/Schema/Keywords/OneOf.php:

        foreach ($oneOf as $schema) {
            try {
                $schemaValidator->validate($data, $schema, $this->dataBreadCrumb);
                $matchedCount++;
            } catch (SchemaMismatch $e) {
                // that did not match... its ok
            }
        }

you're supposed to catch the exceptions that validate might throw, ignore them and count it. But for this scenario you're not catching the exception with the MaxLength:

        try {
            Validator::stringType()->assert($data);
            Validator::intType()->assert($maxLength);
            Validator::trueVal()->assert($maxLength >= 0);
        } catch (ExceptionInterface $e) {
            throw InvalidSchema::becauseDefensiveSchemaValidationFailed($e);
        }

and you're checking MaxLength for {} (which is not string) so Validator::stringType()->assert($data) fails, throwing InvalidSchema and you're catching only SchemaMismatch so either you do the checks for maxLength, minLength etc. only for correspondent types (iirc you can only apply certain constraints to certain types and you're checking all constraints for all types) or you add |InvalidSchema on OneOf and AnyOf.

For now I worked around this issue using AnyOf which processes until any criteria is valid and placing type: object before type: string so that it gets evaluated first.

[scaytrase]

yea, good catch. we should do a rethrow for first assert (not for other) чт, 15 авг. 2019 г., 13:49 Vitor de Souza <notifications@github.com>:

…

If you have your definition with something like: SomeObjectHere: type: object properties: some_property_here: oneOf: - type: object - type: string maxLength: 50 minLength: 1 You're supposed to accept requests like: { "some_property_here": {} } and { "some_property_here": "Some Value Here" } The second requests works fine but the first one complains that { } must be a string, because on lezhnev74/openapi-psr7-validator/src/Schema/Keywords/OneOf.php: foreach ($oneOf as $schema) { try { $schemaValidator->validate($data, $schema, $this->dataBreadCrumb); $matchedCount++; } catch (SchemaMismatch $e) { // that did not match... its ok } } you're supposed to catch the exceptions that validate might throw, ignore them and count it. But for this scenario you're not catching the exception with the MaxLength: try { Validator::stringType()->assert($data); Validator::intType()->assert($maxLength); Validator::trueVal()->assert($maxLength >= 0); } catch (ExceptionInterface $e) { throw InvalidSchema::becauseDefensiveSchemaValidationFailed($e); } and you're checking MaxLength for {} (which is not string) so Validator::stringType()->assert($data) fails, throwing InvalidSchema and you're catching only SchemaMismatch so either you do the checks for maxLength, minLength etc. only for correspondent types (iirc you can only apply certain constraints to certain types and you're checking all constraints for all types) or you add |InvalidSchema on OneOf and AnyOf. For now I worked around this issue using AnyOf which processes until any criteria is valid and placing type: object before type: string so that it gets evaluated first. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#57?email_source=notifications&email_token=ABSGB3JZ3JS5YWLWWY4DNPLQEUYCTA5CNFSM4IL5EHFKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HFM63OQ>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABSGB3P7S5MRW3SFB37BJKDQEUYCTANCNFSM4IL5EHFA> .

[scaytrase]

Hm, maybe this one just an invalid order of checks. we should check it is a string first, than check maxLenght. @lezhnev74 WDYT?

[scaytrase]

@vsouz4 check #59

[vsouz4]

nice, that solved it @scaytrase ! 🕺 👍