Add metaschema tests for `patternProperties`

[hansdals]

Hi. I'm working on a schema validator for C using json-c. It's called jsonc-daccord.

Currently I'm running my test runner on the Test Suite tests/draft2020-12/patternProperties.json,

I noticed that it does not cover invalid regex patterns, and it does not cover "ignore non-object schemas"

The draft https://json-schema.org/draft/2020-12/json-schema-core.html#name-patternproperties says

• The value of "patternProperties" MUST be an object.
  What if it isn't ? Shall the patternProperties keyword be ignored ?
• Each property name of this object SHOULD be a valid regular expression, according to the ECMA-262 regular expression dialect
  When it says SHOULD, it means it is recommended.. Here is my problem, I need to test my code for handling of a failed regex since C code can risk of running into segfaults and memory leaks, but since the draft indicates it is recommended, how should the outcome of the test be? If we put a test in the patternProperties.json file, the result cannot be both true and false (or ignore). I wonder if somehow the tests "valid" field should have allowed for "ignore" as well as "true" and "false". A "valid": "ignore" would increase the coverage of the tests. Or perhaps better, to make it backwards compatible, a new field could be added to the json next to "valid", it could says "ignore": true.

for clarity, here is what I suggest:

[
        {
            "description":
                "patternProperties with invalid regex",
            "schema": {
                "patternProperties": {
                    "invalid regex, I cannot thing of one right now :)": {"type": "integer"}
                }
            },
            "tests": [
                {
                    "description": "a single valid match is valid",
                    "data": {"foo": 1},
                    "valid": true,
                    "ignore-result": true
                },
            ]
        }
]

What do you think ? :)

[Julian]

Hello!

The need for tests beyond just valid or invalid is a long-standing issue but one that has to be done carefully given we don't have a formal backwards compatibility policy in the suite (essentially meaning we risk breaking lots of users of the suite). We can't simply start adding additional values to valid, but something like your suggestion to add side-by-side tests is likely what we'll end up with.

We indeed don't have a metaschema test for patternProperties with a non-object -- the MUST there does not mean it's ignored if it's a non-object, it means the schema is invalid if it's not an object.

A PR to add a metaschema test would indeed be welcome for both of these cases, which is something we can express today in the suite - here's an example of how to write one if you care to help:

JSON-Schema-Test-Suite/tests/draft2020-12/id.json

Line 5 in 0b777ff

"$ref": "https://json-schema.org/draft/2020-12/schema"

[hansdals]

Thanks Julian. I can for my C code add side tests.

I noticed I forgot to ask about "ignore non objects". I could be wrong here but in my code, the very first thing I check in my patternProperties validation function is if the JSON object type is an object. If the JSON is not an object the function returns valid (as in "ignore non object JSON"). Is this correct ? I guess the same question goes for the properties validation function as well.

For clarity here is my function:

int _jdac_check_patternproperties(json_object *jobj, json_object *jschema)
{
    //printf("%s\n", __func__);
    if (!json_object_is_type(jobj, json_type_object))
        return JDAC_ERR_VALID; // ignore non objects

    json_object *jprops = json_object_object_get(jschema, "patternProperties");
    if (jprops) {
        json_object_object_foreach(jprops, jprop_key, jprop_val) {
            //printf("key of prop is %s\n", jprop_key);
            regex_t regex;
            int reti = regcomp(&regex, jprop_key, REG_EXTENDED);
            if (reti) {
                fprintf(stderr, "Could not compile regex\n");
                return JDAC_ERR_SCHEMA_ERROR;
            }
            json_object_object_foreach(jobj, jobj_key, jobj_val) {
                //printf("  key of jobj is %s\n", jobj_key);
                reti = regexec(&regex, jobj_key, 0, NULL, 0);
                if (reti==0) {
                    //printf("match: %s\n", jobj_key);
                    int err = jdac_validate_node(jobj_val, jprop_val);
                    if (err) {
                        regfree(&regex);
                        return err;
                    }
                }
            }
            regfree(&regex);
        }
    }
    return JDAC_ERR_VALID;    

}

[Julian]

Side tests are probably best for now yeah until we can express these in the suite, which I hope to do soon.

It's important to differentiate between when the instance being validated is not an object (where indeed then the spec says you consider it valid as your code seems to correctly) and separately where the value in the schema that patternProperties has is not an object, which indicates that the schema itself isn't valid JSON schema. That's what we cannot today represent, though sure your C implementation may need to consider it -- it's the equivalent of what happens if something hands your implementation invalid JSON entirely, some pure garbage like diwosbfjdl;;()// -- you need to consider what your implementation does in that case, but the spec basically just says "that's not valid JSON, let alone valid JSON Schema". Here, non-object values for patternProperties are similar, they're not valid JSON Schema, so the spec doesn't proscribe specifically in what way your implementation indicates that.

Does that help?

[Vinit-Pandit]

Hii @Julian please check if this test matches the requirements for this issue

{
        "description" : "metaschemas test for patternProperties",
        "schema" : {
            "$schema": "https://json-schema.org/draft/2020-12/schema",
            "patternProperties":{
                "a" : {"$ref" : "https://json-schema.org/draft/2020-12/schema"}
            }
        },
        "tests":[
            {
                "description" : "non object value", 
                "data": {"a" : 123},
                "valid": false
            },
            {
                "description" : "non object value",
                "data": {"a" : ""},
                "valid":false
            },
            {
                "description" : "empty object value",
                "data" : {"a" : {}},
                "valid": true
            },
            {
                "description": "boolean value",
                "data" : {"a" : true},
                "valid" : true
            }

        ]
 }

[Julian]

Considering we backed some of these out (in #718 via #716 (comment)) it actually probably makes sense to close these I think.

Thanks for giving it a shot though.