search

netmail-open / wjelement

advanced, flexible JSON manipulation in C
GNU Lesser General Public License v3.0
108 stars 56 forks source link

Issues with number/integer on schema validation #87

Closed coreystinson closed 4 years ago

coreystinson commented 4 years ago

Hey guys,

Great project! This is exactly what I was looking for and very thankful to not be re-inventing the wheel.

Having said that, I think I may have encountered two bugs with how the validation engine is managing number and integer types.

It seems that by default, the reader engine will parse a document and (correctly) identify an integer as a WJR_TYPE_INTEGER. From my reading of the jSON Schema draft, this should be able to pass validation as either a WJR_TYPE_NUMBER or WJR_TYPE_INTEGER, but the validation engine will fail a node if its actually type is WJR_TYPE_INTEGER but the corresponding schema entry is type "number". The simple fix would appear to be allowing schema "number" types to be either of the aforementioned constants?

Secondly, type "integer" in the schema is completely broken, at least when build using MSVC 2017. Referring here to line 316 of schema.c, where I needed to add parenthesis around the type comparison to get the whole expression to evaluate as desired.

Because I'm new to jSON schemas I almost feel like I'm misunderstanding something, but I'd be happy to either submit a patch or fork this with the submit the patches, if desired?

penduin commented 4 years ago

Good catches, thank you! If you have a patch or a pull request I'll be happy to merge it, otherwise when I have some time you've already spelled out pretty well where those bugs are.

coreystinson commented 4 years ago

I've attached a patch here which should get for you the necessary and very minor changes. Note: edit patch as needed to remove MSVC solution changes which are probably not wanted for your master branch. 0001-Patches-to-schema.c-to-fix-validation-issues-with-ty.zip

penduin commented 4 years ago

pushed a fix 1c792c1..cc087dc

WJR_TYPE_INTEGER is actually conditionally defined; from wjreader.h:

ifdef WJE_DISTINGUISH_INTEGER_TYPE

WJR_TYPE_INTEGER        = 'I',

endif

(maybe that's not so smart anymore, but I didn't change it just now.) I took your changes and wrapped them with similar directives. thanks @coreystinson !