From 41865c2b49aae7ce92443fb65005ee3670b62629 Mon Sep 17 00:00:00 2001 From: Stephen Cresswell <229672+cressie176@users.noreply.github.com> Date: Fri, 8 Mar 2024 08:46:57 +0000 Subject: [PATCH 1/2] Decode connection url --- lib/config/configure.js | 2 +- test/config.tests.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/config/configure.js b/lib/config/configure.js index 5386a7e..3e4b720 100644 --- a/lib/config/configure.js +++ b/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, }; } diff --git a/test/config.tests.js b/test/config.tests.js index 4ec4bec..ad7808e 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -80,6 +80,23 @@ describe('Configuration', () => { ); }); + it('should support encoded urls', () => { + // See https://www.rabbitmq.com/docs/uri-spec#appendix-a-examples + configure( + { + vhosts: { + v1: { + connection: 'amqp://encoded%23user:encoded%23password@ho%61stname:9000/v%2fhost?heartbeat=10&channelMax=100', + }, + }, + }, + (err, config) => { + assert.ifError(err); + assert.strictEqual(config.vhosts.v1.connections[0].url, 'amqp://encoded%23user:encoded%23password@hoastname:9000/v/host?heartbeat=10&channelMax=100'); + }, + ); + }); + it('should report invalid urls', () => { configure( { From a68704f9783c5517462660b7f1555709056eeac3 Mon Sep 17 00:00:00 2001 From: Stephen Cresswell <229672+cressie176@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:06:47 +0000 Subject: [PATCH 2/2] Require broker and management urls to be encoded --- CHANGELOG.md | 5 ++ README.md | 9 ++-- lib/config/configure.js | 13 +++-- test/config.tests.js | 107 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 124 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f07699d..8b5fcba 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/README.md b/README.md index 9e43ea4..bcd668d 100644 --- a/README.md +++ b/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 { diff --git a/lib/config/configure.js b/lib/config/configure.js index 3e4b720..dea2716 100644 --- a/lib/config/configure.js +++ b/lib/config/configure.js @@ -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, }); diff --git a/test/config.tests.js b/test/config.tests.js index ad7808e..d0a2306 100644 --- a/test/config.tests.js +++ b/test/config.tests.js @@ -86,13 +86,42 @@ describe('Configuration', () => { { vhosts: { v1: { - connection: 'amqp://encoded%23user:encoded%23password@ho%61stname:9000/v%2fhost?heartbeat=10&channelMax=100', + 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%23user:encoded%23password@hoastname:9000/v/host?heartbeat=10&channelMax=100'); + 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'); }, ); }); @@ -481,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( { @@ -553,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( {