Merge pull request #49 from chimurai/proxyTable-does-not-work-for-web…

…socket-connections feat(proxyTable): add proxyTable support for ws
chimurai · Jan 26, 2016 · 6cfd0f3 · 6cfd0f3
2 parents fe24b97 + bb28503
commit 6cfd0f3
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 21 deletions.
diff --git a/examples/websocket/index.html b/examples/websocket/index.html
@@ -29,7 +29,7 @@ <h2>WebSocket demo</h2>
     <fieldset id="configuration">
         <p>
             <label for="location">location:</label>
-            <input id="location" type="text" disabled="disabled" value="ws://localhost:3000">
+            <input id="location" type="text" value="ws://localhost:3000">
             <button id="connect">connect</button>
             <button id="disconnect" disabled="disabled">disconnect</button>
         </p>
@@ -66,7 +66,7 @@ <h2>WebSocket demo</h2>
             disconnect.disabled = false;
             messaging.disabled = false;
 
-            socket = new WebSocket('ws://localhost:3000');
+            socket = new WebSocket(location.value);
             socket.onopen    = function () { log('CONNECTED'); };
             socket.onclose   = function () { log('DISCONNECTED'); };
             socket.onerror   = function () { log('SOCKET ERROR OCCURED'); };

diff --git a/index.js b/index.js
@@ -58,19 +58,10 @@ var httpProxyMiddleware = function (context, opts) {
         if (contextMatcher.match(config.context, req.url)) {
             logger.debug('[HPM] Context match: "%s" -> "%s"', config.context, req.url);
 
-            // handle option.pathRewrite
-            if (pathRewriter) {
-                req.url = pathRewriter(req.url);
-            }
-            if (proxyOptions.proxyTable) {
-                // change option.target when proxyTable present.
-                var altOpts = ProxyTable.createProxyOptions(req, proxyOptions);
-                logger.debug('[HPM] Proxying "%s": "%s" -> "%s"', req.url, req.hostname, altOpts.target);
-                proxy.web(req, res, altOpts);
-            } else {
-                logger.debug('[HPM] Proxying "%s": "%s" -> "%s"', req.url, req.hostname, proxyOptions.target);
-                proxy.web(req, res);
-            }
+            var activeProxyOptions = __prepareProxyRequest(req);
+
+            logger.debug('[HPM] Proxying "%s": "%s" -> "%s"', req.url, req.hostname, activeProxyOptions.target);
+            proxy.web(req, res, activeProxyOptions);
 
         } else {
             next();
@@ -93,14 +84,49 @@ var httpProxyMiddleware = function (context, opts) {
 
     function handleUpgrade (req, socket, head) {
         if (contextMatcher.match(config.context, req.url)) {
-            if (pathRewriter) {
-                req.url = pathRewriter(req.url);
-            }
-            proxy.ws(req, socket, head);
+
+            var activeProxyOptions = __prepareProxyRequest(req);
+
+            proxy.ws(req, socket, head, activeProxyOptions);
             logger.info('[HPM] Upgrading to WebSocket');
         }
     }
 
+    /**
+     * Apply option.proxyTable and option.pathRewrite
+     * Order matters:
+          ProxyTable uses original path for routing;
+          NOT the modified path, after it has been rewritten by pathRewrite
+     */
+    function __prepareProxyRequest(req) {
+        // apply option.proxyTable
+        var alteredProxyOptions = __applyProxyTableOption(req, proxyOptions);
+
+        // apply option.pathRewrite
+        __applyPathRewrite(req, pathRewriter);
+
+        return alteredProxyOptions;
+    }
+
+    // Modify option.target when proxyTable present.
+    // return altered options
+    function __applyProxyTableOption (req) {
+        var result = proxyOptions;
+
+        if (proxyOptions.proxyTable) {
+            result = ProxyTable.createProxyOptions(req, proxyOptions);
+        }
+
+        return result;
+    }
+
+    // rewrite path
+    function __applyPathRewrite (req) {
+        if (pathRewriter) {
+            req.url = pathRewriter(req.url);
+        }
+    }
+
     function getProxyErrorHandler () {
         if (_.isFunction(proxyOptions.onError)) {
             return proxyOptions.onError;   // custom error listener
@@ -111,7 +137,7 @@ var httpProxyMiddleware = function (context, opts) {
 
     function proxyErrorLogger (err, req, res) {
         var hostname = (req.hostname || req.host) || (req.headers && req.headers.host) // (node0.10 || node 4/5) || (websocket)
-        var targetUri = proxyOptions.target.host + req.url;
+        var targetUri = (proxyOptions.target.host || proxyOptions.target) + req.url;
 
         logger.error('[HPM] Proxy error: %s. %s -> "%s"', err.code, hostname, targetUri);
     }

diff --git a/test/websocket.spec.js b/test/websocket.spec.js
@@ -106,12 +106,40 @@ describe('WebSocket proxy', function () {
         });
     });
 
+    describe('with proxyTable and pathRewrite', function () {
+
+        beforeEach(function () {
+            proxyServer.close();
+            // override
+            proxy = proxyMiddleware('ws://notworkinghost:6789', {proxyTable: {'/socket': 'ws://localhost:8000'}, pathRewrite: {'^/socket' : ''}});
+            proxyServer = createServer(3000, proxy);
+        });
+
+        beforeEach(function (done) {
+            proxyServer.on('upgrade', proxy.upgrade);
+
+            ws = new WebSocket('ws://localhost:3000/socket');
+
+            ws.on('message', function incoming(message) {
+                responseMessage = message;
+                done();
+            });
+
+            ws.on('open', function open() {
+              ws.send('foobar');
+            });
+        });
+
+        it('should proxy to path', function () {
+            expect(responseMessage).to.equal('foobar');
+        });
+    });
+
     afterEach(function () {
         proxyServer.close();
         wss.close();
         ws = null;
     });
-
 });
 
 function createServer (portNumber, middleware) {