From edc2248df91a2d61fa7a86f0e6d28fe38b1bab14 Mon Sep 17 00:00:00 2001 From: Jarrod Parkes Date: Fri, 10 Dec 2021 13:34:24 -0500 Subject: [PATCH] Query parameters are not being checked by spec tests (#79) --- src/Middleware.php | 26 ++++++++++++++---- src/Validation/RequestValidator.php | 42 ++++++++++++++++++++--------- tests/Fixtures/Test.v1.json | 4 +-- tests/RequestValidatorTest.php | 19 +++++++++++++ 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/src/Middleware.php b/src/Middleware.php index 2baf45b..aee6ff2 100644 --- a/src/Middleware.php +++ b/src/Middleware.php @@ -2,8 +2,10 @@ namespace Spectator; +use cebe\openapi\exceptions\IOException; use cebe\openapi\exceptions\TypeErrorException; use cebe\openapi\exceptions\UnresolvableReferenceException; +use cebe\openapi\json\InvalidJsonPointerSyntaxException; use cebe\openapi\spec\PathItem; use Closure; use Illuminate\Contracts\Debug\ExceptionHandler; @@ -99,7 +101,12 @@ protected function formatResponse($exception, $code): JsonResponse * @return mixed * * @throws InvalidPathException - * @throws MissingSpecException|RequestValidationException|ResponseValidationException + * @throws MissingSpecException + * @throws RequestValidationException + * @throws TypeErrorException + * @throws UnresolvableReferenceException + * @throws IOException + * @throws InvalidJsonPointerSyntaxException */ protected function validate(Request $request, Closure $next) { @@ -107,11 +114,20 @@ protected function validate(Request $request, Closure $next) $pathItem = $this->pathItem($request_path, $request->method()); - RequestValidator::validate($request, $pathItem, $request->method()); + RequestValidator::validate( + $request, + $pathItem, + $request->method() + ); $response = $next($request); - ResponseValidator::validate($request_path, $response, $pathItem->{strtolower($request->method())}, $this->version); + ResponseValidator::validate( + $request_path, + $response, + $pathItem->{strtolower($request->method())}, + $this->version + ); $this->spectator->reset(); @@ -127,8 +143,8 @@ protected function validate(Request $request, Closure $next) * @throws MissingSpecException * @throws TypeErrorException * @throws UnresolvableReferenceException - * @throws \cebe\openapi\exceptions\IOException - * @throws \cebe\openapi\json\InvalidJsonPointerSyntaxException + * @throws IOException + * @throws InvalidJsonPointerSyntaxException */ protected function pathItem($request_path, $request_method): PathItem { diff --git a/src/Validation/RequestValidator.php b/src/Validation/RequestValidator.php index 2a3e609..14ee511 100644 --- a/src/Validation/RequestValidator.php +++ b/src/Validation/RequestValidator.php @@ -8,6 +8,7 @@ use Opis\JsonSchema\ValidationResult; use Opis\JsonSchema\Validator; use Spectator\Exceptions\RequestValidationException; +use Spectator\Exceptions\SchemaValidationException; class RequestValidator extends AbstractValidator { @@ -26,6 +27,11 @@ class RequestValidator extends AbstractValidator */ protected string $method; + /** + * @var array + */ + protected array $parameters; + /** * RequestValidator constructor. * @@ -45,11 +51,11 @@ public function __construct(Request $request, PathItem $pathItem, string $method /** * @param Request $request * @param PathItem $pathItem - * @param $method + * @param string $method * * @throws RequestValidationException */ - public static function validate(Request $request, PathItem $pathItem, $method) + public static function validate(Request $request, PathItem $pathItem, string $method) { $instance = new self($request, $pathItem, $method); @@ -69,13 +75,17 @@ protected function handle() } /** - * @throws RequestValidationException + * @throws RequestValidationException|SchemaValidationException */ protected function validateParameters() { $route = $this->request->route(); - $parameters = $this->pathItem->parameters; + $parameters = array_merge( + $this->pathItem->parameters, + $this->operation()->parameters + ); + $required_parameters = array_filter($parameters, fn ($parameter) => $parameter->required === true); foreach ($required_parameters as $parameter) { @@ -89,12 +99,15 @@ protected function validateParameters() } elseif ($parameter->in === 'cookie' && ! $this->request->cookies->has($parameter->name)) { throw new RequestValidationException("Missing required cookie [{$parameter->name}]."); } + } + foreach ($parameters as $parameter) { // Validate schemas, if provided. if ($parameter->schema) { $validator = new Validator(); $expected_parameter_schema = $parameter->schema->getSerializableData(); $result = null; + $actual_parameter = null; // Get parameter, then validate it. if ($parameter->in === 'path' && $route->hasParameter($parameter->name)) { @@ -106,21 +119,24 @@ protected function validateParameters() } elseif ($parameter->in === 'cookie' && $this->request->cookies->has($parameter->name)) { $actual_parameter = $this->request->cookies->get($parameter->name); } - $result = $validator->validate($actual_parameter, $expected_parameter_schema); - - // If the result is not valid, then display failure reason. - if ($result instanceof ValidationResult && $result->isValid() === false) { - $message = RequestValidationException::validationErrorMessage($expected_parameter_schema, $result->error()); - throw RequestValidationException::withError($message, $result->error()); - } elseif ($result->isValid() === false) { - throw RequestValidationException::withError("Parameter [{$parameter->name}] did not match provided JSON schema.", $result->error()); + + if ($actual_parameter) { + $result = $validator->validate($actual_parameter, $expected_parameter_schema); + + // If the result is not valid, then display failure reason. + if ($result instanceof ValidationResult && $result->isValid() === false) { + $message = RequestValidationException::validationErrorMessage($expected_parameter_schema, $result->error()); + throw RequestValidationException::withError($message, $result->error()); + } elseif ($result->isValid() === false) { + throw RequestValidationException::withError("Parameter [{$parameter->name}] did not match provided JSON schema.", $result->error()); + } } } } } /** - * @throws RequestValidationException + * @throws RequestValidationException|SchemaValidationException */ protected function validateBody(): void { diff --git a/tests/Fixtures/Test.v1.json b/tests/Fixtures/Test.v1.json index 8de8143..49567ce 100644 --- a/tests/Fixtures/Test.v1.json +++ b/tests/Fixtures/Test.v1.json @@ -61,6 +61,8 @@ "operationId": "get-users", "parameters": [ { + "in": "query", + "name": "order", "schema": { "type": "string", "enum": [ @@ -68,8 +70,6 @@ "email" ] }, - "in": "query", - "name": "order", "allowEmptyValue": true } ] diff --git a/tests/RequestValidatorTest.php b/tests/RequestValidatorTest.php index a8cf6cd..5d151e7 100644 --- a/tests/RequestValidatorTest.php +++ b/tests/RequestValidatorTest.php @@ -433,6 +433,25 @@ public function allOfSchemaProvider() ], ]; } + + public function test_handles_query_parameters() + { + Spectator::using('Test.v1.json'); + + // When testing query parameters, they are not found nor checked by RequestValidator->validateParameters(). + Route::get('/users', function () { + return []; + })->middleware(Middleware::class); + + $this->getJson('/users?order=invalid') + ->assertInvalidRequest() + ->assertErrorsContain([ + 'The data should match one item from enum', + ]); + + $this->getJson('/users?order=name') + ->assertValidRequest(); + } } class TestUser extends Model