thephpleague / json-guard

Validation of json-schema.org compliant schemas.
http://json-guard.thephpleague.com/
MIT License
175 stars 26 forks source link

patternProperties doesn’t appear to be working #104

Closed RicLeP closed 7 years ago

RicLeP commented 7 years ago

When using patternProperties it doesn’t seem to run the validation. I can see it matches my rule in PatternProperties.php but it won’t do anything with it.

In the below example the patternProperties will match the ‘600’ property in the json file. However, even though that includes an ‘additional’ property and excludes the required ‘notincludedin600’ it still passes validation.

If was was to test directly for ‘600’ like so

"properties": {
    "default": { "$ref": "#/definitions/fileSettingsDimensions" },
    "600": { "$ref": "#/definitions/fileSettingsDimensions" }
}

It will fail as expected

--

Schema:

{
    "$schema": "http://json-schema.org/draft-04/schema#",
    "title": "Schema",
    "type": "object",
    "patternProperties": {
        "^[0-9]+$": { "$ref": "#/definitions/fileSettingsDimensions" }
    },
    "properties": {
        "default": { "$ref": "#/definitions/fileSettingsDimensions" }
    },
    "definitions": {
        "fileSettingsDimensions": {
            "type": "object",
            "required": [
                "height",
                "width",
                "notincludedin600"
            ],
            "properties": {
                "height": {
                    "type": [
                        "integer",
                        "string"
                    ],
                    "pattern": "^auto$"
                },
                "width": {
                    "type": [
                        "integer",
                        "string"
                    ],
                    "pattern": "^auto$"
                },
                "notincludedin600": {
                    "type": "string"
                }
            },
            "additionalProperties": false
        }
    }
}

--

json

{
    "default": {
        "width": 300,
        "height": 200,
        "notincludedin600": "blah"
    },
    "600": {
        "width": 600,
        "height": 200,
        "additional": true
    }
}
matt-allan commented 7 years ago

The JSON Pointer implementation can't find ^[0-9]+$, so that isn't being dereferenced.

When parsing the pointer we urldecode it, but we didn't encode it when building it so it tries to find ^[0-9] $.

I think the urldecode is only necessary if the pointer is in a URI fragment so we might be able to skip it in these cases instead of encoding the pointer just to decode it.

RicLeP commented 7 years ago

Thanks, I understand now. Got the code to work if I don’t use a reference.

I just commented out the urldecode as a test and you’re right, it’ll work if skipped when using a reference on a patternProperty.

RicLeP commented 7 years ago

Looks like this should work. If we check for patternProperties in the pointer path then skip the URLDecode method. Not sure if untilde() needs skipping too?

src/Pointer/Parser.php

/**
 * @param string $pointer
 *
 * @return array
 */
private function parse($pointer)
{
    if (strpos($pointer, 'patternProperties')) {
        return $this
            ->explode($pointer)
            ->untilde()
            ->get();
    }

    return $this
        ->explode($pointer)
        ->urlDecode()
        ->untilde()
        ->get();
}
matt-allan commented 7 years ago

Hey @SirriC,

Thanks for looking into this. I'm in the middle of rewriting the pointer and dereferencing code for 1.0. I pushed a fix last night here.

The idea behind the change is we only need to urldecode the pointer if it starts with #, since that means it's a URL fragment. Since the pointer to your schema is /patternProperties/^[0-9]+$ it won't get encoded.

You won't want to use that code yet because it's unstable 😀 . The last thing I need to do for the 1.0 release is improve the error messages so hopefully I will be able to release 1.0 in the next week.

RicLeP commented 7 years ago

Great, I look forward to version 1.0, been enjoying working with json schema on a new project and your package works well.

matt-allan commented 7 years ago

I just tagged 1.0.0-alpha which includes this fix. I'm going to close this issue but feel free to re-open it if you encounter this issue again.