bpdusk / jsonschema

Automatically exported from code.google.com/p/jsonschema
0 stars 0 forks source link

Arrays/Objects with circular references cause infinite recursion in jsonschema-b4.js #15

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Issue

What steps will reproduce the problem?

1. Create an object/array with a circular reference, either direct or
indirect.  For example, a pair of instances of schemas in the extension
example in the Extending and Referencing section of the JSON Schema Proposal.

2. Attempt to validate the instance using an appropriate schema (again I
used the schema in the Extending and Referencing section of the JSON Schema
Proposal).

What is the expected output? What do you see instead?

The expected output is, of course, an object indicating whether or not the
instance is valid.  Instead, an error is thrown by Firefox indicating that
there has been "Too much recursion".

What version of the product are you using? On what operating system?

jsonschema-b4.js, Firefox 3.5.1, Windows Vista

Please provide any additional information below.

I've solved this problem in the attached files by simply adding, in
checkProp, a __jvSeen__ property to each object encountered without it and
returning from checkProp for objects that have it.  In order to remove
these properties without having to traverse the entire object/array I added
an array to the _validate method wherein I register an object the first
time it is encountered.  At the end of the _validate method I simply loop
through this array and call delete on the __jvSeen__ property.

The relevant changes occur on lines: 60, 64-68, and 248-250.  Note that
these lines are with respect to the file jsonschema-b4.2.js which I have
attached.  I've made some other changes in that file as well which will
cause the line numbers to be different from those in jsonschema-b4.js.  

Just to note those changes here in case you would like to integrate them
(obviously these changes only reflect my preferences):

- The JSONSchema namespace has been changed to JSON.schema to mirror the
implementation in the Dojo library.  The "property" property in the error
object has been changed to "path" and to the path has been added the root
($) indicator.  
- A problem with the additionalProperties restriction applying to tuple
types has been resolved (which I'll post another issue for).
- The additionalProperties restriction with respect to tuple types has been
changed to additionalItems to be consistent with the rest of the proposal
using the word items where tuple types are concerned also, in my opinion,
reducing confusion. This would also make a more rigorous validation of
schema extensions possible as the rules necessary to validate
additionalProperties with respect to objects would be different than those
when applying it to arrays (validating the schema itself rather than an
instance of the schema).
- The text of the usage/summaries has been reflowed as the previous flow
was getting on my nerves, X-D.

Thoughts

I haven't done any performance tests so I don't know what the impacts are
of this solution in that regard.

It may be better to use a randomly generated property name pre/suffixed by
__ instead of jvSeen to ensure that an existing property is not
overwritten.  I'm not sure of the necessity of this, however.  The __
prefix ensures, according to the current code, that the property will be
ignored in terms of validation.

Files Attached

crjsTest.html (My test html file)
json-ref.js  (crjsTest.html makes use of this.  I restructured the parse
method into parse and resolveRefs to allow references in objects to be
resolved without having to convert the object to a string and then back to
an object.  I also switched the namespace to JSON.ref to mirror the Dojo
implementation.)
jsonschema-b4.2.js (crjsTest.html makes use of this.  My fixed implementation)

Sorry for all the modifications, these files are for my personal use and as
such I've modified them to my tastes.

Original issue reported on code.google.com by swessits...@gmail.com on 3 Aug 2009 at 10:14

Attachments: