Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request to make bodyParser optional #405

Closed
lizardruss opened this issue Feb 7, 2018 · 8 comments
Closed

Request to make bodyParser optional #405

lizardruss opened this issue Feb 7, 2018 · 8 comments

Comments

@lizardruss
Copy link

I'm submitting a...


[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

bodyParser.json() is always added here. However, I am using node-http-proxy to implement a reverse proxy, and parsing the body is not needed when proxying the request through to a destination.

I'd like to be able to more selectively apply body-parser middleware for this purpose.

Expected behavior

Perhaps make the default middleware overridable:

const server = express();
await NestFactory.create(AppModule, server, {
   middlewares: [],
});

... or add a flag allow people to manage their own middleware:

const server = express();
await NestFactory.create(AppModule, server, { parserMiddleware: false });

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?

More granular control of where body-parser middleware is used.

Environment


Nest version: 4.5.10

 
For Tooling issues:
- Node version: XX  
- Platform:  

Others:

@lizardruss lizardruss changed the title Reques to make bodyParser optional Request to make bodyParser optional Feb 7, 2018
@shekohex
Copy link
Contributor

shekohex commented Feb 8, 2018

Hi @lizardruss , in express all middlewares a registered in the express instance stack

...
const instance = express();
const app = await NestFactory.create(AppModule, instance);
const expressStack = instance.stack; // an array of middlewares, configuration, etc..
for (const i = 0; i < expressStack.length;  i++) {
   console.log(expressStack[i].name);
   // TODO: slice the `bodyPaser` from the stack.
}
...

honestly, i did't try it, but you should give it a shoot

@lizardruss
Copy link
Author

@shekohex Yeah, that's the workaround I'm using at the moment. I was hoping to avoid modifying the stack directly.

@lizardruss
Copy link
Author

lizardruss commented Feb 12, 2018

@shekohex @kamilmysliwiec Another idea for customizing the middleware stack might be to make the MiddlewaresModule into a 3rd party module, and include a default implementation for the default behavior. This would allow total control over the middlewares and their order, but still provide the defaults that everyone's come to expect.

I've been digging into 3rd party modules for another purpose (@Upgrade decorator for handling HTTP upgrade events). I'd be willing to work on a PR if that seems like valid direction to go.

I'm not sure how possible it is to achieve because of express, but for my purposes, I'd like to have a different middleware stack per module. For example, I have a module where I want to exclude the bodyParser middleware, but everyone other module needs to include it.

I realize that this is mostly achievable by implementing the NestModule interface, except for the middleware that's added by default.

@lizardruss
Copy link
Author

This. 💯 #255 (comment)

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Feb 15, 2018

Hi @lizardruss,
Thanks for ideas 🙂 Since v4.6.1 you can simply disable bodyParser in this way:

const app = await NestFactory.create(ApplicationModule, {
  bodyParser: false,
});

@krzkaczor
Copy link
Contributor

@kamilmysliwiec I want to disable body parsing only for one route (which is basically file uploader, and it breaks on uploading JSON files since they got parsed).

How can I achieve this?

@krzkaczor
Copy link
Contributor

Sorry to bother you. I figured it out:

// first disable body parser as shown above, then create custom middleware

  import { json } from "body-parser";

  const jsonParseMiddleware = json();
  app.use((req: any, res: any, next: any) => {
    // do not parse json bodies if we are hitting file uploading controller
    if (req.path.indexOf("/v1/files") === 0) {
      next();
    } else {
      jsonParseMiddleware(req, res, next);
    }
  });

@lock
Copy link

lock bot commented Sep 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants