Supporting proxy after request body has been read

[brandon-leapyear]

Our exact use-case is a bit special:

1. We have a NestJS application, and we still want to use our NestJS guards before proxying
2. Our endpoint needs to set { strict: false } for the JSON body parser, because our proxied endpoint can take in non-objects as input
3. We don't want to parse the incoming request at all. The exact use case we're thinking of is JSON data with big ints that would get rounded if it went through standard body-parser functions

So to implement this, we extracted out the raw body before body-parser can touch it, then for our proxy, we did

proxy('...', {
  proxyReqBodyDecorator: () => rawBody,
})

to completely ignore the body that was parsed by body-parser/express-http-proxy and manually set the body back to the raw body we got.

But in our testing, we noticed that sending values that would be parsed as falsy JSON values, like 'false' or '""', our tests would hang. Basically, it would get to

https://github.com/villadora/express-http-proxy/blob/2aec18894bfe4d06fc61d959cf718c9e1496226e/lib/requestOptions.js#L91-L100

and req.body would be falsy, so it would try to read the request body stream. But the stream had already been read, meaning that getRawBody will never return.

Just setting parseReqBody to false didn't help either because it would skip the entire section that sends the result of proxyReqBodyDecorator and try to pipe the original request's body to the proxy request, which wouldn't pipe anything because the request body had already been read.

https://github.com/villadora/express-http-proxy/blob/2aec18894bfe4d06fc61d959cf718c9e1496226e/app/steps/sendProxyRequest.js#L40-L68

To fix the latter point, I'm checking bodyContent if parseReqBody is false and sending that if it's populated. In normal operation, bodyContent is null, but with proxyReqBodyDecorator, bodyContent is populated.

To fix the hanging, I added an explicit check in requestOptions that makes sure the request hasn't already been read. This shouldn't happen in normal operation, but it would've saved me a lot of headache if it were there. Another option is to change if (req.body) to if (req.body !== undefined), but I figured that would be more change in behavior than this library might want.

[giteshk]

@monkpow can we get this PR merged.

This enabled me to write an elegant code instead of having to add a route for POST and PUT operations.

[raghav1uphealth]

this seems to match my use case as well! I use express.raw to parse my body (now available in express 4.17): app.use(express.raw({ ..., type: 'application/xml' })); and have verified the body is stored as a Buffer in req.body. and configure the proxy like:

proxy('...', {
  parseReqBody: false,
  proxyReqBodyDecorator: (bodyContent, srcReq) => srcReq.body,
})

but it looks like the empty stream is what's sent in the proxy request - if i set parseReqBody to true, then I get the issue mentioned here: https://github.com/villadora/express-http-proxy/issues/487

[monkpow]

Thanks for this thoughtful fix and the appropriate tests.

[Ekansh82]

@monkpow, when will this fix be released?

[Ekansh82]

@monkpow, when will this fix be released?

Hey @monkpow, gentle reminder! :)

[Ekansh82]

@monkpow, when will this fix be released?

Hi @monkpow, @brandon-leapyear gentle reminder!

[brandonchinn178]

(This is @brandon-leapyear - replying from my personal account)

I'm not associated with this project, so I have no control over releasing this. But FYI package.json does have support for specifying a dependency with a commit: https://docs.npmjs.com/cli/v9/configuring-npm/package-json?v=true#git-urls-as-dependencies