diff --git a/README.md b/README.md index 7cc110f0a1..42857e52e8 100644 --- a/README.md +++ b/README.md @@ -211,6 +211,7 @@ limits total amount of connection tries. Setting this to 1 will prevent any reco * `auth_pass`: *null*; If set, client will run redis auth command on connect. * `family`: *IPv4*; You can force using IPv6 if you set the family to 'IPv6'. See Node.js [net](https://nodejs.org/api/net.html) or [dns](https://nodejs.org/api/dns.html) modules how to use the family type. * `disable_resubscribing`: *false*; If set to `true`, a client won't resubscribe after disconnecting +* `to_empty_string`: *null*; convert any undefined or null command argument to an empty string. Be careful using this * `rename_commands`: *null*; pass a object with renamed commands to use those instead of the original functions. See the [redis security topics](http://redis.io/topics/security) for more info. * `tls`: an object containing options to pass to [tls.connect](http://nodejs.org/api/tls.html#tls_tls_connect_port_host_options_callback), to set up a TLS connection to Redis (if, for example, it is set up to be accessible via a tunnel). diff --git a/index.js b/index.js index 3043a55627..179902aedf 100644 --- a/index.js +++ b/index.js @@ -719,39 +719,49 @@ RedisClient.prototype.send_command = function (command, args, callback) { callback = process.domain.bind(callback); } - if (command === 'set' || command === 'setex') { - // if the value is undefined or null and command is set or setx, need not to send message to redis - if (args[args.length - 1] === undefined || args[args.length - 1] === null) { - command = command.toUpperCase(); - err = new Error('send_command: ' + command + ' value must not be undefined or null'); - err.command = command; - this.callback_emit_error(callback, err); - // Singal no buffering - return true; - } - } - for (i = 0; i < args.length; i += 1) { - if (Buffer.isBuffer(args[i])) { + if (typeof args[i] === 'string') { + // 30000 seemed to be a good value to switch to buffers after testing and checking the pros and cons + if (args[i].length > 30000) { + big_data = true; + args[i] = new Buffer(args[i], 'utf8'); + if (this.pipeline !== 0) { + this.pipeline += 2; + this.writeDefault = this.writeBuffers; + } + } + } else if (args[i] === null) { + if (!this.options.to_empty_string) { // > 2 // ( 2 + 3 where 3 stands for both, null and undefined and 3 for null) + console.warn( + 'node_redis: Deprecated: The %s command contains a "null" argument.\n' + + 'This is converted to a "null" string now and will return an error from v.3.0 on.\n' + + 'If you wish to convert null to an empty string instead, please use the "to_empty_string" option.', command.toUpperCase() + ); + args[i] = 'null'; // Backwards compatible :/ + } else { + args[i] = ''; + } + } else if (typeof args[i] === 'object') { // Buffer.isBuffer(args[i])) { buffer_args = true; if (this.pipeline !== 0) { this.pipeline += 2; this.writeDefault = this.writeBuffers; } - } else if (typeof args[i] !== 'string') { - args[i] = String(args[i]); - // 30000 seemed to be a good value to switch to buffers after testing and checking the pros and cons - } else if (args[i].length > 30000) { - big_data = true; - args[i] = new Buffer(args[i]); - if (this.pipeline !== 0) { - this.pipeline += 2; - this.writeDefault = this.writeBuffers; + } else if (args[i] === undefined) { + if (!this.options.to_empty_string) { + console.warn( + 'node_redis: Deprecated: The %s command contains a "undefined" argument.\n' + + 'This is converted to a "undefined" string now and will return an error from v.3.0 on.\n' + + 'If you wish to convert undefined to an empty string instead, please use the "to_empty_string" option.', command.toUpperCase() + ); + args[i] = 'undefined'; // Backwards compatible :/ + } else { + args[i] = ''; } } } - command_obj = new Command(command, args, false, buffer_args, callback); + command_obj = new Command(command, args, buffer_args, callback); if (!this.ready && !this.send_anyway || !stream.writable) { if (this.closing || !this.enable_offline_queue) { @@ -812,9 +822,9 @@ RedisClient.prototype.send_command = function (command, args, callback) { for (i = 0; i < args.length; i += 1) { arg = args[i]; - if (typeof arg === 'string') { + if (typeof arg !== 'object') { // string; number; boolean this.write('$' + Buffer.byteLength(arg) + '\r\n' + arg + '\r\n'); - } else { + } else { // buffer this.write('$' + arg.length + '\r\n'); this.write(arg); this.write('\r\n'); diff --git a/lib/command.js b/lib/command.js index 4aa795b696..d04052ff6d 100644 --- a/lib/command.js +++ b/lib/command.js @@ -2,10 +2,9 @@ // This Command constructor is ever so slightly faster than using an object literal, but more importantly, using // a named constructor helps it show up meaningfully in the V8 CPU profiler and in heap snapshots. -function Command(command, args, sub_command, buffer_args, callback) { +function Command(command, args, buffer_args, callback) { this.command = command; this.args = args; - this.sub_command = sub_command; this.buffer_args = buffer_args; this.callback = callback; } diff --git a/test/commands/incr.spec.js b/test/commands/incr.spec.js index 2f7daae0d1..1910bd2943 100644 --- a/test/commands/incr.spec.js +++ b/test/commands/incr.spec.js @@ -43,7 +43,6 @@ describe("The 'incr' method", function () { describe("when connected and a value in Redis", function () { var client; - // Also, why tf were these disabled for hiredis? They work just fine. before(function (done) { /* 9007199254740992 -> 9007199254740992 diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js index 7a508920a1..4868ed06e6 100644 --- a/test/commands/set.spec.js +++ b/test/commands/set.spec.js @@ -113,7 +113,7 @@ describe("The 'set' method", function () { describe("with undefined 'key' and missing 'value' parameter", function () { it("emits an error without callback", function (done) { client.on('error', function (err) { - assert.equal(err.message, 'send_command: SET value must not be undefined or null'); + assert.equal(err.message, "ERR wrong number of arguments for 'set' command"); assert.equal(err.command, 'SET'); done(); }); @@ -132,13 +132,10 @@ describe("The 'set' method", function () { it("emit an error without any parameters", function (done) { client.once("error", function (err) { - assert.equal(err.message, 'send_command: SET value must not be undefined or null'); + assert.equal(err.message, "ERR wrong number of arguments for 'set' command"); assert.equal(err.command, 'SET'); done(); }); - - // This was not supported not to throw earlier and was added by the test refactoring - // https://github.com/NodeRedis/node_redis/commit/eaca486ab1aecd1329f7452ad2f2255b1263606f client.set(); }); }); diff --git a/test/commands/setex.spec.js b/test/commands/setex.spec.js index 47f616253e..575c7e854d 100644 --- a/test/commands/setex.spec.js +++ b/test/commands/setex.spec.js @@ -21,18 +21,14 @@ describe("The 'setex' method", function () { it('sets a key with an expiry', function (done) { client.setex(["setex key", "100", "setex val"], helper.isString("OK")); - client.exists(["setex key"], helper.isNumber(1)); + var buffering = client.exists(["setex key"], helper.isNumber(1)); + assert(typeof buffering === 'boolean'); client.ttl(['setex key'], function (err, ttl) { - assert.ok(ttl > 0); + assert(ttl > 0); return done(); }); }); - it('returns an error if no value is provided', function (done) { - var buffering = client.SETEX(["setex key", "100", undefined], helper.isError(done)); - assert(typeof buffering === 'boolean'); - }); - afterEach(function () { client.end(true); }); diff --git a/test/connection.spec.js b/test/connection.spec.js index 1ce5b61698..9e98c3732e 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -90,7 +90,7 @@ describe("connection tests", function () { }); client.on("reconnecting", function (params) { - client.end(); + client.end(true); setTimeout(done, 100); }); });