Possibility to fail if string format is unknown

[peterpostmann]

For security reasons I want the validation to fail in case that there are unknown attributes.

The spec says "If the type of the instance to validate is not in this set, validation for this format attribute and instance SHOULD succeed", hence I left the default behavior unchanged.

I added the possibility to add a default handler if the format attribute is unknown.

[matt-allan]

Hello,

Thanks for the PR.

The spec says "If the type of the instance to validate is not in this set, validation for this format attribute and instance SHOULD succeed", hence I left the default behavior unchanged.

Based on the other wording in the spec I think this is actually referring to the type of the input JSON, not the format parameter. For example, if you specify "format": "date-time" and the input is null it will pass because the spec states 'This attribute applies to string instances.' It does not mean that the validator should ignore unknown formats. It doesn't look like the spec clarifies how unknown formats should be handled.

I think that being strict about unknown formats is a great idea and would make sense as a default. There are two things I would change with the implementation:

This implementation is returning a validation error if the format is unknown. The library only uses validation errors for issues with the data, not the schema. For schema issues it throws an InvalidSchemaException.

I'm not sure this behavior needs to be combined with the format extensions since you just need to throw an InvalidSchemaException if the format is unknown (unlike a format extension which needs to validate the data, build error messages, etc). Instead this could be a constructor parameter that determines if we throw on unknown formats. Something like:

default:
    if (!$this->ignoreUnknownFormats) {
        throw new InvalidSchemaException('...');
    }

Making it optional and defaulting to true would allow releasing this change without having to wait for a major version bump. Then we can change the default in version 2.0.

What do you think, would that work for your use case?

[peterpostmann]

Hi Matt,

yes that would work for me was well. I would suggest to add a setter as well.

How do I change the PR? Can I just push and rebase?

[matt-allan]

Great! Setter is a good idea too.

Push and rebase is fine. I will squash when I merge so you don't really need to worry about it too much.

Thanks!

[peterpostmann]

Hi,

I updated the code, please have a look!

[peterpostmann]

Hi updated the parameter bool but travis seems to be stuck? Furthermore I don´t know what to do to resolve the conflicting files

[matt-allan]

Hi,

I fixed the conflict locally; I will merge it soon.

[matt-allan]

Merged in 225d650de8375c1746da8eab644ef5a9afda9c91. Thanks!