Decode connection url (#229)

* Decode connection url

* Require broker and management urls to be encoded

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Change Log
 
+## Unreleased
+
+- Fixes https://github.com/onebeyond/rascal/issues/227 by requiring special characters to be URL encoded.
+- Consolidated broker and management url configuration logic
+
 ## 17.0.2
 
 - Update guidesmiths references to onebeyond.

README.md

@@ -47,9 +47,9 @@ Rascal extends the existing [RabbitMQ Concepts](https://www.rabbitmq.com/tutoria
 
 A **publication** is a named configuration for publishing a message, including the destination queue or exchange, routing configuration, encryption profile and reliability guarantees, message options, etc. A **subscription** is a named configuration for consuming messages, including the source queue, encryption profile, content encoding, delivery options (e.g. acknowledgement handling and prefetch), etc. These must be [configured](#configuration) and supplied when creating the Rascal broker. After the broker has been created the subscriptions and publications can be retrieved from the broker and used to publish and consume messages.
 
-### Breaking Changes in Rascal@14
+### Breaking Changes
 
-Rascal@14 waits for inflight messages to be acknowledged before closing subscriber channels. Prior to this version Rascal just waited an arbitrary amount of time. If your application does not acknowledge a message for some reason (quite likely in tests) calling `subscription.cancel`, `broker.unsubscribeAll`, `broker.bounce`, `broker.shutdown` or `broker.nuke` will wait indefinitely. You can specify a `closeTimeout` in your subscription config, however if this is exceeded the `subscription.cancel` and `broker.unsubscribeAll` methods will yield an error via callback or rejection, while the `broker.bounce`, `broker.shutdown` and `broker.nuke` methods will emit an error event, but attempt to continue. In both cases the error will have a code of `ETIMEDOUT`.
+Please refer to the [Change Log](https://github.com/onebeyond/rascal/blob/master/CHANGELOG.md)
 
 ### Special Note
 
@@ -318,6 +318,7 @@ The simplest way to specify a connection is with a url
   }
 }
 ```
+As of Rascal v18.0.0 you must URL encode special characters appearing in the username, password and vhost, e.g. `amqp://guest:secr%23t@broker.example.com:5672/v1?heartbeat=10`
 
 Alternatively you can specify the individual connection details
 
@@ -345,6 +346,8 @@ Alternatively you can specify the individual connection details
 }
 ```
 
+Special characters do not need to be encoded when specified in this form.
+
 Any attributes you add to the "options" sub document will be converted to query parameters. Any attributes you add in the "socketOptions" sub document will be passed directly to amqplib's connect method (which hands them off to `net` or `tls`. Providing you merge your configuration with the default configuration `rascal.withDefaultConfig(config)` you need only specify the attributes you want to override
 
 ```json
@@ -459,7 +462,7 @@ The AMQP protocol doesn't support assertion or checking of vhosts, so Rascal use
 }
 ```
 
-Rascal uses [superagent](https://github.com/visionmedia/superagent) under the hood. URL configuration is supported.
+Rascal uses [superagent](https://github.com/visionmedia/superagent) under the hood. URL configuration is also supported.
 
 ```json
 {

lib/config/configure.js

@@ -91,7 +91,7 @@ module.exports = _.curry((rascalConfig, next) => {
     } = new URL(connectionString);
     const options = Array.from(searchParams).reduce((attributes, entry) => ({ ...attributes, [entry[0]]: entry[1] }), {});
     return {
-      protocol, hostname, port, user, password, vhost, options,
+      protocol, hostname: decodeURIComponent(hostname), port, user: decodeURIComponent(user), password: decodeURIComponent(password), vhost: decodeURIComponent(vhost), options,
     };
   }
 
@@ -105,10 +105,14 @@ module.exports = _.curry((rascalConfig, next) => {
   }
 
   function configureManagementConnection(vhostConfig, vhostName, connection) {
-    _.defaultsDeep(connection.management, { hostname: connection.hostname });
-    const auth = connection.management.auth || getAuth(connection.management.user, connection.management.password) || getAuth(connection.user, connection.password);
-    connection.management.url = connection.management.url || url.format({ ...connection.management, auth });
-    connection.management.loggableUrl = connection.management.url.replace(/:[^:]*?@/, ':***@');
+    connection.management = _.isString(connection.management) ? { url: connection.management } : connection.management;
+    const attributesFromUrl = parseConnectionUrl(connection.management.url);
+    const attributesFromConfig = getConnectionAttributes(connection.management);
+    const defaults = { user: connection.user, password: connection.password, hostname: connection.hostname };
+
+    const connectionAttributes = _.defaultsDeep({ options: null }, attributesFromUrl, attributesFromConfig, defaults);
+    setConnectionAttributes(connection.management, connectionAttributes);
+    setConnectionUrls(connection.management);
   }
 
   function setConnectionAttributes(connection, attributes, defaults) {
@@ -120,7 +124,6 @@ module.exports = _.curry((rascalConfig, next) => {
     const auth = getAuth(connection.user, connection.password);
     const pathname = connection.vhost === '/' ? '' : connection.vhost;
     const query = connection.options;
-
     connection.url = url.format({
       slashes: true, ...connection, auth, pathname, query,
     });

test/config.tests.js

@@ -80,6 +80,52 @@ describe('Configuration', () => {
         );
       });
 
+      it('should support encoded urls', () => {
+        // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: 'amqp://encoded%2523user:encoded%23password@h%6fstname:9000/v%2fhost?heart%26beat=10&channelMax=100%3F',
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].url, 'amqp://encoded%2523user:encoded%23password@hostname:9000/v/host?heart%26beat=10&channelMax=100%3F');
+          },
+        );
+      });
+
+      it('should encode params', () => {
+        // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: {
+                  slashes: true,
+                  protocol: 'amqp',
+                  hostname: 'hostname',
+                  port: 9000,
+                  vhost: 'v/host',
+                  user: 'encoded%23user',
+                  password: 'encoded#password',
+                  options: {
+                    'heart&beat': 10,
+                    channelMax: '100?',
+                  },
+                },
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].url, 'amqp://encoded%2523user:encoded%23password@hostname:9000/v/host?heart%26beat=10&channelMax=100%3F');
+          },
+        );
+      });
+
       it('should report invalid urls', () => {
         configure(
           {
@@ -464,6 +510,62 @@ describe('Configuration', () => {
         );
       });
 
+      it('should configure the management url from the connection url', () => {
+        // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: 'amqp://user:password@hostname:9000/vhost?heartbeat=10&channelMax=100',
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].management.url, 'http://user:password@hostname:15672');
+          },
+        );
+      });
+
+      it('should configure the management connection from the management url', () => {
+        // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: {
+                  url: 'amqp://user:password@hostname:9000/vhost?heartbeat=10&channelMax=100',
+                  management: 'https://user1:password1@hostname1:9001',
+                },
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].management.url, 'https://user1:password1@hostname1:9001');
+          },
+        );
+      });
+
+      it('should support encoded management urls', () => {
+        // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: {
+                  management: 'https://encoded%2523user:encoded%23password@h%6Fstname:9001',
+                },
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].management.url, 'https://encoded%2523user:encoded%23password@hostname:9001');
+          },
+        );
+      });
+
       it('should configure the management connection from an object', () => {
         configure(
           {
@@ -536,6 +638,24 @@ describe('Configuration', () => {
         );
       });
 
+      it('should fully obscure the password in the management loggable url (a)', () => {
+        configure(
+          {
+            vhosts: {
+              v1: {
+                connection: {
+                  management: 'http://user:badp@assword@hostname',
+                },
+              },
+            },
+          },
+          (err, config) => {
+            assert.ifError(err);
+            assert.strictEqual(config.vhosts.v1.connections[0].management.loggableUrl, 'http://user:***@hostname');
+          },
+        );
+      });
+
       it('should generate a namespace when specified', () => {
         configure(
           {