ExodusMovement / schemasafe

A reasonably safe JSON Schema validator with draft-04/06/07/2019-09/2020-12 support.
https://npmjs.com/@exodus/schemasafe
MIT License
161 stars 12 forks source link

Using oneOf for object type results in always-failing condition #64

Closed kklash closed 4 years ago

kklash commented 4 years ago

When you try to use only oneOf to describe an object type in strong mode (+ additionalItems = false), it ignores the oneOf array and always fails to validate any data.

Example:

const schema = {
  "type": "object",
  "additionalProperties": false,
  "oneOf": [
    {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "a": {
          "type": "boolean",
          "const": true
        }
      }
    },
    {
      "type": "object",
      "additionalProperties": false,
      "properties": {
        "b": {
          "type": "boolean",
          "const": false
        }
      }
    }
  ]
}
const { validator } = require('.')

const validate = validator(schema, { mode: 'strong', includeErrors: true })
validate({ a: true }) // false
validate({ b: false }) // false
console.log(validate.errors)
// [
//   {
//     field: 'data',
//     message: 'has additional properties',
//     schemaPath: '#'
//   }
// ]

Examining the IIFE code, we see the problem is caused by oneOf being ignored in favor of properties or patternProperties:

(function() {
'use strict'
const hasOwn = Function.prototype.call.bind(Object.prototype.hasOwnProperty);
return (function validate(data) {
  if (data === undefined) data = null
  validate.errors = null
  let errors = 0
  if (!(typeof data === "object" && data && !Array.isArray(data))) {
    validate.errors = [{"field":"data","message":"is the wrong type","schemaPath":"#"}]
    return false
    return false
  }
  for (const key0 of Object.keys(data)) {
    if (true) { // <-------- PROBLEM
      validate.errors = [{"field":"data","message":"has additional properties","schemaPath":"#"}]
      return false
      return false
    }
  }
...
ChALkeR commented 4 years ago

That's the correct per-spec behavior, I believe. Top-level additionalProperties rule is checked against empty/missing properties in the top-level schema, not against sub-schemas in oneOf.

There doesn't seem to be any related issues in the produced IIFE. Also, this matches ajv.

I.e. this is exactly how additionalProperties and unevaluatedProperties are different, but the latter is introduced only in draft2019-09 spec which we don't support here. Refs: https://github.com/json-schema-org/json-schema-spec/issues/556 additionalProperties, unlike unevaluatedProperties (unsupported), doesn't see through refs and allOf/anyOf/oneOf.

So, this is an issue, but it's an issue of enforceValidation, not of code generation. It shouldn't enforce to set additionalProperties when it has already been set in all anyOf/oneOf/allOf clauses.

I'll fix that.