Skip to content

Fix: validate() returns true with wrong top level elements #231

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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/SpecBaseObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use cebe\openapi\json\JsonPointer;
use cebe\openapi\json\JsonReference;
use cebe\openapi\spec\Reference;
use cebe\openapi\spec\Schema;
use cebe\openapi\spec\Type;

/**
Expand Down Expand Up @@ -234,7 +235,15 @@ public function validate(): bool
}
$this->_recursingValidate = true;
$valid = true;
foreach ($this->_properties as $v) {
$allowedFields = array_keys($this->attributes());
if (static::class === Schema::class) {
$allowedFields[] = 'additionalProperties';
}
foreach ($this->_properties as $k => $v) {
if ($allowedFields && !in_array($k, $allowedFields, true) && substr($k, 0, 2) !== 'x-') {
$valid = false;
$this->addError('Invalid field: "' . $k . '"');
}
if ($v instanceof SpecObjectInterface) {
if (!$v->validate()) {
$valid = false;
Expand Down Expand Up @@ -275,7 +284,7 @@ public function getErrors(): array
if (($pos = $this->getDocumentPosition()) !== null) {
$errors = [
array_map(function ($e) use ($pos) {
return "[{$pos->getPointer()}] $e";
return $pos->getPointer() ? "[{$pos->getPointer()}] $e" : $e;
}, $this->_errors)
];
} else {
Expand Down
13 changes: 9 additions & 4 deletions tests/ReaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ public function testReadYamlWithAnchors()
$openApiFile = __DIR__ . '/spec/data/traits-mixins.yaml';
$openapi = \cebe\openapi\Reader::readFromYamlFile($openApiFile);

$this->assertApiContent($openapi);
$this->assertApiContent($openapi, [
'[/paths/~1foo/put/responses/200] Invalid field: "schema"',
'[/paths/~1foo/put/responses/404] Invalid field: "schema"',
'[/paths/~1foo/put/responses/428] Invalid field: "schema"',
'[/paths/~1foo/put/responses/default] Invalid field: "schema"'
]);

$putOperation = $openapi->paths['/foo']->put;
$this->assertEquals('create foo', $putOperation->description);
Expand Down Expand Up @@ -77,11 +82,11 @@ public function testReadYamlWithAnchors()
$this->assertEquals('uuid of the resource', $foo->properties['uuid']->description);
}

private function assertApiContent(\cebe\openapi\spec\OpenApi $openapi)
private function assertApiContent(\cebe\openapi\spec\OpenApi $openapi, $expected = [])
{
$result = $openapi->validate();
$this->assertEquals([], $openapi->getErrors());
$this->assertTrue($result);
$this->assertEquals($expected, $openapi->getErrors());
$expected ? $this->assertFalse($result) : $this->assertTrue($result);


$this->assertEquals("3.0.0", $openapi->openapi);
Expand Down
23 changes: 23 additions & 0 deletions tests/spec/OpenApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,4 +233,27 @@ public function testSpecs($openApiFile)
}

}

public function testInvalidTopLevelField()
{
$openapi = new OpenApi([
'AAAAAcomponents' => [
'User' => [
'type' => 'object',
'properties' => [
'id' => [
'type' => 'integer'
],
]
]
]
]);
$this->assertFalse($openapi->validate());
$this->assertEquals([
'Invalid field: "AAAAAcomponents"',
'OpenApi is missing required property: openapi',
'OpenApi is missing required property: info',
'OpenApi is missing required property: paths',
], $openapi->getErrors());
}
}
14 changes: 10 additions & 4 deletions tests/spec/PathTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,11 @@ public function testPathItemReference()
$openapi = Reader::readFromYamlFile($file, \cebe\openapi\spec\OpenApi::class, false);

$result = $openapi->validate();
$this->assertEquals([], $openapi->getErrors(), print_r($openapi->getErrors(), true));
$this->assertTrue($result);
$this->assertEquals([
'Invalid field: "X-EXTENSION"',
'Invalid field: "xyz-extension"'
], $openapi->getErrors(), print_r($openapi->getErrors(), true));
$this->assertFalse($result);

$this->assertInstanceOf(Paths::class, $openapi->paths);
$this->assertInstanceOf(PathItem::class, $fooPath = $openapi->paths['/foo']);
Expand Down Expand Up @@ -177,8 +180,11 @@ public function testPathItemReference()
$openapi = Reader::readFromYamlFile($file, \cebe\openapi\spec\OpenApi::class, true);

$result = $openapi->validate();
$this->assertEquals([], $openapi->getErrors(), print_r($openapi->getErrors(), true));
$this->assertTrue($result);
$this->assertEquals([
'Invalid field: "X-EXTENSION"',
'Invalid field: "xyz-extension"'
], $openapi->getErrors());
$this->assertFalse($result);

$this->assertInstanceOf(Paths::class, $openapi->paths);
$this->assertInstanceOf(PathItem::class, $fooPath = $openapi->paths['/foo']);
Expand Down
1 change: 1 addition & 0 deletions tests/spec/SchemaTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public function testDiscriminator()

$result = $schema->validate();
$this->assertEquals([
'Invalid field: "map"',
'Discriminator is missing required property: propertyName'
], $schema->getErrors());
$this->assertFalse($result);
Expand Down