express-openapi-validator icon indicating copy to clipboard operation
express-openapi-validator copied to clipboard

Response validation - Serialized value replacement fails when response object is frozen (`x-eov-res-serdes`)

Open Jackman3005 opened this issue 2 years ago • 3 comments

Describe the bug

When providing a serializer for a format, you often take an object and serialize to a string. For example, if I provide the following serDes configuration

serDes: [
      {
        format: "date-time",
        serialize: (o) => {
          if (o instanceof Date) {
            return o.toISOString();
          }
          return "";
        },
      },
    ],

When calling res.json(myObj) where myObj has been frozen like with Object.freeze the AJV keyword 'x-eov-res-serdes' fails to perform the following line of code: ctx.parentData[ctx.parentDataProperty] = sch.serialize(data);.

To Reproduce

Set up an OpenAPI spec for a response body that has a field like so:

myField:
  type: string
  format: date-time

From the appropriate endpoint provide a response like so:

res.json(Object.freeze({myField: new Date()}));

Actual behavior

The hardest part about this entire bug is that it has taken me months to figure out what the problem is. I have played with every option of serDes, formats, etc. available and nothing ever seemed to work correctly because I would always get an error like the following:

  {
    path: '/response/58/xxx/startedAt',
    message: 'format is invalid',
    errorCode: 'serdes.openapi.validation'
  }

Which was NOT helpful enough for me to actually figure out the problem at hand!

I finally dove deep into the transpiled library code adding console logs at various key points and levels and found out that the raw error thrown is:

TypeError: Cannot redefine property: startedAt
    at Function.defineProperty (<anonymous>)
    at Ajv.validate (/Users/me/my-project/node_modules/express-openapi-validator/src/framework/ajv/index.ts:145:25)
    at validate10 (eval at compileSchema (/Users/me/my-project/node_modules/express-openapi-validator/node_modules/ajv/lib/compile/index.ts:171:26)

Expected behavior

  1. At minimum I hope to have better debugging tools to uncover this type of error. Maybe a debug log level that prints the raw exception message and stack? Or maybe just use the exception message instead or as a secondary message to 'format is invalid'
  2. I hope that we can provide frozen objects and that format serDes and validations will not be effected.

Examples and context

I'm using "express-openapi-validator": "^5.0.3"

INB4 "Why are you freezing your response objects? How about you try not doing that?"

~I am not freezing them in my code, it is a downstream library Prisma that is freezing all objects returned from calls to the DB, which honestly makes sense and is a push towards quality immutable programming.~ Update: I lied, Prisma does not do this, it appears there was an augmentation on a Prisma result that went through immer.

However, I don't see why I should have to perform deep clones on my response bodies just to support validation from this library.

~Additionally, I could not find out how/where I could inject a middleware or the like in order to do this deep clone before the open api validator code performs this step.~ Update: see my current workaround below

The relevant code in express-open-api that performs the serialization and exhibits the error and then hides the real useful error message is as follows:

if (options.serDesMap) {
    ajv.addKeyword({
        keyword: 'x-eov-res-serdes',
        modifying: true,
        errors: true,
        // Serialization occurs BEFORE type validations
        before: 'x-eov-type',
        compile: (sch, p, it) => {
            const validate = (data, ctx) => {
                console.log("index:130 - ")
                if (typeof data === 'string')
                    return true;
                try {
                    console.log("index:134 - ")
                    ctx.parentData[ctx.parentDataProperty] = sch.serialize(data);
                }
                catch (e) {
                    console.log("Can I please have the real error pretty please!?!?", e)
                    validate.errors = [
                        {
                            keyword: 'serdes',
                            instancePath: ctx.instancePath,
                            schemaPath: it.schemaPath.str,
                            message: `format is invalid`,
                            params: { 'x-eov-res-serdes': ctx.parentDataProperty },
                        },
                    ];
                    return false;
                }
                return true;
            };
            return validate;
        },
    });
}

P.S. Apologies for my sassy tone 😅, this issue really has been a 🤬 for me for quite some months. Thanks for the hard work put into this project as it really is important to us and we're very happy to be using it overall 🙏

Jackman3005 avatar Apr 13 '23 05:04 Jackman3005

If someone knows of a good place for me to "inject" some code that executes after calling res.send/res.json but before express-openapi-validator performs the serialization and validation steps I would be very grateful to get a hotfix in that does the deep clone work (and maybe even one that re-freezes after serializing to prevent other middleware from changing the body).

I'm a bit confused with how the modded.express.mung works or if the only way for me to accomplish a hotfix (besides updating dozens of endpoints individually) is to use express-mung myself...

Jackman3005 avatar Apr 13 '23 06:04 Jackman3005

Okay, I installed express-mung and have a workaround now. Here's how it goes in case others have this issue:

Workaround

Make a new file with the following code:

import mung from "express-mung";
import _ from "lodash";

/**
 * This middleware is used to un-freeze the response body.
 * This is done to allow the express-openapi-validator to overwrite properties with their serialized format before
 * validating the response body. See 
 * [this issue for more details](https://github.com/cdimascio/express-openapi-validator/issues/834)
 */
function unFreezeBody(body: unknown) {
  return _.cloneDeep(body);
}

export const unfreezeResponseBodyMiddleware = mung.json(unFreezeBody);

Add the unfreezeResponseBodyMiddleware AFTER the OpenApiValidator.middleware as Express Mung executes middleware in LIFO order.


app.use(
  OpenApiValidator.middleware({
    apiSpec: path.join(apiSrcPath, "../openapi-spec/api.yaml"),
    serDes: [
      {
        format: "date-time",
        deserialize: (s) => {
          return s;
        },
        serialize: (o) => {
          if (o instanceof Date) {
            return o.toISOString();
          }
          return "";
        },
      },
    ],
    ajvFormats: ["uuid"],
    validateResponses: {
      onError: (error, body, req) => {
        console.error(`Response body fails validation!`);
        console.error(`Emitted from:`, req.originalUrl);
        console.debug("invalid body: ", JSON.stringify(body));
        if (process.env.NODE_ENV !== "production") {
          throw error;
        } else {
          const lessScaryError = new Error(
            `Response body fails OpenAPI Spec Validation! Original error: ${error.message}`
          );
          Sentry.captureException(lessScaryError, {
            extra: {
              message:
                "This error was caught and swallowed because we are in production.",
              body,
              url: req.originalUrl,
            },
          });
        }
      },
    },
  }),
  unfreezeResponseBodyMiddleware
);

🤔 I have concerns about performance with cloning on all my requests and to be honest I'm not entirely sure of how express-openapi-validator would get around that either... Maybe there is a cheaper way to do this, or maybe I'm overestimating the impact it has as I have not measured it.

Jackman3005 avatar Apr 13 '23 06:04 Jackman3005

Another note of clarity here... I was playing around with the serDes configuration and overriding the date-time serialization in an attempt to fix the original error message or to learn more. In the end, I do not need to provide a serDes configuration at all and can benefit from the built-in ajvFormats: ["uuid", "date-time"] for the same effect. It still requires the unfreezing to work though...

Jackman3005 avatar Apr 13 '23 06:04 Jackman3005