fastify / fastify-express

Express compatibility layer for Fastify
MIT License
247 stars 26 forks source link

fix: prevent body parser error #79

Closed SuperOleg39 closed 2 years ago

SuperOleg39 commented 2 years ago

After integrating fastify, POST requests to my application stopped working. The problem is somewhere in the depths of body-parser library for express. If parsing for body from this library works, request hangs, and fastify when calling next in onRequest doesn't reach routing, request just hangs.

We're not the only ones who are like this - https://github.com/fastify/fastify-express/issues/73#issuecomment-1067022626

To solve this problem, we can reject body-parser and cookie-parser as well. To maintain backward compatibility, we can use already prepared parsing result from fastify-cookie and fastify-formbody, put it into req.raw.cookies and req.raw.body respectively. Then we need to move the express call to the preHandler hook, since we don't have the parsing results ready before that.

SuperOleg39 commented 2 years ago

res.end should block middleware execution test case are broken with this changes. Is this case are really important?

SuperOleg39 commented 2 years ago

Thanks and good spot! I'm not sure what is the setup for your express app, however how things are wired here have worked for some people (including myself).

Could you make the hook configurable, document the new option in the README, and add some tests? Thanks!

Yes, add tests and troubleshooting section)

SuperOleg39 commented 2 years ago

Looks like I added unstable tests?

mcollina commented 2 years ago

Doesn't look like it, things are passing on master

SuperOleg39 commented 2 years ago

Nice, thanks!