ajv-validator / ajv-validator/ajv
required/properties/dependencies treat Object.prototype property names (toString, constructor, __proto__) as present in the data
- Lingua principale
- TypeScript
- Stelle
- 14.8k
- Fork
- 1k
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Descrizione
**What version of Ajv are you using? Does the issue happen if you use the latest version?**
`ajv@8.20.0` (latest). Also reproduces on `master` at `f177fe32`.
**Ajv options object**
```javascript
const options = {} // defaults
```
**JSON Schema**
```json
{"type": "object", "required": ["toString"]}
```
**Sample data**
```json
{}
```
**Your code**
```javascript
const Ajv = require("ajv")
const ajv = new Ajv(options)
console.log(ajv.validate(schema, data))
```
**Validation result, data AFTER validation, error messages**
```
true
```
Data is unchanged. No errors.
**What results did you expect?**
`false`. `{}` does not have a `toString` property — `Object.keys({})`, `for (const k in {})` and `JSON.stringify({})` all agree that it has no properties at all.
The cause is that `required` (and `properties` and `dependencies`) detect presence with `data[prop] !== undefined`, which is true for every object inheriting from `Object.prototype` when the property name is one of `Object.prototype`'s own members (`toString`, `constructor`, `valueOf`, `hasOwnProperty`, `__proto__`, ...).
Three more cases of the same defect:
```javascript
// 2. valid data rejected
ajv.validate({type: "object", properties: {constructor: {type: "number"}}}, {})
// -> false
// errors: [{"instancePath":"/constructor","schemaPath":"#/properties/constructor/type",
// "keyword":"type","params":{"type":"number"},"message":"must be number"}]
// The `constructor` subschema is applied to the inherited `Object` function.
// 3. invalid data accepted
ajv.validate({type: "object", required: ["__proto__"]}, {})
// -> true
// 4. dependencies triggered by an absent property
ajv.validate({type: "object", dependencies: {toString: ["x"]}}, {})
// -> false
```
Why I think this is a bug rather than intended behaviour:
1. **`required` accepts data that is missing a required property.** This is the direction that has not been discussed before (see prior art below) and it is a false negative from the one keyword whose entire job is to reject such data.
2. **Ajv's keywords disagree with each other about what "present" means.** `additionalProperties` uses `for...in`, so with `{"additionalProperties": false}` Ajv correctly treats `toString` as absent from `{}` — while `required` treats it as present in the very same object.
3. **It contradicts the documented semantics of `ownProperties`.** [options.md](https://ajv.js.org/options.html#ownproperties) says *"By default Ajv iterates over all **enumerable** object properties"*. `Object.prototype`'s members are not enumerable.
4. **The JSON-Schema-Test-Suite forbids it, in non-optional tests.** `tests//required.json` and `tests//properties.json` both have a group `"...properties whose names are Javascript object property names"` with the comment *"Ensure JS implementations don't universally consider e.g. `__proto__` to always be present in an object."* Ajv fails 4 of the `required.json` cases on draft-06, draft-07, draft-2019-09 and draft-2020-12. On draft-07 those 4 are, apart from 3 pre-existing `$ref` failures, the only non-optional failures left.
Ajv's own CI does not see these groups because `spec/JSON-Schema-Test-Suite` is pinned at a Nov 2021 commit that predates them.
**Prior art, and the reason I am opening this anyway**
#1045 ("Can't validate json if it has `constructor` property", 2019, v6) is exactly case 2 above. It was closed with *"see option `ownProperties`"*, and #197 shows `ownProperties` was designed deliberately to cover `properties`/`required`/`dependencies`. So this is a re-opening of ground that was already ruled on, and I want to be upfront about that rather than pretend otherwise.
What I think has changed since that ruling:
- the official test suite now has non-optional tests forbidding this on every draft (they did not exist in 2019);
- case 1 (`required` accepting data missing the property) was never raised in #1045;
- `ownProperties: true` is a wider change than the problem requires — it also stops inherited *enumerable* properties from validating, which is the behaviour the option exists to control. A user who wants inherited enumerable properties to validate has no way to also get correct `required` handling of `toString`.
**Are you going to resolve the issue?**
Yes — I have a fix ready and will open a PR referencing this issue.
The approach: a small runtime helper that treats a property as present if it is an own property, or an inherited *enumerable* property. It is only consulted for the ~12 names in `Object.getOwnPropertyNames(Object.prototype)`; every other property name keeps the current `!== undefined` test, and the helper itself short-circuits on non-`Object.prototype` names so that the `$data` and `loopRequired` code paths (where the name is not known at compile time) behave identically. `ownProperties: true` is untouched.
Generated code for schemas that do not mention one of those names is byte-for-byte unchanged (verified over 400 random schemas plus the pinned draft-07 suite), and the full test suite passes on all 16 option combinations plus standalone.
If you would rather not change the default behaviour, please say so and I will close the PR — I would rather ask than not raise it. A narrower variant is also possible: fix only `required`/`dependencies` (the false-negative direction and all 4 suite failures) and leave `properties` exactly as #1045 ruled.
Guida per i contributori
Apri la guida per i contributori
Valutazione
Questa issue non è ancora stata valutata.