Skip to content

Commit d0ccb50

Browse files
RomanHotsiyJLekawa
andauthored
fix: fixed the wrong validation of constructor property (#2126)
* fix: fixed the wrong validation of `constructor` property * chore: add e2e test * chore: use getOwn check instead of Object.create(null * chore: fix prettier * Update .changeset/thick-planets-fail.md --------- Co-authored-by: Jacek Łękawa <[email protected]>
1 parent 6a20b93 commit d0ccb50

File tree

10 files changed

+72
-8
lines changed

10 files changed

+72
-8
lines changed

.changeset/thick-planets-fail.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@redocly/openapi-core": patch
3+
---
4+
5+
Fixed incorrect validation logic for the `constructor` property.

__tests__/commands.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ describe('E2E', () => {
7474
const result = getCommandOutput(args, {}, { testPath });
7575
await expect(cleanupOutput(result)).toMatchFileSnapshot(join(testPath, 'snapshot_2.txt'));
7676
});
77+
78+
test('lint file with constructor property in schema', async () => {
79+
const testPath = join(__dirname, 'fixtures/constructor-property');
80+
const args = getParams(indexEntryPoint, ['lint', 'openapi.yaml']);
81+
82+
const result = getCommandOutput(args, {}, { testPath });
83+
await expect(cleanupOutput(result)).toMatchFileSnapshot(join(testPath, 'snapshot.txt'));
84+
});
7785
});
7886

7987
describe('zero-config', () => {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
openapi: 3.0.0
2+
servers:
3+
- url: http://test.com:3000
4+
info:
5+
title: Test API - Constructor Property
6+
version: 1.0.0
7+
license:
8+
name: MIT
9+
url: https://opensource.org/licenses/MIT
10+
components:
11+
schemas:
12+
SchemaWithConstructor:
13+
type: object
14+
properties:
15+
id:
16+
type: string
17+
constructor: # This is the property we are testing
18+
type: object
19+
description: 'A property named constructor'
20+
properties:
21+
foo:
22+
type: string
23+
paths:
24+
/test:
25+
get:
26+
summary: Test endpoint
27+
security: []
28+
operationId: test
29+
responses:
30+
'200':
31+
description: OK
32+
content:
33+
application/json:
34+
schema:
35+
$ref: '#/components/schemas/SchemaWithConstructor'
36+
'400':
37+
description: Bad Request
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
2+
No configurations were provided -- using built in recommended configuration by default.
3+
4+
validating openapi.yaml...
5+
openapi.yaml: validated in <test>ms
6+
7+
Woohoo! Your API description is valid. 🎉
8+

packages/core/src/resolve.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
isExternalValue,
1111
} from './ref-utils.js';
1212
import { isNamedType, SpecExtension } from './types/index.js';
13-
import { readFileFromUrl, parseYaml, nextTick } from './utils.js';
13+
import { readFileFromUrl, parseYaml, nextTick, getOwn } from './utils.js';
1414

1515
import type { YAMLNode, LoadOptions } from 'yaml-ast-parser';
1616
import type { NormalizedNodeType } from './types/index.js';
@@ -299,7 +299,7 @@ export async function resolveDocument(opts: {
299299

300300
for (const propName of Object.keys(node)) {
301301
let propValue = node[propName];
302-
let propType = type.properties[propName];
302+
let propType = getOwn(type.properties, propName);
303303
if (propType === undefined) propType = type.additionalProperties;
304304
if (typeof propType === 'function') propType = propType(propValue, propName);
305305
if (propType === undefined) propType = unknownType;

packages/core/src/rules/common/no-required-schema-properties-undefined.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isRef } from '../../ref-utils.js';
2+
import { getOwn } from '../../utils.js';
23

34
import type { Oas2Rule, Oas3Rule } from '../../visitors.js';
45
import type { Oas3Schema, Oas3_1Schema } from '../../typings/openapi.js';
@@ -41,7 +42,7 @@ export const NoRequiredSchemaPropertiesUndefined: Oas3Rule | Oas2Rule = () => {
4142
const allProperties = elevateProperties(schema);
4243

4344
for (const [i, requiredProperty] of schema.required.entries()) {
44-
if (!allProperties || allProperties[requiredProperty] === undefined) {
45+
if (!allProperties || getOwn(allProperties, requiredProperty) === undefined) {
4546
report({
4647
message: `Required property '${requiredProperty}' is undefined.`,
4748
location: location.child(['required', i]),

packages/core/src/rules/common/scalar-property-missing-example.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { SpecVersion } from '../../oas-types.js';
2+
import { getOwn } from '../../utils.js';
23

34
import type { Oas2Rule, Oas3Rule } from '../../visitors.js';
45
import type { UserContext } from '../../walk.js';
@@ -14,7 +15,7 @@ export const ScalarPropertyMissingExample: Oas3Rule | Oas2Rule = () => {
1415
{ report, location, oasVersion, resolve }: UserContext
1516
) {
1617
for (const propName of Object.keys(properties)) {
17-
const propSchema = resolve(properties[propName]).node;
18+
const propSchema = resolve(getOwn(properties, propName)).node;
1819

1920
if (!propSchema || !isScalarSchema(propSchema)) {
2021
continue;

packages/core/src/rules/common/struct.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { isNamedType, SpecExtension } from '../../types/index.js';
22
import { oasTypeOf, matchesJsonSchemaType, getSuggest, validateSchemaEnumType } from '../utils.js';
33
import { isRef } from '../../ref-utils.js';
4-
import { isPlainObject } from '../../utils.js';
4+
import { getOwn, isPlainObject } from '../../utils.js';
55

66
import type { UserContext } from '../../walk.js';
77
import type {
@@ -102,7 +102,7 @@ export const Struct:
102102
const propLocation = location.child([propName]);
103103
let propValue = node[propName];
104104

105-
let propType = type.properties[propName];
105+
let propType = getOwn(type.properties, propName);
106106
if (propType === undefined) propType = type.additionalProperties;
107107
if (typeof propType === 'function') propType = propType(propValue, propName);
108108

packages/core/src/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ export function dequal(foo: any, bar: any): boolean {
296296
return foo !== foo && bar !== bar;
297297
}
298298

299+
export function getOwn(obj: Record<string, any>, key: string) {
300+
return obj.hasOwnProperty(key) ? obj[key] : undefined;
301+
}
302+
299303
export type CollectFn = (value: unknown) => void;
300304

301305
export type StrictObject<T extends object> = T & { [key: string]: undefined };

packages/core/src/walk.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Location, isRef } from './ref-utils.js';
2-
import { pushStack, popStack } from './utils.js';
2+
import { pushStack, popStack, getOwn } from './utils.js';
33
import { YamlParseError, makeRefId } from './resolve.js';
44
import { isNamedType, SpecExtension } from './types/index.js';
55

@@ -308,7 +308,7 @@ export function walkDocument<T extends BaseVisitor>(opts: {
308308
loc = location; // properties on the same level as $ref should resolve against original location, not target
309309
}
310310

311-
let propType = type.properties[propName];
311+
let propType = getOwn(type.properties, propName);
312312
if (propType === undefined) propType = type.additionalProperties;
313313
if (typeof propType === 'function') propType = propType(value, propName);
314314

0 commit comments

Comments
 (0)