routing broken with express server #215

Closed
kul opened this Issue Mar 27, 2012 · 13 comments

Comments

Projects
None yet
3 participants

kul commented Mar 27, 2012

I have following setup
$ node -v
v0.6.11
$ npm list
blah/blah/blee/nodex
├─┬ express@2.5.8
│ ├─┬ connect@1.8.5
│ │ └── formidable@1.0.9
│ ├── mime@1.2.4
│ ├── mkdirp@0.3.0
│ └── qs@0.4.2
├─┬ http-proxy@0.8.0
│ ├── colors@0.6.0-1
│ ├─┬ optimist@0.2.8
│ │ └── wordwrap@0.0.2
│ └── pkginfo@0.2.3
└─┬ socket.io@0.9.1-1
├── policyfile@0.0.4
├── redis@0.6.7
└─┬ socket.io-client@0.9.1-1
├─┬ active-x-obfuscator@0.0.1
│ └── zeparser@0.0.5
├── uglify-js@1.2.5
├─┬ ws@0.4.8
│ ├── commander@0.5.0
│ └── options@0.0.2
└── xmlhttprequest@1.2.2

Issue:

I have this simple express server

var express = require('express');
var app = express.createServer();
app.listen(8080);   
app.get('/',
    function(req,res)
    {
        res.send('ok!\n');
    }
  );

And following simple proxy.js

var  httpProxy = require('http-proxy');

var options =
{
    hostnameOnly: true,
    router:
    {
        'localhost:8081':'localhost:8080'
    }
};

var proxyServer = httpProxy.createServer(options);
proxyServer.listen(8081);

Just doesnt works!
$ curl -XGET http://localhost:8080/ -v
...
ok!
...
$ curl -XGET http://localhost:8081/ -v
blah blah...
NOT FOUND
..blah bleh
..

kul commented Mar 27, 2012

Do Something!
plz

Contributor

coderarity commented Mar 27, 2012

Don't include the port in the incoming routes. You've already specified this when you call proxyServer.listen(8081);

var  httpProxy = require('http-proxy');

var options =
{
    hostnameOnly: true,
    router:
    {
        'localhost':'localhost:8080'
    }
};

var proxyServer = httpProxy.createServer(options);
proxyServer.listen(8081);

kul commented Mar 28, 2012

OK Got it! Thanks , Before i close this please help me get this working too..

Server:

 var express = require('express');
 var app = express.createServer();
 app.listen(8181);
 app.get('/hello',
     function(req,res)
     {
         res.send('hi!\n');
     }
     );

and proxy:

 var fs = require('fs'),
 httpProxy = require('http-proxy');

 var options =
{
    router:
    {
        'localhost/hello':'localhost:8181/hello'
    }
};

var proxyServer = httpProxy.createServer(options);
proxyServer.listen(8282);

The curl request "curl -XGET localhost:8282/hello -v" never returns!

Contributor

coderarity commented Mar 28, 2012

I see. ProxyTable actually cuts out the path segments, so it will only send a request to localhost:8181 (with no URL). At this point I'm not sure that this behavior is possible. Let me look into it a bit more.

Contributor

coderarity commented Mar 28, 2012

I'm working on making this function properly. It seems like someone just forgot the second half of the implementation.

Contributor

coderarity commented Mar 28, 2012

Ok - here's the problem with doing this. Since node-http-proxy supports both HTTP and WebSockets, you can't route to a path, since paths are invalid in WebSocket URLs. This is in the specification that you can read here.

Sorry, you shouldn't try to use node-http-proxy in this way. I hope this helped you understand!

Contributor

coderarity commented Mar 28, 2012

Waaaaaiiiiiit. I think I can still add this - give me a few minutes more.

Contributor

coderarity commented Mar 28, 2012

Yeah, I've got this working. That was harder than I thought it would be. Pull request for this incoming. A few things - I was wrong, WebSockets can have paths, I misread the specification. (Maybe I shouldn't be reading specifications anyways =D).

In any case, after the pull request is merged, this should work correctly. This is indeed a bug. Thanks for pointing it out!

kul commented Mar 29, 2012

Are you sure what you did was what you wanted!? you seems to be little confused.
Anyways so what i did should work now once your changes are pulled in?
Also what kind of fix was it..you added http:// to target location...what about if it was https or ws or wss?
and req.url = url.format(requrl);sour <--??!

Contributor

coderarity commented Mar 29, 2012

I explained adding the http:// in a comment - it's just used to make url.parse work correctly. It doesn't actually get used in the final URL.

url.format returns a string from a url object - see the Node.js documentation.

It did take me a few tries to get this right, mostly because it was hard to understand what the old code was actually doing. =P

On Thursday, March 29, 2012 at 12:24 AM, kul wrote:

Are you sure what you did was what you wanted!? you seems to be little confused.
Anyways so whats i did should work now once your changes are pulled in?
Also what kind of fix was it..you added http:// to target location...what about if it was https or ws or wss?
and req.url = url.format(requrl);sour <--??!


Reply to this email directly or view it on GitHub:
#215 (comment)

Contributor

coderarity commented Mar 29, 2012

And yes - it will work after the pull request is merged and the package is published to npm.

On Thursday, March 29, 2012 at 6:34 AM, Christian Howe wrote:

I explained adding the http:// in a comment - it's just used to make url.parse work correctly. It doesn't actually get used in the final URL.

url.format returns a string from a url object - see the Node.js documentation.

It did take me a few tries to get this right, mostly because it was hard to understand what the old code was actually doing. =P

On Thursday, March 29, 2012 at 12:24 AM, kul wrote:

Are you sure what you did was what you wanted!? you seems to be little confused.
Anyways so whats i did should work now once your changes are pulled in?
Also what kind of fix was it..you added http:// to target location...what about if it was https or ws or wss?
and req.url = url.format(requrl);sour <--??!


Reply to this email directly or view it on GitHub:
#215 (comment)

Contributor

cronopio commented Jun 5, 2012

The 0.8.1 version is out, @kul can you test again please?

kul commented Jun 6, 2012

Tested. OK.

@kul kul closed this Jun 6, 2012

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