Content length header getting set inappropriatly

[patrickhousley]

Express is adding a content length header even when the transfer-encoding is chunked. This is an issue in Node 5.6 since they upgraded to http-parser 2.6.1 (https://nodejs.org/en/blog/release/v5.6.0/). Specifically, this is related to CVE-2016-2086. Need to modify line 179-180 of response.js and enclose it in a logical IF to test if transfer-encoding is 'chunked'.

[dougwilson]

Hi! This is not an issue with Express.js in any way. The only way the Transfer-Encoding header would get set would be because you have code somewhere adding it.

If you are using res.send or other helpers, you cannot set headers that deal with transport details, because that's not the business of your code. If you want to interact directly with the transport, you need to use res.write and res.send directly.

You can find a previous discussion on this here: #2866 which was just a bug in the user's code incorrectly setting the Transfer-Encoding header.

[dougwilson]

Sorry, didn't mean to hit the close button; re-opening for discussion. The Transfer-Encoding header should definitely not be getting set by code before calling things like res.send here, so ultimately there is a bug in the code using Express.

That being said, perhaps res.send can just be changed to unconditionally remove the Transfer-Encoding header in Express 5? Or be smarted in some way? We currently have an issue with the Trailer header as well, so perhaps the fix for handing of an existing Trailer header should take all this into account.

[patrickhousley]

In this particular instance, I am using express to setup an https proxy to client services. In addition to the express setup, we have browser-sync spinning up on another port that points to the express https proxy. So from the browser standpoint, our request goes like this: browser -> browser-sync -> express proxy -> client. The client is not setting a content-length header and browser-sync is catching the error being thrown from the http-parser library used in Node.

[patrickhousley]

Created a pull request that fixes the issue. #2894

[dougwilson]

Hi @patrickhousley, sure, but I'm not sure what that really means. Can you provide a way to reproduce the issue that only uses Express? If not, please drop by the Gitter room (https://gitter.im/expressjs/express) for support.

The pull request is a good first pass on a possible fix, though! You should really fix your code that is incorrectly adding the Transfer-Encoding in the mean time, since Express 5 won't be out for some time.