Version 1.2.3 introduced breaking change affecting prototypes

[jasonpincin]

We've been using jsonschema to validate JavaScript objects that inherit properties from prototype chain.

https://github.com/tdegrunt/jsonschema/commit/107ac1afe7cd18fa55e25f1a70d28f1adaab8dc7#diff-aa61954d23c3708bb1b5cafc3163069dR198

^ The change made here broke support for this and caused code of ours to fail by doing a patch version upgrade (1.2.2 -> 1.2.3).

This change removes the ability for jsonschema to be used in this way. Some thoughts of mine around this issue include:

If you want to validate objects without prototypes, create objects without prototypes:

var o = Object.create(null);

I realize JSON.parse creates objects that have the Object.prototype in their prototype chain by default, but this can be changed via the reviver argument to JSON.parse, for example:

JSON.parse(jsonString, (k, v) => {
  if (v && typeof v === 'object' && !Array.isArray(v)) {
    return Object.assign(Object.create(null), v);
  }
  return v;
});

Perhaps jsonschema could expose validation options or transformation utilities to assist with this so that formerly valid use-cases of jsonschema are not needlessly eliminated.

Note: Some previous discussion related to the above bug took place here.

[awwright]

What I see the Object.create(null) workaround doing is telling people "If you want JSON Schema to work like it's documented, you have to add/change how you're parsing JSON into an object." I don't see that as being very accessible to the vast majority of usage.

And a lot of tutorials ship with the caveat, "use hasOwnProperty inside for(var key in object) otherwise you might see unexpected behavior". So I'm inclined to think this is the right way to do it.

But being able to validate objects with prototypes is a novel and useful feature too.

Here's some things to consider.

0. Issue #121 might be relevant to you - an extension that specifies that the instance is an instanceof some object, i.e. has object in its prototype chain.

1. Implement a runtime flag that switches the default behavior.

2. Determine if the property is "enumerable", instead of local(hasOwnProperty). This would exclude all the problematic properties like "constructor" that are always in every object.

3. Only consider properties that are looped over by for(var key in instance), effectively the same thing as above, however this approach would require a large rewrite and breakage of behavior (because of how the code iterates over the schema "properties" separately)

4. Introduce a new JSON Schema keyword/keywords, that makes this distinction that's not in JSON natively.

Thoughts? Anything else?

[jasonpincin]

Agree with your sentiments on not interfering with native JSON parsing. 👍 I was approaching it from the opposite angle for demonstrative purposes.

On your list of considerations, 1 and 4 are interesting. I think relying on enumerable would be a sensible default, but should be configurable. Checking for the existence of non-enumerable properties (functioned defined on prototype) is something we've done.

I can lend a hand on this. Do you have a direction in mind?

[landau]

Bump

[awwright]

I took another look at this.

The reason we have to use Object.hasOwnProperty.call is because of this:

> "constructor" in {}
true

Using the in keyword would cause the following to be valid:

validate( {}, { required: [ 'constructor' ] })

But maybe a flag to configure this behavior would work. There's a few different ways to do that though.

[awwright]

This should be fixed with b247d8c34c9c06c9d15c9f530cb12df05800e48a

I figured out a way to write a function that's equivalent to for(key in object) that doesn't seem to have any performance hit.

[awwright]

@jasonpincin @xavdid @landau I released 1.2.8, which should fix this issue. No code changes should be necessary.

Please open a new issue if you have any further problems.