Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix for hashing long keys and spaces in key names #78

Closed
wants to merge 1 commit into from

3 participants

@crash2burn

I've fixed two issues with the validateArgs function having to do with invalid keys.

crash2burn Fixed bug where long keys are not hashed. Fixed bug where spaces in k…
…ey name result in the callback never being called.
c1da9b8
@3rd-Eden
Owner

Hey,

Thanks for fixing these bugs and for taking the time to build a test against them. I'm currently refactoring and rewriting the internals of the driver for the 1.0 release https://github.com/3rd-Eden/node-memcached/tree/1.0 i'll make sure that these changes will be included in that release.

I'm not sure when i'm going to release the module as it's taking a bit more time then I planned for it.. But I'll keep you in the loop on when it will be released and fixed.

@ronkorving
Collaborator

Will this fix also make it into pre-1.0 releases? (I'm sure you'll manage to push out one or two before the big 1.0 launch, right?)

@3rd-Eden
Owner

@ronkorving

I'll do an alpha and a beta before releasing 1.0

@3rd-Eden
Owner

Closing this as it's been merged in to master using a other pull request and released.

@3rd-Eden 3rd-Eden closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 29, 2013
  1. Fixed bug where long keys are not hashed. Fixed bug where spaces in k…

    crash2burn authored
    …ey name result in the callback never being called.
This page is out of date. Refresh to see the latest.
Showing with 52 additions and 4 deletions.
  1. +3 −4 lib/utils.js
  2. +49 −0 test/memcached-get-set.test.js
View
7 lib/utils.js
@@ -4,8 +4,7 @@ var createHash = require('crypto').createHash
, toString = Object.prototype.toString;
exports.validateArg = function validateArg (args, config) {
- var err
- , callback;
+ var err;
args.validate.forEach(function (tokens) {
var key = tokens[0]
@@ -58,7 +57,7 @@ exports.validateArg = function validateArg (args, config) {
args[key] = createHash('md5').update(value).digest('hex');
// also make sure you update the command
- args.command.replace(value, args[key]);
+ args.command = args.command.replace(value, args[key]);
} else {
err = 'Argument "' + key + '" is longer than the maximum allowed length of ' + config.maxKeySize;
}
@@ -76,7 +75,7 @@ exports.validateArg = function validateArg (args, config) {
});
if (err){
- if(callback) callback(err, false);
+ if(args.callback) args.callback(new Error(err));
return false;
}
View
49 test/memcached-get-set.test.js
@@ -538,4 +538,53 @@ describe("Memcached GET SET", function() {
});
});
});
+
+ /**
+ * Make sure long keys are hashed
+ */
+ it("make sure you can get really long strings", function(done) {
+ var memcached = new Memcached(common.servers.single)
+ , message = 'VALUE hello, I\'m not really a value.'
+ , testnr = "01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"+(++global.testnumbers)
+ , callbacks = 0;
+
+ memcached.set("test:" + testnr, message, 1000, function(error, ok){
+ ++callbacks;
+
+ assert.ok(!error);
+ ok.should.be.true;
+
+ memcached.get("test:" + testnr, function(error, answer){
+ ++callbacks;
+
+ assert.ok(!error);
+
+ assert.ok(typeof answer === 'string');
+ answer.should.eql(message);
+
+ memcached.end(); // close connections
+ assert.equal(callbacks, 2);
+ done();
+ });
+ });
+ });
+
+ /**
+ * Make sure keys with spaces return an error
+ */
+ it("errors on spaces in strings", function(done) {
+ var memcached = new Memcached(common.servers.single)
+ , message = 'VALUE hello, I\'m not really a value.'
+ , testnr = " "+(++global.testnumbers)
+ , callbacks = 0;
+
+ memcached.set("test:" + testnr, message, 1000, function(error, ok){
+ ++callbacks;
+
+ assert.ok(error);
+ assert.ok(error.message == 'The key should not contain any whitespace or new lines')
+
+ done();
+ });
+ });
});
Something went wrong with that request. Please try again.