Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only constructor options are passed to the passes functions #510

Closed
jcrugzz opened this issue Nov 5, 2013 · 17 comments
Closed

Only constructor options are passed to the passes functions #510

jcrugzz opened this issue Nov 5, 2013 · 17 comments

Comments

@jcrugzz
Copy link
Contributor

jcrugzz commented Nov 5, 2013

All of this options parsing that is found here is not being utilized. We are extending the global this.options but not passing them into any of the passes. I will see if the fix is as trivial as it seems otherwise I'll let you handle it @yawnt.

jcrugzz added a commit that referenced this issue Nov 5, 2013
@jcrugzz jcrugzz closed this as completed Nov 5, 2013
@pmalek
Copy link

pmalek commented Feb 1, 2014

This issue is still present ( I am using node v0.10.25 and http-proxy v1.0.2 ).

Why it has been closed ?

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek can you show me a gist of a reproducing test case?

@pmalek
Copy link

pmalek commented Feb 1, 2014

@jcrugzz something like this https://gist.github.com/pmalek/8756997 although I have tried many more options to pass in there but all the time I get the same issue

/home/USER/programming/node/test1/node_modules/http-proxy/lib/http-proxy/passes/web-incoming.js:103
    var proxyReq = (options.target.protocol === 'https:' ? https : http).reque
                                  ^   
TypeError: Cannot read property 'protocol' of undefined
    at Array.stream [as 3] (/home/USER/programming/node/test1/node_modules/http-proxy/lib/http-proxy/passes/web-incoming.js:103:35)
    at ProxyServer.<anonymous> (/home/USER/programming/node/test1/node_modules/http-proxy/lib/http-proxy/index.js:83:21)
    at Server.closure (/home/USER/programming/node/test1/node_modules/http-proxy/lib/http-proxy/index.js:125:43)
    at Server.EventEmitter.emit (events.js:98:17)
    at HTTPParser.parser.onIncoming (http.js:2108:12)
    at HTTPParser.parserOnHeadersComplete [as onHeadersComplete] (http.js:121:23)
    at Socket.socket.ondata (http.js:1966:22)
    at TCP.onread (net.js:525:27)
-------------------------------------

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek look at the httpProxy.createServer(options) function. It only takes an options argument and expects a full URL to proxy to. See the examples in the readme to see that a target is always a string to be parsed with the url module.

@pmalek
Copy link

pmalek commented Feb 1, 2014

@jcrugzz I have changed my options to look like this :

var options = {
  target: "https://localhost",
  ssl: {
    key: fs.readFileSync('./ssl/key.pem'),
    cert: fs.readFileSync('./ssl/cert.pem')
  },
  ws: true,
  secure: true,
  xfwd: true
};

and my createServer to

var proxy = httpProxy.createServer(options);

( I have tried to remove the ws, secure and xfwd options) and now I do not get the aforementioned error but the page on localhost:80 keeps loading as in an infinite loop ( I have my server listening on port 443 and I can see the http-proxy being created on port 80)

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek Got it so there is a slight caveat here because of how the ssl options are used. See my response in #563. They are used as the server cert information in this case AND used as a client cert when trying to proxy. This is obviously not what you want as you are just running it as an https server that you want to proxy via http. This is not how it behaves. You may also want secure set to false as I'm assuming this is a self signed cert (but I'm not positive in how it behaves in regards to localhost so it may not throw an error).

Since I have not figured out a clean way to make this more clear without making sslServer and sslClient options. I would just create a standard https server and use httpProxy as just a proxy (so you dont call the listen function).

@pmalek
Copy link

pmalek commented Feb 1, 2014

@jcrugzz Can you provide a gist how should I use it as I do not really understand what would be there for me if I throw away .listen(80); from it ?

BTW: I already have an https server, that's what I need this proxy to redirect all of the clients to go from localhost:80 to localhost:443.

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek ok lets back up a second, I misunderstood the use case. I thought you were proxying the other way around for whatever reason. What is the exact purpose of your proxy in this case? Until then I'll take a stab at what you MIGHT want.

var http = require('http');
var HttpProxy = require('http-proxy');
var Agent = http.Agent;

var options = {
  target: "https://localhost",
  ssl: {
    key: fs.readFileSync('./ssl/key.pem'),
    cert: fs.readFileSync('./ssl/cert.pem')
  },
  secure: true,
  xfwd: true
};

//
// Note this is the same as HttpProxy.createServer(options) etc..
// It only becomes a "server" if you call listen, otherwise its just a proxy EventEmitter.
//
var proxy = new HttpProxy(options);

//
// So since we are using the certificate for making CLIENT requests, we need to make an `http`
// server, otherwise we assume `https` and use the certs for the proxy server as well.
//
var server = http.createServer(function (req, res) {
  proxy.web(req, res);
});

server.on('upgrade', function(req, socket, head) {
  proxy.ws(req, socket, head);
};

server.listen(80);

@pmalek
Copy link

pmalek commented Feb 1, 2014

@jcrugzz My exact intentions are as follows:

I have a working sails web server on port 443 using https. I would like to make a proxy (redirect clients coming on port 80) to force usage of https at port 443.

So I thought using a node-httpp-proxy which would redirect all traffic from port 80 to 443 is a good idea.

I have tried your solution and I have the following error :

/home/USER/programming/node/test1/node_modules/http-proxy/lib/http-proxy/index.js:119
    throw err;
          ^
Error: DEPTH_ZERO_SELF_SIGNED_CERT
    at SecurePair.<anonymous> (tls.js:1370:32)
    at SecurePair.EventEmitter.emit (events.js:92:17)
    at SecurePair.maybeInitFinished (tls.js:982:10)
    at CleartextStream.read [as _read] (tls.js:469:13)
    at CleartextStream.Readable.read (_stream_readable.js:320:10)
    at EncryptedStream.write [as _write] (tls.js:366:25)
    at doWrite (_stream_writable.js:223:10)
    at writeOrBuffer (_stream_writable.js:213:5)
    at EncryptedStream.Writable.write (_stream_writable.js:180:11)
    at write (_stream_readable.js:583:24)
    at flow (_stream_readable.js:592:7)
    at Socket.pipeOnReadable (_stream_readable.js:624:5)
    at Socket.EventEmitter.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:408:10)
    at emitReadable (_stream_readable.js:404:5)
    at readableAddChunk (_stream_readable.js:165:9)
    at Socket.Readable.push (_stream_readable.js:127:10)
    at TCP.onread (net.js:526:21)

After that I have tried to change secure: true to secure: false and now it works but! on http. So I can access my server on both http (port 80) and https (port 443).

Yet still I would like to redirect clients to https while connecting to port 80.

I have found this connected case request/request#418 and SO question https://stackoverflow.com/questions/14088787/hostname-ip-doesnt-match-certificates-altname which suggest to use

NODE_TLS_REJECT_UNAUTHORIZED=0

or

{rejectUnauthorized:false}

while passing arguments to tls server but those didn't work for me (still the error).

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek the problem is what I outlined in #563 regarding self signed certs. Since you were supplying the ssl credentials I thought this wouldn't be an issue but I was wrong ;). You need an agent in order for secure: false to work (as it uses rejectUnauthorized: false under the hood).

var http = require('http');
var HttpProxy = require('http-proxy');
var Agent = http.Agent;

var options = {
  target: "https://localhost",
  ssl: {
    key: fs.readFileSync('./ssl/key.pem'),
    cert: fs.readFileSync('./ssl/cert.pem')
  },
  secure: false,
  xfwd: true,
  agent: new Agent({ maxSockets: Infinity })
};

//
// Note this is the same as HttpProxy.createServer(options) etc..
// It only becomes a "server" if you call listen, otherwise its just a proxy EventEmitter.
//
var proxy = new HttpProxy(options);

//
// So since we are using the certificate for making CLIENT requests, we need to make an `http`
// server, otherwise we assume `https` and use the certs for the proxy server as well.
//
var server = http.createServer(function (req, res) {
  proxy.web(req, res);
});

server.on('upgrade', function(req, socket, head) {
  proxy.ws(req, socket, head);
});

server.listen(80);

@pmalek
Copy link

pmalek commented Feb 1, 2014

@jcrugzz I get this "loop" (no response/timeout) again on port 80.

So let's summarize:

  • with secure: false I can access the website both on port 80 (using http) and 443 (using https),
  • using secure: true I get Error: DEPTH_ZERO_SELF_SIGNED_CERT on port 80 but can access website through https on port 443 without any problems
  • with agent: new Agent({ maxSockets: Infinity }) and no matter what secure is set to I get loop on port 80 and service ok on 443

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 1, 2014

@pmalek hmm ok, my mistake here is that this agent should be an require('https').Agent not an `http.Agent in this case. Give that a try but you should be at a point where this is debuggable.

@pmalek
Copy link

pmalek commented Feb 2, 2014

@jcrugzz This still gave me the same result (or DEPTH_ZERO_SELF_SIGNED_CERT) and I have decided to use https://npmjs.org/package/express-force-ssl for this purpose but still I would like to get that running (for different ports etc.)

Maybe some other time.

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 2, 2014

@pmalek Ensure you still have secure: false. There is some odd behavior in this area which is why 0.12.x will have a refactored http. Unless Im misunderstanding something about the behavior of HTTP -> HTTPS proxying, this should work. That module looks like your best bet for now.

@blairn
Copy link

blairn commented Feb 10, 2014

I'm still getting the same error from the simplest of examples.

W20140210-20:08:19.146(13)? (STDERR)     var proxyReq = (options.target.protocol === 'https:' ? https : http).reque...
W20140210-20:08:19.147(13)? (STDERR)                                   ^
W20140210-20:08:19.152(13)? (STDERR) TypeError: Cannot read property 'protocol' of undefined

is coming from

httpProxy.createServer({
  hostnameOnly: true,
  router: {
    'www.my-domain.com': '127.0.0.1:9000',
    'www.my-other-domain.de': '127.0.0.1:9001'
  }
}).listen(8090);

@jcrugzz
Copy link
Contributor Author

jcrugzz commented Feb 10, 2014

@blairn you are using an old api. And please create a new issue if you are having a problem using the new api :).

@blairn
Copy link

blairn commented Feb 10, 2014

Thanks!
Is there an example of a plain proxy? Just forwards the traffic onwards (not to a particular place)?
I can't find one in the example set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants