fix: respect the required parameter in the body (#450)

* feat: kill the validator registry concept for body * test: kill tests that aren't needed anymore * test: kill snapshot * test: keep the test that make sense * test: use a verb that does not exist for real * test: fix the documents * test: add harness test 1 * test: harness part 2 * test: add shell:true flag * fix: put better error message when stuff does not match * test: adjust responses * chore(deps): [security] bump lodash.template from 4.4.0 to 4.5.0 (#447) Bumps [lodash.template](https://github.com/lodash/lodash) from 4.4.0 to 4.5.0. **This update includes security fixes.** - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.4.0...4.5.0) Signed-off-by: dependabot-preview[bot] <[email protected]> * chore(deps): [security] bump lodash from 4.17.11 to 4.17.13 (#448) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.11 to 4.17.13. **This update includes security fixes.** - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.11...4.17.13) Signed-off-by: dependabot-preview[bot] <[email protected]> * use bash for harness on windows * test: no harness on windows * test: add failing test * docs: precise yet another limitation * fix: add validation error if body is required but not present * feat: validate only when necessary * test:fix * chore: cleanup * test: make test behave as they should * chore: only post has body * test:fix * test: do not repeat the test stuff and generalise
stoplightio · Jul 12, 2019 · 8329ce7 · 8329ce7
1 parent e85bb18
commit 8329ce7
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 23 deletions.
diff --git a/packages/core/src/factory.ts b/packages/core/src/factory.ts
@@ -1,6 +1,5 @@
 import { DiagnosticSeverity } from '@stoplight/types';
-import { toError } from 'fp-ts/lib/Either';
-import { fromEither, left2v, right2v, tryCatch } from 'fp-ts/lib/TaskEither';
+import { fromEither, left2v, right2v } from 'fp-ts/lib/TaskEither';
 import { configMergerFactory, PartialPrismConfig, PrismConfig } from '.';
 import { IPrism, IPrismComponents, IPrismConfig, IPrismDiagnostic, PickRequired, ProblemJsonError } from './types';
 

diff --git a/packages/http-server/src/__tests__/fixtures/getOperationWithBody.oas2.json b/packages/http-server/src/__tests__/fixtures/getOperationWithBody.oas2.json
@@ -2,7 +2,7 @@
   "swagger": "2.0",
   "paths": {
     "/pet": {
-      "get": {
+      "post": {
         "produces": ["application/json"],
         "consumes": ["application/json"],
         "parameters": [

diff --git a/packages/http-server/src/__tests__/server.oas.spec.ts b/packages/http-server/src/__tests__/server.oas.spec.ts
@@ -62,12 +62,12 @@ describe('GET /pet?__server', () => {
   }
 });
 
-describe('GET /pet with invalid body', () => {
+describe('POST /pet with invalid body', () => {
   it('returns correct error message', async () => {
     const server = await instantiatePrism(resolve(__dirname, 'fixtures', 'getOperationWithBody.oas2.json'));
 
     const response = await server.fastify.inject({
-      method: 'GET',
+      method: 'POST',
       url: '/pet',
       payload: {
         id: 'strings are not valid!',
@@ -78,7 +78,7 @@ describe('GET /pet with invalid body', () => {
     const parsed = JSON.parse(response.payload);
     expect(parsed).toMatchObject({
       type: 'https://stoplight.io/prism/errors#UNPROCESSABLE_ENTITY',
-      validation: [{ location: ['body'], severity: 'Error', code: 'type', message: 'should be object' }],
+      validation: [{ location: ['body', 'id'], severity: 'Error', code: 'type', message: 'should be integer' }],
     });
     await server.fastify.close();
   });

diff --git a/packages/http/src/validator/__tests__/index.spec.ts b/packages/http/src/validator/__tests__/index.spec.ts
@@ -33,7 +33,7 @@ describe('HttpValidator', () => {
 
   describe('validateInput()', () => {
     describe('body validation in enabled', () => {
-      const test = (extendResource?: Partial<IHttpOperation>) => () => {
+      const test = (extendResource: Partial<IHttpOperation> | undefined, expectedError: IPrismDiagnostic[]) => () => {
         jest
           .spyOn(resolveValidationConfigModule, 'resolveRequestValidationConfig')
           .mockReturnValueOnce({ body: true, headers: false, hijack: false, query: false });
@@ -53,30 +53,54 @@ describe('HttpValidator', () => {
             input: { method: 'get', url: { path: '/' } },
             config: { mock: { dynamic: false }, validate: { request: { body: true } } },
           }),
-        ).toEqual([mockError]);
+        ).toEqual(expectedError);
 
         expect(resolveValidationConfigModule.resolveRequestValidationConfig).toHaveBeenCalled();
-        expect(httpBodyValidator.validate).toHaveBeenCalledWith(undefined, [], undefined);
+        expect(httpBodyValidator.validate).not.toHaveBeenCalled();
         expect(httpHeadersValidator.validate).not.toHaveBeenCalled();
         expect(httpQueryValidator.validate).not.toHaveBeenCalled();
       };
 
       describe('request is not set', () => {
-        it('validates body', test());
+        it('does not try to validate the body', test(undefined, []));
       });
 
       describe('request is set', () => {
         describe('request.body is not set', () => {
-          it('validates body', test({ request: { path: [], headers: [], query: [], cookie: [] } }));
+          it(
+            'does not try to validate the body',
+            test({ request: { path: [], headers: [], query: [], cookie: [] } }, []),
+          );
         });
 
         describe('request.body is set', () => {
-          it(
-            'validates body',
-            test({
-              request: { body: { contents: [] }, path: [], query: [], headers: [], cookie: [] },
-            }),
-          );
+          describe('request body is not required', () => {
+            it(
+              'does not try to validate the body',
+              test(
+                {
+                  request: { body: { contents: [] }, path: [], query: [], headers: [], cookie: [] },
+                },
+                [],
+              ),
+            );
+          });
+
+          describe('request body is required', () => {
+            it(
+              'tries to validate the body',
+              test(
+                {
+                  method: 'get',
+                  path: '/',
+                  id: '1',
+                  request: { body: { contents: [], required: true } },
+                  responses: [{ code: '200' }],
+                },
+                [{ message: 'Body parameter is required', code: 'required', severity: DiagnosticSeverity.Error }],
+              ),
+            );
+          });
         });
       });
     });
@@ -196,7 +220,7 @@ describe('HttpValidator', () => {
 
     describe('output is set', () => {
       describe('body validation is enabled', () => {
-        it('validates body', async () => {
+        it('validates the body', async () => {
           jest
             .spyOn(resolveValidationConfigModule, 'resolveResponseValidationConfig')
             .mockReturnValueOnce({ headers: false, body: true });

diff --git a/packages/http/src/validator/index.ts b/packages/http/src/validator/index.ts
@@ -1,5 +1,5 @@
 import { IPrismDiagnostic, IValidator } from '@stoplight/prism-core';
-import { IHttpContent, IHttpHeaderParam, IHttpOperation, IHttpQueryParam } from '@stoplight/types';
+import { DiagnosticSeverity, IHttpContent, IHttpHeaderParam, IHttpOperation, IHttpQueryParam } from '@stoplight/types';
 import * as caseless from 'caseless';
 
 import { IHttpConfig, IHttpNameValue, IHttpNameValues, IHttpRequest, IHttpResponse } from '../types';
@@ -33,10 +33,15 @@ export class HttpValidator implements IValidator<IHttpOperation, IHttpRequest, I
 
     if (config.body) {
       const { body } = input;
-
-      this.bodyValidator
-        .validate(body, (request && request.body && request.body.contents) || [], mediaType)
-        .forEach(validationResult => results.push(validationResult));
+      if (request && request.body) {
+        if (!body && request.body.required) {
+          results.push({ code: 'required', message: 'Body parameter is required', severity: DiagnosticSeverity.Error });
+        } else if (body) {
+          this.bodyValidator
+            .validate(body, (request && request.body && request.body.contents) || [], mediaType)
+            .forEach(validationResult => results.push(validationResult));
+        }
+      }
     }
 
     if (config.headers) {

diff --git a/test-harness/README.md b/test-harness/README.md
@@ -64,6 +64,7 @@ Connection: keep-alive
 * The 4 keywords are `test,spec,server,command,expect,expect-loose`, nothing else at the moment
 * You can run all the tests on the same port `4010`, but you can also choose another one
 * The `curl` command does not support piping stuff into other tools; so if you're trying to be cool and do `curl | grep`, well maybe next time.
+* All the `curl` commands **must** have the `-i` flag, otherwise the trace parser won't understand the output
 
 ## Technical details
 

diff --git a/test-harness/specs/body-required-parameter.oas2.txt b/test-harness/specs/body-required-parameter.oas2.txt
@@ -0,0 +1,39 @@
+====test====
+When I send a request to an operation that has a body parameter
+and such body parameter is not required
+I shuold get 200 response
+====spec====
+swagger: '2.0'
+paths:
+  /pet:
+    post:
+      produces:
+      - application/json
+      consumes:
+      - application/json
+      parameters:
+      - in: body
+        name: body
+        required: false
+        schema:
+          type: object
+          properties:
+            id:
+              type: integer
+              format: int64
+      responses:
+        '200':
+          schema:
+            type: string
+====server====
+mock -p 4010
+====command====
+curl -i -X POST http://127.0.0.1:4010/pet
+====expect====
+HTTP/1.1 200 OK
+content-type: application/json; charset=utf-8
+content-length: 7
+Date: Fri, 21 Jun 2019 09:25:34 GMT
+Connection: keep-alive
+
+"string"