Skip to content

Recursive load fix #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 17, 2017
Merged

Conversation

thekensta
Copy link

Hi

I have found that loading JsonSchema by URL creates an infinite loop where the schema uses its id property to reference itself.

I am using the "id" property of the schema as an explicit URL reference to the schema.

Example:

using test/resource/self_ref/selfRef.json as

{
  "id": "http://localhost:1234/self_ref/selfRef.json",
  "description": "Schema with ID set to its own URL"
}

and creating a test case in JsonSchemaTest.java

    @Test(expected = java.lang.StackOverflowError.class)
    public void testLoadingWithId() throws Exception {
        URL url = new URL("http://localhost:1234/self_ref/selfRef.json");
        JsonNode schemaJson = mapper.readTree(url);
        JsonSchemaFactory factory = new JsonSchemaFactory();
        JsonSchema schema = factory.getSchema(schemaJson);
    }

will result in a stackoverflow error from BaseJsonValidator.ontainSubSchemaNode()

I have added a fix in this patch

  • added Constructor to BaseJsonValidator to accept an explicit subSchema
  • added Constructor to JsonSchema to accept an explicit subSchema
  • in JsonSchemaFactory.getSchema(URL url) check for an id node and if that matches the requested id. If the id matches the requested URL, then create a JsonSchema node with a null subSchema.
  • Added the tests above
  • tidied duplicate constructor code in JsonSchema with an init() function

@stevehu stevehu merged commit f34e3b9 into networknt:master Jun 17, 2017
@stevehu
Copy link
Contributor

stevehu commented Jun 17, 2017

@thekensta Thanks for your help.

@mperkin
Copy link

mperkin commented Jul 1, 2019

Hi - I just tried this with the latest release - v1.0.16 - and I am getting the same stackoverflowerror. Has the fix been released?

Thanks

@stevehu
Copy link
Contributor

stevehu commented Jul 1, 2019

@mperkin Yes. It has been released; however, I think the fix might not cover the use case you have. Could you please submit a PR with your test cases? It would help developers to understand your use case and get it fixed. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants