Skip to content
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

Remote references not fetched #17

Open
gberche-orange opened this issue Jan 30, 2017 · 7 comments
Open

Remote references not fetched #17

gberche-orange opened this issue Jan 30, 2017 · 7 comments

Comments

@gberche-orange
Copy link

Thanks for sharing this great validator with the community.

I could not make references with remote URI to fetch the remote schema and validate their constraints:

When trying to validate following schema:

{
  "$schema": "http://json-schema.org/draft-04/schema#",
  "description": "schema for mysql service",
  "type": "object",
  "allOf": [
    {
      "$ref": "https://raw.githubusercontent.com/orange-cloudfoundry/servicebroker/dialogue-tree-subschemas/tests/schema/use_cases/billing_account/schemas/osb-core-schema.json"
    },
    {
      "oneOf": [
        {
          "properties": {
            "x-servicebroker-context": { "$ref": "#/definitions/standard-plan"},
            "x-servicebroker-request": { "$ref": "#/definitions/empty"}
          }
        },
        {
          "properties": {
            "x-servicebroker-context": { "$ref": "#/definitions/premium-plan"},
            "x-servicebroker-request": { "$ref": "#/definitions/account-id-x-servicebroker-request"}
          }
        }
      ]
    }
  ],
  "definitions": {
    "premium-plan": {
      "type": "object",
      "properties": {
        "plan-id": {
          "enum": [
            "premium-plan-guid"
          ]
        }
      }
    },
    "standard-plan": {
      "type": "object",
      "properties": {
        "plan-id": {
          "enum": [
            "standard-plan-guid"
          ]
        }
      }
    },
    "account-id-x-servicebroker-request": {
      "type": "object",
      "properties": {
        "billing-account": {
          "type": "string"
        }
      },
      "required": [
        "billing-account"
      ]
    },
    "empty": {
      "description": "syntaxic sugar to specify an empty object",
      "type": "object",
      "additionalProperties": false
    }
  }
}

against this incorrect document, then no validation error is displays, where it should complain.

{
  "x-servicebroker-context": {
    "service-id": "postgresql-service-guid",
    "plan-id": "premium-plan-guid",
    "method": "create"
  },
  "x-servicebroker-request": {
    "billing-account": "1234"
  }
}

As a comparison, it seems to work at http://www.jsonschemavalidator.net/

Might be worth at least failing to validate the schema with remote ref URIs as to avoid wrong expectations by users.

Thanks again,

Guillaume.

@nickcmaynard
Copy link
Owner

OK. It looks like our validation libraries don't do this for us transparently.

It's possible (certainly with AJV for draft-04 upwards, not sure about JSV for draft-01 -> -03), but there are a couple of subcases here that we need to think about:

  1. The remote URL is CORS-enabled for jsonschemalint.com. We can probably parse the document and pre-download the schema automatically, providing them to AJV as "extras".
  2. The remote URL is not CORS-enabled. In this case, the tool cannot download the schema automatically. It would have to be uploaded somehow, via some extra GUI.

JSON Schema Validator gets around this by submitting your schema and document to a server-side process - that can go ahead and download whatever it wants, so everything's the first subcase. Because jsonschemalint.com is pure client-side, we can't do that without a serious rearchitecting.

Anyway - subcase 1 is relatively simple - I'd be happy to accept a PR. Subcase 2 is not. I think we'd have to treat it like subcase 1 and catch the CORS errors, revealing them as validation issues.

@nickcmaynard
Copy link
Owner

@gberche-orange If you could drop a saved-gist example here for recreation, that would be helpful.

@nickcmaynard
Copy link
Owner

... scratch that, I'm a dummy.

@nickcmaynard
Copy link
Owner

#27 doesn't fix this, but it does ensure that this remote reference issue throws an error, rather than silently failing.

@nickcmaynard
Copy link
Owner

If anyone else would benefit from this, please raise your hands.

@eliasgruenewald
Copy link

Are there any plans to introduce this feature of external references? Is there an easy way of a local workaround?

@nickcmaynard
Copy link
Owner

I do not have plans to implement this myself. Please see #17 (comment) for the approaches I'm suggesting. Happy to accept PRs.

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

No branches or pull requests

3 participants