0

I am writing a private plugin for nodebb (open forum software). In the nodebb's webserver.js file there is a line that seems to be hogging all incoming json data.

app.use(bodyParser.json(jsonOpts));

I am trying to convert all incoming json data for one of my end-points into raw data. However the challenge is I cannot remove or modify the line above.

The following code works ONLY if I temporarily remove the line above.

    var rawBodySaver = function (req, res, buf, encoding) {
      if (buf && buf.length) {
        req.rawBody = buf.toString(encoding || 'utf8');
      }
    }

    app.use(bodyParser.json({ verify: rawBodySaver }));

However as soon as I put the app.use(bodyParser.json(jsonOpts)); middleware back into the webserver.js file it stops working. So it seems like body-parser only processes the first parser that matches the incoming data type and then skips all the rest?

How can I get around that? I could not find any information in their official documentation.

Any help is greatly appreciated.

** Update **

The problem I am trying to solve is to correctly handle an incoming stripe webhook event. In the official stripe documentation they suggested I do the following:

  // Match the raw body to content type application/json
  app.post('/webhook', bodyParser.raw({type: 'application/json'}), 
  (request, response) => {
    const sig = request.headers['stripe-signature'];

    let event;

    try {
      event = stripe.webhooks.constructEvent(request.body, sig, 
  endpointSecret);
    } catch (err) {
      return response.status(400).send(Webhook Error: 
  ${err.message});
    }

Both methods, the original at the top of this post and the official stripe recommended way, construct the stripe event correctly but only if I remove the middleware in webserver. So my understanding now is that you cannot have multiple middleware to handle the same incoming data. I don't have much wiggle room when it comes to the first middleware except for being able to modify the argument (jsonOpts) that is being passed to it and comes from a .json file. I tried adding a verify field but I couldn't figure out how to add a function as its value. I hope this makes sense and sorry for not stating what problem I am trying to solve initially.

Avan
  • 13
  • 4
  • This appears to be an [XY problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem) where you've described a problem with your attempted solution, but not described the original problem that you really need solved. Please describe the higher level problem you're trying to solve. – jfriend00 Feb 05 '20 at 01:49
  • Edited so it states the higher problem now. At least I hope. – Avan Feb 05 '20 at 16:45

2 Answers2

0

The bodyParser.json() middleware does the following:

  1. Check the response type of an incoming request to see if it is application/json.
  2. If it is that type, then read the body from the incoming stream to get all the data from the stream.
  3. When it has all the data from the stream, parse it as JSON and put the result into req.body so follow-on request handlers can access the already-read and already-parsed data there.

Because it reads the data from the stream, there is no longer any more data in the stream. Unless it saves the raw data somewhere (I haven't looked to see if it does), then the original RAW data is gone - it's been read from the stream already. This is why you can't have multiple different middleware all trying to process the same request body. Whichever one goes first reads the data from the incoming stream and then the original data is no longer there in the stream.

To help you find a solution, we need to know what end-problem you're really trying to solve? You will not be able to have two middlewares both looking for the same content-type and both reading the request body. You could replace bodyParser.json() that does both what it does now and does something else for your purpose in the same middleware, but not in separate middleware.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • I was afraid of that. I am trying to handle a stripe webhook event. In the stripe documentation they suggested to do the following: `app.post('/webhook', bodyParser.raw({type: 'application/json'}), (request, response) => { const sig = request.headers['stripe-signature']; let event; try { event = stripe.webhooks.constructEvent(request.body, sig, endpointSecret); } catch (err) { return response.status(400).send(`Webhook Error: ${err.message}`); }` But as you pointed out I cannot use separate middleware to handle the same data. And I cannot modify the first middleware – Avan Feb 05 '20 at 16:08
  • @Avan - Can you put your stripe request handler BEFORE the existing `app.use(bodyParser.json(jsonOpts));`? That is how one would normally solve this. Then, your request handler would get to run while the request body is still there and it would only run its `bodyparser.raw()` on it's own request so that wouldn't mess up anything else. – jfriend00 Feb 05 '20 at 16:15
  • I am afraid not. However I edited my original post to include one more key piece of information. I have access and can edit the argument (jsonOpts) that the first middleware uses. All options in jsonOpts come from a json file. I was thinking of adding a verify field and then maybe checking what route is being handled before converting the data to raw? However I couldn't figure out how to add a function as a value in a json file. Other than that I am limited when it comes to modifying the webserver file since that is part of the core application. – Avan Feb 05 '20 at 16:51
  • Because all changed would be reverted with the next update. I can only built on top of the application through plugins which use hooks. However I don't think there is hook to modify this particular part of the code. However knowing or sure that there is is no way of fixing this without modifying the webserver file I can now reach out the nodebb developers and ask them if they are planning to address this issue in the future and if there is something I can do in the meantime. – Avan Feb 05 '20 at 17:14
  • Of course. Please stay patient. I am working on creating a plugin for nodebb (forum software). It's purposes as my original post suggests is to add a stripe payment portal. All available hooks that are available to me are listed here: https://github.com/NodeBB/NodeBB/wiki/Hooks I just looked through them again and I could not find a hook that would fire up before the middleware in question. So as of right now it seems the only way I can fix my problem is if the nodebb developers add another hook before the middleware is defined, right? Although it is possible that I missed a useful hook. – Avan Feb 05 '20 at 17:34
  • @Avan - Yeah, so you need a plugin-hook that passes the `app` object or the `server` object as an argument that is called before nodeBB calls `configureBodyParser()` in `setupExpressApp()`. I haven't found any. – jfriend00 Feb 05 '20 at 17:50
  • @Avan - Only option I see right now is to hack into the `app` layer list and insert a new middleware at the front of the list so that it runs before the built-in `bodyParser.json()`. – jfriend00 Feb 05 '20 at 17:53
  • Why do you keep deleting your posts? It get's confusing. The last thing you recommended was to use the `static:app.preload` However as we suspected this hook get's fired up too late. I just tested it just to be sure. Thanks for all you help though. I appreciate it. – Avan Feb 05 '20 at 18:18
  • @Avan - I deleted that answer because I realized it wouldn't work. It comes too late. Are you interested in a temporary solution (until NodeBB gives you the appropriate hook) that manually inserts a route into the app router near the front of the list. That will work as long as Express doesn't change their internals in a major revision. – jfriend00 Feb 05 '20 at 18:19
0

The only solution I can find without modifying the NodeBB code is to insert your middleware in a convenient hook (that will be later than you want) and then hack into the layer list in the app router to move that middleware earlier in the app layer list to get it in front of the things you want to be in front of.

This is a hack so if Express changes their internal implementation at some future time, then this could break. But, if they ever changed this part of the implementation, it would likely only be in a major revision (as in Express 4 ==> Express 5) and you could just adapt the code to fit the new scheme or perhaps NodeBB will have given you an appropriate hook by then.

The basic concept is as follows:

  1. Get the router you need to modify. It appears it's the app router you want for NodeBB.
  2. Insert your middleware/route as you normally would to allow Express to do all the normal setup for your middleware/route and insert it in the internal Layer list in the app router.
  3. Then, reach into the list, take it off the end of the list (where it was just added) and insert it earlier in the list.
  4. Figure out where to put it earlier in the list. You probably don't want it at the very start of the list because that would put it after some helpful system middleware that makes things like query parameter parsing work. So, the code looks for the first middleware that has a name we don't recognize from the built-in names we know and insert it right after that.

Here's the code for a function to insert your middleware.

function getAppRouter(app) {
    // History:
    //   Express 4.x throws when accessing app.router and the router is on app._router
    //      But, the router is lazy initialized with app.lazyrouter()
    //   Express 5.x again supports app.router 
    //      And, it handles the lazy construction of the router for you
    let router;
    try {
        router = app.router;      // Works for Express 5.x, Express 4.x will throw when accessing
    } catch(e) {}
    if (!router) {
        // Express 4.x
        if (typeof app.lazyrouter === "function") {
            // make sure router has been created
            app.lazyrouter();
        }
        router = app._router;
    }

    if (!router) {
        throw new Error("Couldn't find app router");
    }
    return router;
} 

// insert a method on the app router near the front of the list
function insertAppMethod(app, method, path, fn) {
    let router = getAppRouter(app);
    let stack = router.stack;

    // allow function to be called with no path
    // as insertAppMethod(app, metod, fn);
    if (typeof path === "function") {
        fn = path;
        path = null;
    }

    // add the handler to the end of the list
    if (path) {
        app[method](path, fn);
    } else {
        app[method](fn);
    }
    // now remove it from the stack
    let layerObj = stack.pop();
    // now insert it near the front of the stack, 
    // but after a couple pre-built middleware's installed by Express itself
    let skips = new Set(["query", "expressInit"]);
    for (let i = 0; i < stack.length; i++) {
        if (!skips.has(stack[i].name)) {
            // insert it here before this item
            stack.splice(i, 0, layerObj);
            break;
        }
    }
}

You would then use this to insert your method like this from any NodeBB hook that provides you the app object sometime during startup. It will create your /webhook route handler and then insert it earlier in the layer list (before the other body-parser middleware).

let rawMiddleware = bodyParser.raw({type: 'application/json'});

insertAppMethod(app, 'post', '/webhook', (request, response, next) => {
    rawMiddleware(request, response, (err) => {
        if (err) {
            next(err);
            return;
        }
        const sig = request.headers['stripe-signature'];

        let event;

        try {
          event = stripe.webhooks.constructEvent(request.body, sig, endpointSecret);
          // you need to either call next() or send a response here
        } catch (err) {
          return response.status(400).send(`Webhook Error: ${err.message}`);
        }
    });
});
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • 1
    Just incredible! It worked. You are a coding genius. Thank you so much. – Avan Feb 05 '20 at 19:45
  • @Avan - Awesome. It would be really easy for NodeBB to add an `app` hook slightly earlier before their middleware is installed. Hopefully they will do that for you. – jfriend00 Feb 05 '20 at 19:48
  • I will reach out to them to see what their thoughts are. Thanks again. – Avan Feb 05 '20 at 19:50
  • @Avan - I updated the code so it would be compatible with Express 5 if/when NodeBB switches to it. Express 5 is not yet released, but it does contain changes that would affect this code so I've proactively planned for that. I have tested this code with both Express 4 and Express 5. – jfriend00 Feb 05 '20 at 22:24
  • That's great especially if I cannot depend on nodebb to add a new hook. Thanks again. – Avan Feb 06 '20 at 23:04