Browse files

[64] Exposes raw memcache response for empty values

Added test case and fix for setting and getting empty strings
  • Loading branch information...
1 parent 132be99 commit 5677ee84d6c314d2fc3d2dd0282f71aeecf054f1 @jkrems jkrems committed Oct 20, 2012
Showing with 55 additions and 3 deletions.
  1. +10 −1 lib/memcached.js
  2. +3 −0 package.json
  3. +15 −2 tests/common.js
  4. +27 −0 tests/memcached-get-set.test.js
View
11 lib/memcached.js
@@ -404,13 +404,22 @@ Client.config = {
, 'VALUE': function value(tokens, dataSet, err, queue) {
var key = tokens[1]
, flag = +tokens[2]
- , expire = tokens[3]
+ , dataLen = tokens[3] // length of dataSet in raw bytes
, cas = tokens[4]
, multi = this.metaData[0] && this.metaData[0].multi || cas
? {}
: false
, tmp;
+ // In parse data there is an '||' passing us the content of token
+ // if dataSet is empty. This may be fine for other types of responses,
+ // in the case of an empty string being stored in a key, it will
+ // result in unexpected behavior:
+ // https://github.com/3rd-Eden/node-memcached/issues/64
+ if (dataLen === '0') {
+ dataSet = '';
+ }
+
switch (flag) {
case FLAG_JSON:
dataSet = JSON.parse(dataSet);
View
3 package.json
@@ -41,4 +41,7 @@
"mocha": "*"
, "should": "*"
}
+ , "scripts": {
+ "test": "./node_modules/.bin/mocha tests/"
+ }
}
View
17 tests/common.js
@@ -1,14 +1,27 @@
+
+/**
+ * Make should available in all test cases.
+ */
+var should = require('should');
+
/**
* Server ip addresses that get used during the tests
* NOTE! Make sure you configure empty servers as they
* will get flushed!.
+ *
+ * If your memcache hosts is not the default one
+ * (10.211.55.5), you can pass another one using the
+ * environment variable MEMCACHED__HOST. E.g.:
+ *
+ * MEMCACHED__HOST=localhost npm test
*
* @type {Object}
* @api public
*/
+var testMemcachedHost = process.env['MEMCACHED__HOST'] || '10.211.55.5';
exports.servers = {
- single: '10.211.55.5:11211'
-, multi: ['10.211.55.5:11211', '10.211.55.5:11212', '10.211.55.5:11213']
+ single: testMemcachedHost + ':11211'
+, multi: [testMemcachedHost + ':11211', testMemcachedHost + ':11212', testMemcachedHost + ':11213']
};
/**
View
27 tests/memcached-get-set.test.js
@@ -47,6 +47,33 @@ describe("Memcached GET SET", function() {
});
});
+ it("set and get an empty string", function(done) {
+ var memcached = new Memcached(common.servers.single)
+ , testnr = ++global.testnumbers
+ , callbacks = 0;
+
+ memcached.set("test:" + testnr, "", 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("");
+
+ memcached.end(); // close connections
+ assert.equal(callbacks, 2);
+ done();
+
+ });
+ });
+ });
+
/**
* Set a stringified JSON object, and make sure we only return a string
* this should not be flagged as JSON object

0 comments on commit 5677ee8

Please sign in to comment.