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

Validate format constraints on string properties as described in OpenAPI 3.0 spec #374

Open
daffinm opened this issue Aug 10, 2017 · 6 comments

Comments

@daffinm
Copy link
Contributor

daffinm commented Aug 10, 2017

For example, the following fragment gives no warnings or errors and yet it is not legal according to the specification.

---
openapi: "3.0.0"
info:
  version: "1.0.0"
  title: Whatever you like
  description: API_description
  termsOfService: EXPECT ERROR: Spec says "MUST be in the format of a URL."
  contact:
    name: contact_name
    url: EXPECT ERROR: Spec says "MUST be in the format of a URL."
    email: EXPECT ERROR: Spec says "MUST be in the format of an email address."
  license:
    name: MIT
    url: EXPECT ERROR: Spec says "MUST be in the format of a URL."
paths: {}

@tedepstein comments:

It's a general problem that traces back to the fact that our JSON schema is auto-generated by a markdown spec-scraping utility built by Tim Burks for the Google Gnostic OSS project. That utility doesn't scrape the format requirement, so we have to scour the spec for these constraints. A search through the spec for "Must be in the format" should help here.

@tedepstein tedepstein changed the title Validation: ensure fields are validated according to the lang spec Validation: ensure properties are validated according to the lang spec Aug 10, 2017
@tedepstein
Copy link
Collaborator

tedepstein commented Aug 10, 2017

@daffinm , thanks for logging this. Please apply the "OpenAPIv3" label to these issues, as I've done on this one.

@tedepstein tedepstein changed the title Validation: ensure properties are validated according to the lang spec Validation: ensure properties are validated according to the OpenAPI spec Aug 10, 2017
@tedepstein tedepstein changed the title Validation: ensure properties are validated according to the OpenAPI spec Validate format constraints on string properties as described in OpenAPI 3.0 spec Aug 30, 2017
ghillairet added a commit that referenced this issue Oct 12, 2017
…in OpenAPI 3.0 spec

Modified schema to validate email property
@ghillairet
Copy link
Member

PR #415 shows how to add email validation.

@ghillairet
Copy link
Member

In PR #415 I put some screencasts showing the current state of url/email validation. Those work in the v2 editor because the v2 schema includes the format email and uri for those properties.

But as shown in the screencasts the validation of url values may be not what we expect (for example a value foo is consider valid URI). The JSON schema validator that we are using, constructs a java.net.URI object to validate those values (see code). According to the JSON schema specification, this is correct because only the format uri is considered. There is a discussion about whether adding url as format for URL validation should be considered or not.

The OpenAPI v3 specification states that those properties should be valid URL. If that means that in this case URI should not be considered, then we could implement our own URL validator (that uses java.net.URL instead of java.net.URI). Or we could consider the current validation sufficient and only adds the missing format properties in the v3 schema.

@tedepstein
Copy link
Collaborator

@ghillairet, I didn't read the entire background on URI vs. URL. But another aspect of this is relative vs. absolute URIs / URLs. Assuming relative URLs are allowed, something like "foo" might actually be valid, syntactically.

In general. I don't think it's worth implementing our own validation for URL syntax.

Where the URL is used as part of a Reference Object (or other reference-object-like construct), we have the additional validation to ensure that the URL resolves to a valid object of the expected type. I think that covers the most important cases for us.

tfesenko added a commit that referenced this issue Oct 16, 2017
[#374] Validate format constraints on string properties as described …
@daffinm
Copy link
Contributor Author

daffinm commented Oct 20, 2017

Just looking at this again as I prep the new release. In the following code sample the values are all direct quotations from the spec. I now get 2 errors instead of none. But I am expecting to see 4, perhaps 6 errors in total:

image

The contact url and email errors are spot on. But what about errors for info/termsOfService & license/url? (The other two are possibilities. Is a sentence value for openapi really valid?)

Note also that the spec clearly says URL and not URI. Why can't we just use a regex?

@daffinm daffinm reopened this Oct 20, 2017
@tedepstein
Copy link
Collaborator

I think openapi should report an error if the value doesn't conform to a semantic version with the major component 3.

info/version should probably not be validated, except that it must be present and (I guess) must be a non-null value. Users can have different ways of expressing versions.

URLs are what we discussed in this PR, above. I'm open to trying a regex as our own custom validation of URL-format string properties. But I really don't want to burn a lot of development time or support time on this. If we think the URL regex will be foolproof, OK, let's try it.

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

No branches or pull requests

4 participants