Skip to content

additionalProperties check is "too aggresive" #471

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
castarco opened this issue Oct 2, 2018 · 1 comment
Closed

additionalProperties check is "too aggresive" #471

castarco opened this issue Oct 2, 2018 · 1 comment

Comments

@castarco
Copy link

castarco commented Oct 2, 2018

Hi, I've found a strange error while using this library (v2.6). The additionalProperties check is behaving too aggressively, rejecting some "JSON" documents that (in theory, as far as I can see) should fit into the schema.

Here the (very simplified) schema (look into the estimator_node definition):

{
  "$schema": "http://json-schema.org/draft-07/schema#",

  "type": "object",
  "title": "Traffic Distribution Specification",
  "description": "Document describing how traffic is distributed across pipelines",

  "properties": {
    "schema_version": {
      "description": "Specifies the schema version used to validate the traffic distribution document: X, X.Y, X.Y.Z",
      "type": "string",
      "pattern": "^([1-9][0-9]*|0)(\\.([1-9][0-9]*|0)(\\.([1-9][0-9]*|0))?)?$"
    },
    "pipeline_parts": {
      "description": "Optional block to make easier grouping pipeline part definitions",
      "type": "object",
      "properties": {
        "estimators": {
          "type": "object",
          "additionalProperties": { "$ref": "#/definitions/estimator_node" },
          "required": []
        }
      },
      "required": []
    }
  },
  "dependencies": {},

  "definitions": {
    "estimator_node": {
      "type": "object",
      "properties": {
        "name": { "type": "string", "minLength": 3 },
        "type": { "type": "string" },
        "train_only": { "type": "boolean", "default": false },
        "params": { "type": "object", "properties": {}, "additionalProperties": true}
      },
      "required": ["name", "type"],
      "additionalProperties": false
    }
  }
}

As you can see, only name and type are required, while params and train_only are not required (but defined in the "properties" object). The validator does not complain in any case when I'm passing a params key to the validated objects, but it complains (incorrectly) when I pass a train_only key telling me that the additionalProperties directive does not allow me to use that key... when the JSON schema spec explicitly tells that the only forbidden keys are the ones not defined in the properties object.

Example of a YAML document (yeah, YAML, but this hasn't been a problem in many months) (I have removed parts of the YAML that contain business data):

schema_version: "2.0"

pipeline_parts:
  estimators:
    estimator_example:
      name: "estimator_example"
      type: "MyMagicEstimator"
      train_only: true  # This is the problematic key
      params:
        a: 1.0
        b: 0.0
@castarco
Copy link
Author

castarco commented Oct 2, 2018

Error on my side, of course.

@castarco castarco closed this as completed Oct 2, 2018
Julian added a commit that referenced this issue Apr 13, 2021
15ec577 Merge pull request #471 from json-schema-org/ether/id-anchor-in-enum
9f97865 test for confusing not-identifiers in enums
0f7ecd4 Merge pull request #475 from marksparkza/marksparkza-patch-1
783d22a Add jschon
fc68499 Merge pull request #472 from json-schema-org/ether/unevaluatedProperties_uncles
ed4cf5f more test cases for unevaluatedItems, unevaluatedProperties
d0d814d Merge pull request #469 from json-schema-org/ether/ipv4-vulnerability
7ca5f36 reject ipv4 strings with an octet with a leading zero
8e1e1c1 fix spelling error in test descriptions
77f1d10 Merge pull request #462 from jdesrosiers/dynamic-ref-tests
72a32fe Merge pull request #468 from json-schema-org/ether/combine-test-cases
0c48ffb Merge pull request #453 from notEthan/float-overflow-d4-int
76a4ba0 these test cases can be combined since the schemas are the same
cd73775 Merge pull request #464 from json-schema-org/ether/format-by-default-always-validates
043dc63 by default, "format" only annotates, not validates
3c45b81 Merge pull request #460 from amosonn/remove-remotes-from-script
b09e48d Fix $ref with siblings in pre-2019-09 tests
ff9f22e Add tests for $dynamicRef/$dynamicAnchor
0faaf09 Fix refs to Draft 2019-09 schema to be refs to 2020-12
ebbcbc8 Use flask to server remotes directly
bb98b03 Remove remotes from bin/jsonschema_suite
fcae732 Merge pull request #455 from jdesrosiers/bootstrap-202012
e002409 Update tests for 2020-12
405b3fb Copy 2019-09 tests to bootstrap 2020-12 tests
1636a22 draft4 float-overflow instance may be considered not an integer
8daea3f Merge pull request #451 from json-schema-org/ether/more-relative-json-pointer
69fe40f some more relative-json-pointer tests
6505944 Merge pull request #450 from json-schema-org/ether/recursiveRef-dynamic-path
afd0cd3 Move content* keyword tests to non-optional
e2b2a4b Change all content* keyword tests to always validate
8999eae $recursiveRef example demonstrating dynamic nature of the resolution scope
f47003f fix duplicate test description
bcf1dc8 Merge pull request #391 from ether/recursiveRef (rebased, squashed)
3d88f34 test $recursiveRef + $recursiveAnchor
3b79a45 Merge pull request #418 from ChALkeR/chalker/contentSchema
29f609b Add tests for contentSchema

git-subtree-dir: json
git-subtree-split: 15ec577f5ddee0115319f4e7f856cd57567a9c78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant