Inconsistent body validation behavior

[setaman]

I find there are some inconsistencies in the validation of request bodies, especially with multipart data. First of all, empty body produces TypeError: validator.validate is not a function on validation, where specification requires a multipart body. Secondly the Content-Type attribute is not checked, I can send multipart body where e.g. a JSON body is expected.

I am not sure if this is the desired behavior. But from my point of view the existence of the body should be checked during validation if the specification requires a body in the request. Otherwise the consumer of the API will see a mysterious error message and will not be informed about the specification violation.

My setup is like follows:

Specification

    post:
      requestBody:
        required: true
        content:
          multipart/form-data:
            schema:
              type: object
              properties:
                image:
                  type: string
                  format: binary
                  ...

Route

router.post(
  "/users",
  multer().single("image"),
  validator.validate,
...
);

Making a request to this endpoint without a body produces an error:

TypeError: validator.validate is not a function
    at Middleware._validateBody (/home/serg/Documents/Projects/drm/services/user-management/node_modules/openapi-validator-middleware/src/middleware.js:92:28)
    at Middleware._validateRequest (/home/serg/Documents/Projects/drm/services/user-management/node_modules/openapi-validator-middleware/src/middleware.js:63:43)
    at /home/serg/Documents/Projects/drm/services/user-management/node_modules/openapi-validator-middleware/src/middleware.js:51:90
    at Middleware.validate [as validationMiddleware] (/home/serg/Documents/Projects/drm/services/user-management/node_modules/openapi-validator-middleware/src/frameworks/express.js:4:24)
    at Middleware.validate (/home/serg/Documents/Projects/drm/services/user-management/node_modules/openapi-validator-middleware/src/middleware.js:41:21)
    at Layer.handle [as handle_request] (/home/serg/Documents/Projects/drm/services/user-management/node_modules/express/lib/router/layer.js:95:5)
    at next (/home/serg/Documents/Projects/drm/services/user-management/node_modules/express/lib/router/route.js:137:13)
    at multerMiddleware (/home/serg/Documents/Projects/drm/services/user-management/node_modules/multer/lib/make-middleware.js:18:41)
    at Layer.handle [as handle_request] (/home/serg/Documents/Projects/drm/services/user-management/node_modules/express/lib/router/layer.js:95:5)
    at next (/home/serg/Documents/Projects/drm/services/user-management/node_modules/express/lib/router/route.js:137:13)

But I expect to see a validation error here. I assume that this is a bug, because a application/json body does not cause this error. Although the validation of a missing (but required in spec) raw body correctly indicates that the required fields are not present, but it would be more correct to report that a body itself is missing in the request.

So please let me know if my expectations are legitimate or if I have misunderstood the world.

[kobik]

Hi @setaman , this kind f error message isn't something that should be returned in any circumstances, so it's a bug indeed.

I'm not sure about how this package is handling multipart requests and would require investing some time on it.

[setaman]

Hi @kobik, It is getting even worse. I just found out that the validation of the multipart/form-data body fails, if i set image to be required. The root of all evil is the _validateBody method.

My first issue, where TypeError: validator.validate is not a function is thrown, is caused by:

...
// is undefined, no body --> no Content-Type header
const contentType = this._getContentType(requestOptions.headers);
...
// the validator is there, but we cannot reach it because of the undefined 'contentType'
// the validator is in methodSchema.body.multipart/form-data 
const validator = methodSchema.body[contentType] || methodSchema.body;

I put the validation right after multer in the chain. Multer assigns the image to req.file, so the validation of the body do not pass:

const validator = methodSchema.body[contentType] || methodSchema.body;
// body has no image field
// methodSchema.body.multipart/form-data has no image to
if (!validator.validate(body)) {
  return validator.errors || [];
 }

I really want this library to work correctly and would like to help. But there are inconsistencies in the validation strategies in general. We see that in the described cases the headers are not checked, which in turn leads to the fact that Body cannot be validated. The question is whether you, the creators and maintainers, want to see the validation of the presence of the headers and the body and the correctness of the headers according to the specification as part of the functionality of this library.

[kobik]

Hi @setaman, indeed we'd like this package to be as consistent and validate correctly according to the specification.

You're more than welcome to submit a PR.

[setaman]

@kobik I will try to find some time between my work and studies to look at the issues

[kobik]

Thanks @setaman, much appreciated.