req.on('end', function() {}) never fires, middleware hangs

[dblock]

With another body-parser above it (see https://github.com/alexa-js/alexa-app/issues/152) the middleware hangs. The middleware needs to read and process the entire request body in order for the validation to work, and express doesn't expose that on the request object directly. So it accumulates the whole data of the request before any processing happens. That doesn't explain why the request would hang.

[tejashah88]

Is this bug solved yet, or is there still a problem?

[dblock]

I believe it's not solved. The middleware part was addressed, ie. the way alexa-app uses alexa-verifier-middleware doesn't exhibit this behavior unless the user throws a bodyparser above it. When the user does, the middleware hangs.

[tejashah88]

I did some more testing and apparently, the body-parser seems to swallow up the entire request. This is because the req.on('data', function() {}) is never fired in the first place.

I dug through the body-parser source code, specifically json.js, and I found that there are some debug statements that can be printed when you set the environment variable of DEBUG. I did that and this is the relevant output that I got:

If you look at the middle, there is a debug line of interest: "read body". I traced it to read.js and here's some interesting results:

According to the few comments that I found, the body-parser seems to be reading the entire request body and then processing it.

[dblock]

Makes sense, this confirms the bug. There needs to be some code in the middleware that skips it when there's not going to be any data.

[tejashah88]

I disagree, as the purpose of this module would be defeated. If someone used a similar practice to test_alexa_verifier_middleware_hangs.js, he/she wouldn't really know why the verification fails until we told him/her.

I'd say that the module should throw an error if the body is already processed and tell the user to change the code as necessary. An example would be:

if (req._body) {
  return res.status(401).json({ status: 'failure', reason: 'The incoming body was already parsed! Are you sure that this module is loaded before any body-parser modules?' })
}

[dblock]

Yes you're right.