Skip to content

Mixing properties and patternProperties fails for properties not matching the patternProperties definition #705

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

Closed
fabian-mcfly opened this issue Nov 30, 2023 · 4 comments

Comments

@fabian-mcfly
Copy link

Schema (incomplete, just the important part):

"link": {
  {
    "type": "object",
    "properties": {
      "controller": {
        "type": "string",
        "pattern": "^[A-Z][a-zA-Z0-9]+$"
      },
      "action": {
        "type": "string",
        "pattern": "^[a-z][a-z0-9-]+$"
      }
    },
    "patternProperties": {
      "^[a-z][a-z0-9-]+$": {"pattern": "^[a-z0-9-]+$"}
    },
    "required": ["controller", "action"],
    "additionalProperties": false
  }
}

Data:

"link": {
  "controller": "Dashboard",
  "action": "overview",
  "param1": 123,
  "param2": "foobar"
}

Validating this data will result in an error:
Violations: [link.controller] Does not match the regex pattern ^[a-z0-9-]+$
Maybe I am the problem here, but I understand the "patternProperties" to only require validation for properties that aren't explicitly defined.
Since "controller" already is set, no validation with the regex pattern in patternProperties should occur.

I already found a fix, but I'd like to wait with a PR until someone can tell me whether this is, indeed, a bug or just my misunderstanding of how properties & patternProperties work.

@erayd
Copy link
Contributor

erayd commented Nov 30, 2023

This isn't a bug; it's how the spec says it should behave 🙂

Maybe I am the problem here, but I understand the "patternProperties" to only require validation for properties that aren't explicitly defined.

Your understanding is incorrect. According to the spec, patternProperties applies to all matching properties, and does not exclude those which also match a properties rule. The two keywords operate entirely independently of each other.

The relevant spec wording is:

6.19 patternProperties

The value of "patternProperties" MUST be an object. Each property name of this object SHOULD be a valid regular expression, according to the ECMA 262 regular expression dialect. Each property value of this object MUST be a valid JSON Schema.

This keyword determines how child instances validate for objects, and does not directly validate the immediate instance itself. Validation of the primitive instance type against this keyword always succeeds.

Validation succeeds if, for each instance name that matches any regular expressions that appear as a property name in this keyword's value, the child instance for that name successfully validates against each schema that corresponds to a matching regular expression.

Omitting this keyword has the same behavior as an empty object.

A contrasting example is additionalProperties, which does contain such an exclusion:

Validation with "additionalProperties" applies only to the child values of instance names that do not match any names in "properties", and do not match any regular expression in "patternProperties".

@erayd erayd closed this as completed Nov 30, 2023
@erayd
Copy link
Contributor

erayd commented Nov 30, 2023

Edit: My advice below is incorrect, as the second anyOf branch will still capture the defined properties if their names match the pattern - my apologies. @fabian-mcfly has suggested a workaround that excludes specific properties from the name pattern.

Discussion of this limitation following the release of draft-06 was inconclusive, and the proposed addition of additionalPatternProperties to the spec did not ultimately proceed.

If you are wanting patternProperties to constrain only properties which do not pass validation against properties, the spec-compliant way to do this is with anyOf. For example:

{
    "type": "object",
    "anyOf": [
        {
            "properties": {...},
            "additionalProperties": false
        {,
        {
            "patternProperties": {...}
        }
    ]
}

Note that additionalProperties is necessary in order to ensure that non-matching property names fail validation on the properties branch of the anyOf constraint.

@fabian-mcfly
Copy link
Author

Thanks for the quick reply and clarification. It seems like PHPStorm's implementation is wrong.

Sadly, I cannot make this work using anyOf like this results - it results in errors as well:

"title": {
  "type": "object",
  "anyOf": [
    {
      "properties": {
        "controller": {
          "type": "string",
          "pattern": "^[A-Z][a-zA-Z0-9]+$"
        },
        "action": {
          "type": "string",
          "pattern": "^[a-z][a-z0-9-]+$"
        }
      },
      "additionalProperties": false
    },
    {
      "patternProperties": {
        "^[a-z][a-z0-9-]+$": {"pattern": "^[a-z0-9-]+$"}
      }
    }
  ]
}
[link] The property param1 is not defined and the definition does not allow additional properties 
[link] The property param2 is not defined and the definition does not allow additional properties 
[link.controller] Does not match the regex pattern ^[a-z0-9-]+$
[link] Failed to match at least one schema

I was able to make it work using a negative lookahead in the patternProperties:
^((?!^(controller|action)))[a-z][a-z0-9-]+$

Thanks again and sorry for any inconvenience.

@erayd
Copy link
Contributor

erayd commented Nov 30, 2023

Sadly, I cannot make this work using anyOf like this results - it results in errors

Good call on the regex change - I clearly did not think the implications of what I was suggesting through properly! My apologies, and please ignore my advice from earlier to use anyOf; it's incorrect.

At this point, your workaround using the regex exclusion may be the best path forward - I cannot think of another sane way to do what you need within the confines of the draft-06 spec, which is the newest version that this library supports.

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

2 participants