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

routing broken with express server #215

Closed
kul opened this issue Mar 27, 2012 · 13 comments · Fixed by #218
Closed

routing broken with express server #215

kul opened this issue Mar 27, 2012 · 13 comments · Fixed by #218

Comments

@kul
Copy link

kul commented Mar 27, 2012

I have following setup
$ node -v
v0.6.11
$ npm list
blah/blah/blee/nodex
├─┬ [email protected]
│ ├─┬ [email protected]
│ │ └── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ └── [email protected]
├─┬ [email protected]
│ ├── [email protected]
│ ├─┬ [email protected]
│ │ └── [email protected]
│ └── [email protected]
└─┬ [email protected]
├── [email protected]
├── [email protected]
└─┬ [email protected]
├─┬ [email protected]
│ └── [email protected]
├── [email protected]
├─┬ [email protected]
│ ├── [email protected]
│ └── [email protected]
└── [email protected]

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
Copy link
Author

kul commented Mar 27, 2012

Do Something!
plz

@coderarity
Copy link
Contributor

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
Copy link
Author

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!

@coderarity
Copy link
Contributor

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.

@coderarity
Copy link
Contributor

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

@coderarity
Copy link
Contributor

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!

@coderarity
Copy link
Contributor

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

@coderarity
Copy link
Contributor

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
Copy link
Author

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 <--??!

@coderarity
Copy link
Contributor

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)

@coderarity
Copy link
Contributor

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)

@cronopio
Copy link
Contributor

cronopio commented Jun 5, 2012

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

@kul
Copy link
Author

kul commented Jun 6, 2012

Tested. OK.

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

Successfully merging a pull request may close this issue.

3 participants