Node HTTP: Handling Request Body Conversion

[mkoumila]

Hi,
I'm using @edge-csrf/node-http in my project which has a form that sends data from Next.js ( using custom Node server ) to Drupal CMS. After integrating @edge-csrf/node-http in my project the Post request payload was in the format of Buffer instead of an array, example:

array:1 [
"{"type":"Buffer","data":_110,97,109,101,61,97,100,109,105,110,38,101,109,97,105,108,61,38,99,115,114,102,95,116,111,107,101,110,61,65,65" => ""
]

Instead of :

array:7 [
  "name" => "user"
  "email" => "[email protected]"
  "csrf_token" => "b9300a66319b9aee5668f451ffe31250404c6813078be82f8733bb201cacj9Sd"
  "subject" => "Testing forms title"
  "message" => "Testing forms desc"
  "webform_id" => "contact"
]

Problem: The @edge-csrf/node-http package was improperly converting request bodies to strings without considering their content type, leading to issues with specific formats like application/x-www-form-urlencoded.

Solution: A patch was implemented to check the Content-Type header and appropriately convert the Buffer to a string or JSON, ensuring correct downstream processing.

[amorey]

What does your middleware chain look like without @edge-csrf/node-http? In particular, I'm curious how the request is processed without @edge-csrf/node-http in the picture.

Ideally, Edge-CSRF should leave request body parsing decisions to code further downstream since that isn't the purpose of the project. This is the intention of line 33:

req.body = Buffer.concat(buffer);

which passes along the raw content of the request body without parsing it. Next.js or another handler is probably checking to see if the request body attribute is present and then assuming it was already parsed so I'm wondering if there's a way to signal to the code that this isn't the case or to avoid setting body altogether.

Can you try this version of getRequestBody() that resets the request object so it can be re-read downstream:

function getRequestBody(req: IncomingMessage): Promise<string> {
  return new Promise((resolve, reject) => {
    const buffer: any[] = [];

    const onAborted = () => {
      reject(new Error('request aborted'));
    };

    const onData = (chunk: any) => {
      buffer.push(chunk);
    };

    const onEnd = () => {
      // concatenate all the chunks into a single buffer
      const bodyBuffer = Buffer.concat(buffer);

      // create a new Readable stream from the buffered data
      const newStream = new Readable();
      newStream.push(bodyBuffer);
      newStream.push(null);

      // Replace the original request's stream with the new one
      Object.assign(req, {
        read: newStream.read.bind(newStream),
        pipe: newStream.pipe.bind(newStream),
        unpipe: newStream.unpipe.bind(newStream),
        on: newStream.on.bind(newStream),
        pause: newStream.pause.bind(newStream),
        resume: newStream.resume.bind(newStream),
      });

      // Resolve the promise with the body as a string
      resolve(bodyBuffer.toString());
    };

    const onErr = (err: Error) => {
      reject(err);
    };

    const onClose = () => {
      req.removeListener('data', onData);
      req.removeListener('end', onEnd);
      req.removeListener('error', onErr);
      req.removeListener('aborted', onAborted);
      req.removeListener('close', onClose);
    };

    // Attach listeners
    req.on('aborted', onAborted);
    req.on('data', onData);
    req.on('end', onEnd);
    req.on('error', onErr);
    req.on('close', onClose);
  });
}

[amorey]

@mkoumila Have you had a chance to try the suggested patch?

[mkoumila]

Hey @amorey ,

Sorry for late answering, i out-of-office.
I will test it this Friday and let you know 🙏🏻

Thanks for your support

[amorey]

@mkoumila Have you had a chance to try the patch?

[amorey]

@mkoumila Any updates? Would be great to patch the code and close this pr.