GenericSchemaDocument does not report errors

[smhdfdl]

There is currently no concept in the GenericSchemaDocument of detecting errors in the JSON document it is 'compiling'. It can't return an error, because all the processing happens in the constructor. But it could set a private member and provide a getter method to return an error code. The class is obviously written to be tolerant but I think some error reporting is needed. In our usage, we validate a JSON schema against the Draft-04 metaschema, before letting it be used to validate instance documents, but that doesn't catch things like failure to resolve a $ref. @miloyip your thoughts please. I am happy to create a PR.

[miloyip]

The constructor currently ignore any unsupported schema.
When I wrote it, I thought it would be the best using meta-schema.
For ref, it seems we can use the remote resolver to detect whether the resolution is succeeded.
Any other errors that cannot be detected by meta-scehma?

[smhdfdl]

@miloyip I think just $ref-related errors.

• The value after the # is not a valid JSON pointer
• The JSON pointer does not refer to a location in the schema document

Please note that I am running with PR #1393 merged. It is a very useful change when the schema is embedded with an OpenAPI definition.

[miloyip]

I checked #1393 have CI error now.
I think you may merge that in your branch and make a new PR.

[smhdfdl]

@miloyip I have changes for this working in my local copy. They are not yet visible in my fork because they depend on PR #1848.

[smhdfdl]

@miloyip I have created a branch in my fork schema-errors for these changes. No PR yet as it is dependent on PR #1848 which is waiting for your review and merge. These are the error checks that I have added.

enum SchemaErrorCode {
    kSchemaErrorNone = 0,                      //!< No error.

    kSchemaErrorStartUnknown,                  //!< Pointer to start of schema does not resolve to a location in the document
    kSchemaErrorRefPlainName,                  //!< $ref fragment must be a JSON pointer
    kSchemaErrorRefInvalid,                    //!< $ref must not be an empty string
    kSchemaErrorRefPointerInvalid,             //!< $ref fragment is not a valid JSON pointer at offset
    kSchemaErrorRefUnknown,                    //!< $ref does not resolve to a location in the target document
    kSchemaErrorRefCyclical,                   //!< $ref is cyclical
    kSchemaErrorRefNoRemoteProvider,           //!< $ref is remote but there is no remote provider
    kSchemaErrorRefNoRemoteSchema,             //!< $ref is remote but the remote provider did not return a schema
    kSchemaErrorRegexInvalid                   //!< Invalid regular expression in 'pattern' or 'patternProperties'
};

It works in a similar way to the validator errors, and creates a very similar error structure which you can access using GenericSchemaDocument::GetError().

I have encountered one problem, and that is with check for kSchemaErrorRegexInvalid. I have found several instances of regexes that are valid but are flagged as invalid by the RapidJSON internal regex class. This prevents, for example, validation of an OpenAPI document against the OpenAPI meta-schema, as the meta-schema uses such regexes. I also found that using std::regex did not fix this. The only solution I found that reliably worked on all platforms was to use boost::regex.

Example: ^\\S*$

The alternative is not to check regexes, which is what your code does today. This is bad, as this has the result of not detecting validation errors where the JSON property value does not match the regex, so is a false positive. Please advise how you would like to proceed.

[smhdfdl]

@miloyip Please note that I have further changes, not yet in a branch, that add detection of $schema draft and openapi version. This adds 3 further schema errors, and so is dependent on this issue. This 'specification' change allows extensions to be added to support validation according to OpenAPI 2.0 and 3.0.x rules, which I have also added. And of course it allows future extension to support drafts 06, 07, etc. These changes will be in a separate PR.