From d4bf2a147b7ab41e2f148a30b7baa5083b4f9745 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Apr 2016 16:08:07 -0700 Subject: [PATCH] Make paths and Aphlict instance names less ambiguous Summary: Fixes T10783 (what little of it remains). Ref T10697. Aphlict currently uses request paths for two different things: - multi-tenant instancing in the Phacility cluster (each instance gets its own namespace within an Aphlict server); - some users configure nginx and apache to do proxying or SSL termination based on the path. Currently, these can collide. Put a "~" before the instance name to make it unambiguous. At some point we can possibly just use a GET parameter, but I think there was some reason I didn't do that originally and this sequence of changes is disruptive enough already. Test Plan: Saw local Aphlict unambiguously recognize "local.phacility.com" as instance "local", with a "~"-style URI. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10697, T10783 Differential Revision: https://secure.phabricator.com/D15705 --- .../PhabricatorNotificationServerRef.php | 2 +- .../aphlict/server/lib/AphlictClientServer.js | 27 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/applications/notification/client/PhabricatorNotificationServerRef.php b/src/applications/notification/client/PhabricatorNotificationServerRef.php index cfdbb5e8eb..b183221eee 100644 --- a/src/applications/notification/client/PhabricatorNotificationServerRef.php +++ b/src/applications/notification/client/PhabricatorNotificationServerRef.php @@ -162,7 +162,7 @@ final class PhabricatorNotificationServerRef public function getWebsocketURI($to_path = null) { $instance = PhabricatorEnv::getEnvConfig('cluster.instance'); if (strlen($instance)) { - $to_path = $to_path.$instance.'/'; + $to_path = $to_path.'~'.$instance.'/'; } $uri = $this->getURI($to_path); diff --git a/support/aphlict/server/lib/AphlictClientServer.js b/support/aphlict/server/lib/AphlictClientServer.js index 4d173171c2..1d4375cbba 100644 --- a/support/aphlict/server/lib/AphlictClientServer.js +++ b/support/aphlict/server/lib/AphlictClientServer.js @@ -52,18 +52,33 @@ JX.install('AphlictClientServer', { response.end('HTTP/501 Use Websockets\n'); }, + _parseInstanceFromPath: function(path) { + // If there's no "~" marker in the path, it's not an instance name. + // Users sometimes configure nginx or Apache to proxy based on the + // path. + if (path.indexOf('~') === -1) { + return 'default'; + } + + var instance = path.split('~')[1]; + + // Remove any "/" characters. + instance = instance.replace(/\//g, ''); + if (!instance.length) { + return 'default'; + } + + return instance; + }, + listen: function() { var self = this; var server = this._server.listen.apply(this._server, arguments); var wss = new WebSocket.Server({server: server}); wss.on('connection', function(ws) { - var instance = url.parse(ws.upgradeReq.url).pathname; - - instance = instance.replace(/\//g, ''); - if (!instance.length) { - instance = 'default'; - } + var path = url.parse(ws.upgradeReq.url).pathname; + var instance = self._parseInstanceFromPath(path); var listener = self.getListenerList(instance).addListener(ws);