From eae3f9d34b90b9bf505709231582ada4a7c9d056 Mon Sep 17 00:00:00 2001 From: Sean McIntyre Date: Thu, 19 Oct 2017 21:52:52 -0400 Subject: [PATCH 1/2] Restricting certain kvstore methods. Providing clone() function for fresh connections. --- lib/core/module.js | 6 ++++ lib/core/routes.js | 6 ++++ lib/util/kvstore.js | 42 ++++++++++++++++++++-- test/util/test-kvstore.js | 76 +++++++++++++++++++++++++++++++++------ 4 files changed, 117 insertions(+), 13 deletions(-) diff --git a/lib/core/module.js b/lib/core/module.js index 17d5f852..83a25b25 100644 --- a/lib/core/module.js +++ b/lib/core/module.js @@ -213,6 +213,12 @@ class Module { * A reference to the internal Ravel key-value store connection (redis). * See [node-redis](https://github.com/NodeRedis/node_redis) for more information. * + * Since this is Ravel's own internal, long-lived connection, it is important that + * it not be blocked or suspended by calls to `exit`, `subcribe`, `psubscribe`, + * `unsubscribe` or `punsubscribe`. + * + * To retrieve an unrestricted connetion to Redis, use `kvstore.clone()`. + * * @type Object */ get kvstore () { diff --git a/lib/core/routes.js b/lib/core/routes.js index caa3d494..141edbdd 100644 --- a/lib/core/routes.js +++ b/lib/core/routes.js @@ -319,6 +319,12 @@ class Routes { * A reference to the internal Ravel key-value store connection (redis). * See [node-redis](https://github.com/NodeRedis/node_redis) for more information. * + * Since this is Ravel's own internal, long-lived connection, it is important that + * it not be blocked or suspended by calls to `exit`, `subcribe`, `psubscribe`, + * `unsubscribe` or `punsubscribe`. + * + * To retrieve an unrestricted connetion to Redis, use `kvstore.clone()`. + * * @type Object */ get kvstore () { diff --git a/lib/util/kvstore.js b/lib/util/kvstore.js index a039b416..eaa644f2 100644 --- a/lib/util/kvstore.js +++ b/lib/util/kvstore.js @@ -28,12 +28,28 @@ function retryStrategy (ravelInstance) { } /** - * Abstraction for redis-like data store. + * For disabling redis methods. + * + * @param {Object} client - Client to disable a method on. + * @param {string} fn - Name of the function to disable. + * @private + */ +function disable (client, fn) { + client[fn] = function () { + throw new ApplicationError.General( + `kvstore cannot use ${fn}(). Use kvstore.clone() to retrieve a fresh connection first.`); + }; +} + +/** + * Returns a fresh connection to Redis. * * @param {Ravel} ravelInstance - An instance of a Ravel app. + * @param {boolean} restrict - Iff true, disable `exit`, `subcribe`, `psubscribe`, `unsubscribe` and `punsubscribe`. + * @returns {Object} Returns a fresh connection to Redis. * @private */ -module.exports = function (ravelInstance) { +function createClient (ravelInstance, restrict = true) { const client = redis.createClient( ravelInstance.get('redis port'), ravelInstance.get('redis host'), @@ -55,7 +71,27 @@ module.exports = function (ravelInstance) { clearInterval(redisKeepaliveInterval); }); + if (restrict) { + disable(client, 'quit'); + disable(client, 'subscribe'); + disable(client, 'psubscribe'); + disable(client, 'unsubscribe'); + disable(client, 'punsubscribe'); + } + + client.clone = function () { + return createClient(ravelInstance, false); + }; + return client; -}; +} + +/** + * Abstraction for redis-like data store. + * + * @param {Ravel} ravelInstance - An instance of a Ravel app. + * @private + */ +module.exports = createClient; module.exports.retryStrategy = retryStrategy; diff --git a/test/util/test-kvstore.js b/test/util/test-kvstore.js index 307bd17b..4c7de802 100644 --- a/test/util/test-kvstore.js +++ b/test/util/test-kvstore.js @@ -5,7 +5,7 @@ const expect = chai.expect; chai.use(require('chai-things')); const mockery = require('mockery'); -let Ravel, redisClientStub, redisMock, coreSymbols; +let Ravel, coreSymbols; describe('Ravel', () => { beforeEach((done) => { @@ -15,20 +15,13 @@ describe('Ravel', () => { warnOnReplace: false, warnOnUnregistered: false }); - redisMock = { - createClient: () => { - redisClientStub = new (require('events').EventEmitter)(); // eslint-disable-line no-extra-parens - redisClientStub.auth = function () {}; - redisClientStub.end = function () {}; - return redisClientStub; - } - }; - mockery.registerMock('redis', redisMock); + mockery.registerMock('redis', require('redis-mock')); Ravel = new (require('../../lib/ravel'))(); coreSymbols = require('../../lib/core/symbols'); Ravel.log.setLevel(Ravel.log.NONE); Ravel.set('redis port', 0); Ravel.set('redis host', 'localhost'); + Ravel.set('redis keepalive interval', 1000); done(); }); @@ -102,5 +95,68 @@ describe('Ravel', () => { done(); }); }); + + describe('kvstore', () => { + let kvstore, clone; + beforeEach(() => { + Ravel[coreSymbols.parametersLoaded] = true; + kvstore = require('../../lib/util/kvstore')(Ravel, true); + }); + + it('should prevent use of quit()', () => { + expect(() => { + kvstore.quit(() => {}); + }).to.throw(Ravel.ApplicationError.General); + }); + + it('should prevent use of qsubscribeuit()', () => { + expect(() => { + kvstore.subscribe('chan'); + }).to.throw(Ravel.ApplicationError.General); + }); + + it('should prevent use of psubscribe()', () => { + expect(() => { + kvstore.psubscribe('chan'); + }).to.throw(Ravel.ApplicationError.General); + }); + + it('should prevent use of unsubscribe()', () => { + expect(() => { + kvstore.unsubscribe('chan'); + }).to.throw(Ravel.ApplicationError.General); + }); + + it('should prevent use of punsubscribe()', () => { + expect(() => { + kvstore.punsubscribe('chan'); + }).to.throw(Ravel.ApplicationError.General); + }); + + describe('#clone()', () => { + beforeEach(() => { + clone = kvstore.clone(); + }); + it('should support the use of quit()', () => { + expect(clone.quit).to.not.throw(Ravel.ApplicationError.General); + }); + + it('should support the use of qsubscribeuit()', () => { + expect(clone.subscribe).to.not.throw(Ravel.ApplicationError.General); + }); + + it('should support the use of psubscribe()', () => { + expect(clone.psubscribe).to.not.throw(Ravel.ApplicationError.General); + }); + + it('should support the use of unsubscribe()', () => { + expect(clone.unsubscribe).to.not.throw(Ravel.ApplicationError.General); + }); + + it('should support the use of punsubscribe()', () => { + expect(clone.punsubscribe).to.not.throw(Ravel.ApplicationError.General); + }); + }); + }); }); }); From 72966d0a17401a3efc6adcfc677ce9f68b1d5481 Mon Sep 17 00:00:00 2001 From: Sean McIntyre Date: Thu, 19 Oct 2017 21:53:49 -0400 Subject: [PATCH 2/2] Bumping version --- documentation.yml | 3 ++- package.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/documentation.yml b/documentation.yml index 9670e6f4..900870d7 100644 --- a/documentation.yml +++ b/documentation.yml @@ -1,6 +1,7 @@ name: Ravel API -version: 0.22.2 +version: 0.22.3 versions: + - 0.22.2 - 0.22.1 - 0.22.0 - 0.21.1 diff --git a/package.json b/package.json index a82be1f7..9a8c788f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ravel", - "version": "0.22.2", + "version": "0.22.3", "author": "Sean McIntyre ", "description": "Ravel Rapid Application Development Framework", "engines": {