hotmeteor / spectator

OpenAPI testing for PHP
MIT License
285 stars 53 forks source link

Dictionary objects break validation #167

Closed docdauck closed 9 months ago

docdauck commented 9 months ago

Overview

When the schema contains a dictionary or free-form object the validation does not work. Any response/request to a valid path with a valid status code passes tests independent of any violations of the provided openapi schema.

Free-form objects or map/dictionary like objects are valid openapi: https://spec.openapis.org/oas/latest.html#model-with-map-dictionary-properties

The desired outcome would be for the schema validation to work as intended but almost more importantly to not have tests that should have failed pass only due to an internal unhandled exception. Any such exception should probably cause the tests to fail to alert the developer of an issue

Example

Consider the following schema

openapi: 3.1.0
info:
  title: FreeFormObject
  version: '1.0'
servers:
  - url: 'http://localhost:3000'
paths:
  /api/test:
    get:
      summary: Test endpoint with free form object response
      tags: [ ]
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                type: object
                additionalProperties:
                  type: string

with the following route defined in Laravel

Route::get('/api/test', static function () {
    return response('string response', 200, ['content-type' => 'application/json']);
});

then the following PestPHP test passes

it('fails appropriately', function () {
    Spectator::using('FreeFormObject.yaml');

    $response = $this->getJson('api/test');

    // response is: "string response"
    $response->dump();

    // have a single error: "Undefined array key "properties"
    dump($response->collectExceptionMessages());

    // but both these assertions pass
    $response->assertValidRequest()->assertValidResponse(200);
});

with the following output

image

Clearly a string response does not comply with the openapi spec and the assertValidResponse(200) should have failed.

Analysis

On the following lines of code https://github.com/hotmeteor/spectator/blob/9ee692b34f619ebf0ea168b468abc2a06d59eb11/src/Exceptions/SchemaValidationException.php#L249-L256 an assumption is made that every schema of type object has a properties key. This causes an exception which wipes the genuine validation errors, but is ignored by the assertValidResponse() assertion.

bastien-phi commented 9 months ago

Thanks for the report. Currently working on it

docdauck commented 9 months ago

Thanks for the quick response!

I also found something similar when an array contains a anyOf or similar as its items, i.e.

type: array
items:
    anyOf:
        - ...

This throws the error Undefined property: stdClass::$properties and seems to be due to the filterProperty() method being called when the type is array in

https://github.com/hotmeteor/spectator/blob/9ee692b34f619ebf0ea168b468abc2a06d59eb11/src/Validation/AbstractValidator.php#L108-L130