Passing multipart data as a Buffer corrupts req.body and increase x100 Mongoose time to update records

[jeanbmar]

Bug Report

After 2 days of headaches wondering why it took 15s to update a single record on our serverless deployment, I figured out that req.body gets corrupted by express-fileupload if the multipart body passed to express is a Buffer instance (reasons explained here: richardgirges/express-fileupload#290).

Popular serverless express proxies do this transformation, which is probably valid and properly managed by busboy anyway.

Mongoose will then receive an object containing the usual properties as well as extra properties, 1 property per byte in the req.body buffer to be accurate, and will try to validate every one of them, so you see the disaster coming.

Anyone using Payload with Serverless and AWS Lambda will have this problem.

Expected Behavior

The req.body object should contain only the relevant properties.

Possible Solution

I doubt express-fileupload will be fixed anytime soon.

• A quick fix for Payload could be to add a middleware before express-fileupload to transform any buffer into something express-fileupload can manage properly.
• Multer doesn't reuse req.body, but replacing express-fileupload would require some work.
• Disabling parseNested in Payload upload config could help, but I'm not sure how deepCopyObject would behave on a buffer anyway.

Steps to Reproduce

const serverlessHttp = require('serverless-http');
require('dotenv').config();
const app = require('./server');

module.exports.handler = serverlessHttp(app);

const cookie = 'payload-token=...';

(async () => {
  // wait for payload build
  await new Promise((resolve) => setTimeout(resolve, 30000));
  console.time('issue');
  await module.exports.handler({
    resource: '/{proxy+}',
    path: '/api/resource/test',
    httpMethod: 'PUT',
    headers: {
      'content-type':
        'multipart/form-data; boundary=----WebKitFormBoundaryvl9BV6cBtmvisceh',
      cookie,
    },
    body: "------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"name\"\r\n\r\nTset2\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"slug\"\r\n\r\ntset\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"type\"\r\n\r\nsuper\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"shortDescription\"\r\n\r\ntste desc\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"description\"\r\n\r\ncdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccccdesccca\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"brawler\"\r\n\r\nnull\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"summon\"\r\n\r\nnull\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh\r\nContent-Disposition: form-data; name=\"stats\"\r\n\r\n0\r\n------WebKitFormBoundaryvl9BV6cBtmvisceh--\r\n",
  });
  console.timeEnd('issue'); // takes 8-9s
})();

req.body corruption can be observed by logging the body in a middleware before and after the express-fileupload middleware.

Detailed Description

Payload 0.10.6

[jeanbmar]

In the meantime I've created richardgirges/express-fileupload#291 to try getting it fixed on express-fileupload.

[jmikrut]

Hey @jeanbmar — thanks for finding this and sorry to hear about your serverless struggles! I took a look into your PR within express-fileupload and that's certainly, at least in my opinion, the best fix at the moment.

If not, another potential fix I could see would be to iterate through the data coming in from Express to only pass properties that are explicitly defined as fields within Payload. We'd likely be able to do this right in the requestHandlers so as to not interfere with local operations or GraphQL. This might be a good idea to do anyway in the long run.

I say we wait to see what happens with your PR, and if it gets merged, then we'll be good. If not, we will make some quick moves to pair down incoming Express request bodies.

What do you think?

[jeanbmar]

Perfect, thank you!
Let's wait for express-fileupload maintainers response, and for now I'll monkey patch the PR file.

[jmikrut]

Update - this is now fixed, and a minor version will be coming shortly. Closing the issue now!

[jeanbmar]

Thank you very much for this and for the follow-up on express-fileupload.

[github-actions]

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.