vercel / next.js

The React Framework
https://nextjs.org
MIT License
125k stars 26.7k forks source link

Fastify: next.js attempts (and fails) to consume a request body that's already been consumed by Fastify #24894

Open chrskrchr opened 3 years ago

chrskrchr commented 3 years ago

NOTE: this description was updated on Dec 14th, 2022 to reflect modern repro details

What version of Next.js are you using?

13.0.6

What version of Node.js are you using?

16.16.0

What browser are you using?

Chrome

What operating system are you using?

macOS

How are you deploying your application?

custom server (fastify 4.10.2)

Describe the Bug

When using Fastify as a custom web server, performing a POST to a Next.js /api route results in a 400 Invalid body response.

Expected Behavior

The API handler is allowed to process the request.

To Reproduce

  1. Clone this repo: https://github.com/chrskrchr/next-js-api-body-bug
  2. Run npm install (Node v16)
  3. Run npm run dev
  4. Run this command:
    • curl -v -POST -H "Content-Type: application/json" -d '{"ping":true}' http://localhost:3000/api/test

You should see a 400 Invalid Body response like the following:

*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> POST /api/test HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.84.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 13
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 400 Invalid body
< Date: Wed, 14 Dec 2022 13:19:46 GMT
< Connection: keep-alive
< Keep-Alive: timeout=72
< Transfer-Encoding: chunked
<
* Connection #0 to host localhost left intact
Invalid body%  

Uncomment the no-op content parser in server.ts to work around the issue.

chrskrchr commented 3 years ago

This is the same issue described in #9978 that I think was mistakenly marked as fixed. The issue still exists, and we have to work around it by wiring up a no-op content parser in Fastify that disables parsing of the request body on Next.js routes.

It would be great if Next.js had a way to handle this defensively and not attempt to consume a request body that's already been consumed.

PabloSzx commented 3 years ago

This is the same issue described in #9978 that I think was mistakenly marked as fixed. The issue still exists, and we have to work around it by wiring up a no-op content parser in Fastify that disables parsing of the request body on Next.js routes.

It would be great if Next.js had a way to handle this defensively and not attempt to consume a request body that's already been consumed.

can you give an example of the no-op you mention?

I've been trying

  app.addContentTypeParser('application/json', { parseAs: 'buffer' }, (_req, body, done) => {
    done(null, body);
  });

and

  app.addContentTypeParser('application/json', { parseAs: 'string' }, (_req, body, done) => {
    done(null, body);
  });

and it doesn't work

EDIT:

This works:

app.addContentTypeParser('application/json', (_req, body, done) => {
    done(null, body);
});
chrskrchr commented 3 years ago

Right - in your first example, the { parseAs: '...' } triggered Fastify to consume the body prior to passing it to your handler, which mean that Next.js would continue to fail since the body had already been consumed.

For posterity - our async no-op handler looks like this:

  async function noOpParser(req: FastifyRequest, payload: IncomingMessage) {
    return payload;
  }

  fastifyInstance.addContentTypeParser('text/plain', noOpParser);
  fastifyInstance.addContentTypeParser('application/json', noOpParser);
felippem commented 2 years ago

Do maintainers understand the problem or are they already working on it? cc/ @Timer

MarioSerano commented 1 year ago

Hey. I'm interested to take on this task.

There's this statement that is raised by @chrskrchr that is:

"... It would be great if Next.js had a way to handle this defensively and not attempt to consume a request body that's already been consumed."

I wonder whether I can have a bit of a clue over where does Next.js store their content parsing and I want to see what I can do there.

Or, probably whether it's the decision of the maintainer over which this isn't handled defensively for reasons, I can then help with modifying the fastify example!

Thanks so much!

cc @timneutkens

rubiagatra commented 1 year ago

I will take a look the example given

rubiagatra commented 1 year ago

I think we should close this issue. @Timer @chrskrchr

➜ npx create-next-app --example custom-server-fastify custom-server-fastify-app
Could not locate an example named "custom-server-fastify". It could be due to the following:
 1. Your spelling of example "custom-server-fastify" might be incorrect.
 2. You might not be connected to the internet or you are behind a proxy.
seanparmelee commented 1 year ago

@rubiagatra The custom server examples were removed in https://github.com/vercel/next.js/commit/984627a65ed3e8fcb001d886bd25e96fcff47466 but you can still do

npx create-next-app@latest --example https://github.com/vercel/next.js/tree/v12.3.4/examples/custom-server-fastify custom-server-fastify-app
chrskrchr commented 1 year ago

I've updated the issue's description with modern repro steps using node@16, next@13, and fastify@4.

nclsjstnn commented 11 months ago

looking for a solution as well

archcorsair commented 11 months ago

Ran into this problem today, unable to use a POST request if it contains any sort of body. 400 Invalid Body Adding a noop content parser for application/json fixed it for me. Looking for a permanent solution though.