Skip to content
This repository

fix empty unsub/punsub TypeError calling toString on null #408

Merged
merged 3 commits into from 12 months ago

2 participants

Jeff Barczewski Bryce Baril
Jeff Barczewski

When unsubscribe or punsubscribe is called and it has
no subscriptions, the reply[1] is a null which causes
TypeError: Cannot call method 'toString' of null

Failing tests are provided along with fix.

Fix is to check if reply[1] is null before calling toString otherwise
just pass null.

Stack trace for exception:

TypeError: Cannot call method 'toString' of null
    at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65)
    at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300
    at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22
    at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27)
    at Socket.<anonymous> (/Users/barczewskij/projects/node_redis/index.js:82:14)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:396:14)
added some commits March 26, 2013
Jeff Barczewski failing tests for empty unsub and punsub
When unsubscribe or punsubscribe is called
and there is nothing to unsubscribe from, the reply[1]
argument is a null which causes a TypeError
Cannot call method 'toString' of null

```
TypeError: Cannot call method 'toString' of null
    at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65)
    at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14)
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300:10)
    at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22)
    at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27)
    at Socket.<anonymous> (/Users/barczewskij/projects/node_redis/index.js:82:14)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:396:14)
```
0c143a7
Jeff Barczewski fix empty unsub/punsub TypeError
When unsubscribe or punsubscribe is called and it has
no subscriptions, the reply[1] is a null which causes
`TypeError: Cannot call method 'toString' of null`

Check if reply[1] is null before calling toString otherwise
just pass null.
655681f
Bryce Baril
Collaborator

Hi @jeffbski thanks for your submission!

For me (0.8.22) merging from master the last two of your four tests hang when I run node test -- any ideas?

Jeff Barczewski

Are you running the latest redis stable version 2.6.11?

The reason I became aware of the problem was due to changes from 2.5.11 to 2.6.11.

Bryce Baril
Collaborator

Looks like I'm running 2.6.9 where I tested it.

If this is redis-version specific we'll need a guard on the test at least. (see other tests that use server_version_at_least())

Jeff Barczewski limit cbtests to 2.6.11 and above
Test hangs on older versions of Redis
383bafd
Jeff Barczewski

ok, see if this is acceptable.

Bryce Baril brycebaril merged commit 383bafd into from April 27, 2013
Bryce Baril brycebaril closed this April 27, 2013
Bryce Baril
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 3 unique commits by 1 author.

Mar 26, 2013
Jeff Barczewski failing tests for empty unsub and punsub
When unsubscribe or punsubscribe is called
and there is nothing to unsubscribe from, the reply[1]
argument is a null which causes a TypeError
Cannot call method 'toString' of null

```
TypeError: Cannot call method 'toString' of null
    at RedisClient.return_reply (/Users/barczewskij/projects/node_redis/index.js:633:65)
    at ReplyParser.RedisClient.init_parser (/Users/barczewskij/projects/node_redis/index.js:266:14)
    at ReplyParser.EventEmitter.emit (events.js:96:17)
    at ReplyParser.send_reply (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:300:10)
    at ReplyParser.execute (/Users/barczewskij/projects/node_redis/lib/parser/javascript.js:211:22)
    at RedisClient.on_data (/Users/barczewskij/projects/node_redis/index.js:483:27)
    at Socket.<anonymous> (/Users/barczewskij/projects/node_redis/index.js:82:14)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:396:14)
```
0c143a7
Jeff Barczewski fix empty unsub/punsub TypeError
When unsubscribe or punsubscribe is called and it has
no subscriptions, the reply[1] is a null which causes
`TypeError: Cannot call method 'toString' of null`

Check if reply[1] is null before calling toString otherwise
just pass null.
655681f
Mar 27, 2013
Jeff Barczewski limit cbtests to 2.6.11 and above
Test hangs on older versions of Redis
383bafd
This page is out of date. Refresh to see the latest.

Showing 2 changed files with 50 additions and 4 deletions. Show diff stats Hide diff stats

  1. 8  index.js
  2. 46  test.js
8  index.js
@@ -629,10 +629,12 @@ RedisClient.prototype.return_reply = function (reply) {
629 629
                 }
630 630
                 // subscribe commands take an optional callback and also emit an event, but only the first response is included in the callback
631 631
                 // TODO - document this or fix it so it works in a more obvious way
  632
+                // reply[1] can be null
  633
+                var reply1String = (reply[1] === null) ? null : reply[1].toString();
632 634
                 if (command_obj && typeof command_obj.callback === "function") {
633  
-                    try_callback(command_obj.callback, reply[1].toString());
  635
+                    try_callback(command_obj.callback, reply1String);
634 636
                 }
635  
-                this.emit(type, reply[1].toString(), reply[2]); // channel, count
  637
+                this.emit(type, reply1String, reply[2]); // channel, count
636 638
             } else {
637 639
                 throw new Error("subscriptions are active but got unknown reply type " + type);
638 640
             }
@@ -708,7 +710,7 @@ RedisClient.prototype.send_command = function (command, args, callback) {
708 710
             return callback(err);
709 711
         }
710 712
     }
711  
-    
  713
+
712 714
     buffer_args = false;
713 715
     for (i = 0, il = args.length, arg; i < il; i += 1) {
714 716
         if (Buffer.isBuffer(args[i])) {
46  test.js
@@ -614,7 +614,7 @@ tests.detect_buffers = function () {
614 614
             assert.strictEqual("<Buffer 76 61 6c 20 31>", reply[0].inspect(), name);
615 615
             assert.strictEqual("<Buffer 76 61 6c 20 32>", reply[1].inspect(), name);
616 616
         });
617  
-        
  617
+
618 618
         // array of strings with undefined values (repro #344)
619 619
         detect_client.hmget("hash key 2", "key 3", "key 4", function(err, reply) {
620 620
             assert.strictEqual(null, err, name);
@@ -864,6 +864,50 @@ tests.SUBSCRIBE = function () {
864 864
     });
865 865
 };
866 866
 
  867
+tests.UNSUB_EMPTY = function () {
  868
+  // test situation where unsubscribe reply[1] is null
  869
+  var name = "UNSUB_EMPTY";
  870
+  client3.unsubscribe(); // unsubscribe from all so can test null
  871
+  client3.unsubscribe(); // reply[1] will be null
  872
+  next(name);
  873
+};
  874
+
  875
+tests.PUNSUB_EMPTY = function () {
  876
+  // test situation where punsubscribe reply[1] is null
  877
+  var name = "PUNSUB_EMPTY";
  878
+  client3.punsubscribe(); // punsubscribe from all so can test null
  879
+  client3.punsubscribe(); // reply[1] will be null
  880
+  next(name);
  881
+};
  882
+
  883
+tests.UNSUB_EMPTY_CB = function () {
  884
+  var name = "UNSUB_EMPTY_CB";
  885
+  // test hangs on older versions of redis, so skip
  886
+  if (!server_version_at_least(client, [2, 6, 11])) return next(name);
  887
+
  888
+  // test situation where unsubscribe reply[1] is null
  889
+  client3.unsubscribe(); // unsubscribe from all so can test null
  890
+  client3.unsubscribe(function (err, results) {
  891
+      // reply[1] will be null
  892
+      assert.strictEqual(null, err, "unexpected error: " + err);
  893
+      next(name);
  894
+  });
  895
+};
  896
+
  897
+tests.PUNSUB_EMPTY_CB = function () {
  898
+  var name = "PUNSUB_EMPTY_CB";
  899
+  // test hangs on older versions of redis, so skip
  900
+  if (!server_version_at_least(client, [2, 6, 11])) return next(name);
  901
+
  902
+  // test situation where punsubscribe reply[1] is null
  903
+  client3.punsubscribe(); // punsubscribe from all so can test null
  904
+  client3.punsubscribe(function (err, results) {
  905
+      // reply[1] will be null
  906
+      assert.strictEqual(null, err, "unexpected error: " + err);
  907
+      next(name);
  908
+  });
  909
+};
  910
+
867 911
 tests.SUB_UNSUB_SUB = function () {
868 912
     var name = "SUB_UNSUB_SUB";
869 913
     client3.subscribe('chan3');
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.