Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Prevent headers from being sent twice #388

Closed
wants to merge 2 commits into from
@samalba

In some cases, when there is an error, the headers are sent twice which throws an exception. I was able to reproduce by doing this:

$ nc localhost 1080 << EOF
> GET / HTTP/1.1
> Host: foobar
>
> EOF

`foobar' is a valid proxied website. A proxy using node-http-proxy is running on localhost:1080.

This produces the following error:

http.js:708
    throw new Error('Can\'t set headers after they are sent.');
          ^
Error: Can't set headers after they are sent.
    at ServerResponse.OutgoingMessage.setHeader (http.js:708:11)
    at /Users/shad/Dropbox/work/hipache/node_modules/http-proxy/lib/node-http-proxy/http-proxy.js:332:13
    at Array.forEach (native)
    at ClientRequest.<anonymous> (/Users/shad/Dropbox/work/hipache/node_modules/http-proxy/lib/node-http-proxy/http-proxy.js:328:35)
    at ClientRequest.g (events.js:175:14)
    at ClientRequest.EventEmitter.emit (events.js:95:17)
    at HTTPParser.parserOnIncomingClient [as onIncoming] (http.js:1630:21)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:119:23)
    at Socket.socketOnData [as ondata] (http.js:1528:20)
    at TCP.onread (net.js:495:27)

The problem here is that netcat does not read anything. I tried with a script that reads the response and the error does not occur. The simple `if' statement I added prevents the bug to happen and everything then works normally.

@indexzero
Owner

@samalba Could you add a test case for this?

@samalba

I could yes, however I tried to run the tests and they're hanging on my machine: http://pastie.org/private/syspc2w6h0yl98u1kivnw

Did a miss a step? I tested this fix by running hipache's test (https://github.com/dotcloud/hipache/blob/master/test/test_parseerror.py#L95). However I'd be glad to provide one for node-http-proxy!

@indexzero
Owner

What node version are you on?

@indexzero
Owner

That's a good test; would love to see it in node.js. Create a net.Server instance and write a bad header line to it. Should be straight-forward.

@bwaters

Please merge, I'm having this problem too.

@bfx81

Me too... pleaseeeeee :tongue:

@indexzero
Owner

@bwaters @bfx81 We will merge when there is a test for this.

@kadishmal

I'm getting the same problem.

@bfx81

I've tested the patch and this didn't solve the problem... checked at a "lower" level in http.js nodejs core module.
This solved my problem
bfx/node@0149cce

@bfx81

Sorry not sure anymore... this problem happens in a very chaotic way!

@bslayton

What's the latest with this bug?

@nfroidure

Did someone find a way to reproduce the bug ?

@drewzboto drewzboto referenced this pull request in drewzboto/grunt-connect-proxy
Closed

Explodes on 204 response #19

@davidsteinberger

I was hit today by that nasty Error: Can't render headers after they are sent to the client. error.

It obviously is caused by setting headers after some data has already been sent.
It took me a while but I found that in my case the proxyError-event handler is causing that issue:
Data has already been sent, the proxyError-handler sets the header 'Content-Type' and it blows up.

For the time being I simple commented out the writeHead-call:
//res.writeHead(500, { 'Content-Type': 'text/plain' });

@achselschweisz

Is this issue resolved yet? Admittedly it is hard to reproduce. I encountered the

Error: Can't render headers after they are sent to the client.
    at ServerResponse.OutgoingMessage._renderHeaders (http.js:733:11)
    at ServerResponse.writeHead (http.js:1150:20)
    at ClientRequest.proxyError (/opt/napp/router/node_modules/http-proxy/lib/node-http-proxy/http-proxy.js:203:9)
    at ClientRequest.g (events.js:175:14)
    at ClientRequest.EventEmitter.emit (events.js:95:17)
    at Socket.socketErrorListener (http.js:1547:9)
    at Socket.EventEmitter.emit (events.js:95:17)
    at net.js:441:14
    at process._tickCallback (node.js:415:13)

today.

@jcrugzz
Owner

@achselschweisz checkout the caronte branch. It is a complete refactor that will become node-http-proxy 1.0. Please open a new issue if you find this to be the case

@samalba

Nice! @jcrugzz any ETA for this 1.0 release?

@achselschweisz

cheers, I will try that and report back (btw., just had the error again).
I cannot though offer any help in reproducing the problem since this happens sporadically when streaming data to services which are proxied through node-http-proxy. Just now it took a few hours, numerous requests and about 40GiB of streaming data to make this bugger reappear.

@kahnjw

I am experiencing this same error, using https://github.com/drewzboto/grunt-connect-proxy to proxy to my backend API in development. Only happens on file upload. Don't know if I am being naive or if I am experiencing the same issue.

Error: Can't render headers after they are sent to the client.
    at ServerResponse.OutgoingMessage._renderHeaders (http.js:733:11)
    at ServerResponse.res._renderHeaders (/Users/kahnjw/Documents/stompdrop/frontside/node_modules/grunt-contrib-connect/node_modules/connect/lib/patch.js:69:27)
    at ServerResponse.writeHead (http.js:1150:20)
    at ServerResponse.res.writeHead (/Users/kahnjw/Documents/stompdrop/frontside/node_modules/grunt-contrib-connect/node_modules/connect/lib/patch.js:75:22)
    at ClientRequest.proxyError (/Users/kahnjw/Documents/stompdrop/frontside/node_modules/grunt-connect-proxy/node_modules/http-proxy/lib/node-http-proxy/http-proxy.js:203:9)
    at ClientRequest.g (events.js:180:16)
    at ClientRequest.EventEmitter.emit (events.js:95:17)
    at Socket.socketErrorListener (http.js:1547:9)
    at Socket.EventEmitter.emit (events.js:95:17)
    at onwriteError (_stream_writable.js:233:10)
    at onwrite (_stream_writable.js:251:5)
    at WritableState.onwrite (_stream_writable.js:97:5)
    at fireErrorCallbacks (net.js:438:13)
    at Socket._destroy (net.js:472:3)
    at Object.afterWrite (net.js:718:10)

If it seems like the same issue I'd be interested to know.

@kahnjw

Nevermind, im dumb

@indexzero
Owner

Despite @samalba never submitting a test for this I'm going to roll this into the last 0.10.x maintenance release because the logic is sound.

@samalba write moar tests!

@indexzero
Owner

Cherry-picked. Thanks!

@indexzero indexzero closed this
@inian

I am still getting this error when trying to proxy http://www.amazon.de/ website. Can someone reproduce the bug for this website?

@nolanlawson

FWIW, I'm seeing this error constantly as well, using http-proxy version 0.10.4, when I set up an http-proxy in front of CouchDB:

http.js:731
    throw new Error('Can\'t render headers after they are sent to the client.'
    ^
Error: Can't render headers after they are sent to the client.
    at ServerResponse.OutgoingMessage._renderHeaders (http.js:731:11)
    at ServerResponse.writeHead (http.js:1148:20)
    at ClientRequest.proxyError (/Users/nolan/workspace/pouchdb/node_modules/http-proxy/lib/node-http-proxy/http-proxy.js:213:9)
    at ClientRequest.g (events.js:180:16)
    at ClientRequest.EventEmitter.emit (events.js:95:17)
    at Socket.socketOnData (http.js:1588:9)
    at TCP.onread (net.js:527:27)

Whose responsibility is it to fix a bug like this? Node? This module? The implementor (e.g. proxy.on('error')? I'm not sure.

@nolanlawson

Ah wait, I am using a super old version of this module. Please ignore the above comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 22, 2013
  1. @samalba
Commits on Nov 15, 2013
  1. @samalba
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 4 deletions.
  1. +6 −4 lib/node-http-proxy/http-proxy.js
View
10 lib/node-http-proxy/http-proxy.js
@@ -326,10 +326,12 @@ HttpProxy.prototype.proxyRequest = function (req, res, buffer) {
}
// Set the headers of the client response
- Object.keys(response.headers).forEach(function (key) {
- res.setHeader(key, response.headers[key]);
- });
- res.writeHead(response.statusCode);
+ if (res.sentHeaders !== true) {
+ Object.keys(response.headers).forEach(function (key) {
+ res.setHeader(key, response.headers[key]);
+ });
+ res.writeHead(response.statusCode);
+ }
function ondata(chunk) {
if (res.writable) {
Something went wrong with that request. Please try again.