Merge pull request #72 from pusher/validate_socket_id

Validate socket IDs

Author: zimbatm

lib/pusher/channel.rb

@@ -34,7 +34,10 @@ def initialize(base_url, name, client = Pusher)
     #
     def trigger_async(event_name, data, socket_id = nil)
       params = {}
-      params[:socket_id] = socket_id if socket_id
+      if socket_id
+        validate_socket_id(socket_id)
+        params[:socket_id] = socket_id
+      end
       @client.trigger_async(name, event_name, data, params)
     end
 
@@ -60,7 +63,10 @@ def trigger_async(event_name, data, socket_id = nil)
     #
     def trigger!(event_name, data, socket_id = nil)
       params = {}
-      params[:socket_id] = socket_id if socket_id
+      if socket_id
+        validate_socket_id(socket_id)
+        params[:socket_id] = socket_id
+      end
       @client.trigger(name, event_name, data, params)
     end
 
@@ -115,9 +121,7 @@ def users
     # @return [String]
     #
     def authentication_string(socket_id, custom_string = nil)
-      if socket_id.nil? || socket_id.empty?
-        raise Error, "Invalid socket_id #{socket_id}"
-      end
+      validate_socket_id(socket_id)
 
       unless custom_string.nil? || custom_string.kind_of?(String)
         raise Error, 'Custom argument must be a string'
@@ -162,5 +166,13 @@ def authenticate(socket_id, custom_data = nil)
       r[:channel_data] = custom_data if custom_data
       r
     end
+
+    private
+
+    def validate_socket_id(socket_id)
+      unless socket_id && /\A\d+\.\d+\z/.match(socket_id)
+        raise Pusher::Error, "Invalid socket ID #{socket_id.inspect}"
+      end
+    end
   end
 end

spec/channel_spec.rb

@@ -99,9 +99,9 @@ def authentication_string(*data)
     end
 
     it "should return an authentication string given a socket id" do
-      auth = @channel.authentication_string('socketid')
+      auth = @channel.authentication_string('1.1')
 
-      auth.should == '12345678900000001:827076f551e22451357939e4c7bb1200de29f921d5bf80b40d71668f9cd61c40'
+      auth.should == '12345678900000001:02259dff9a2a3f71ea8ab29ac0c0c0ef7996c8f3fd3702be5533f30da7d7fed4'
     end
 
     it "should raise error if authentication is invalid" do
@@ -112,17 +112,17 @@ def authentication_string(*data)
 
     describe 'with extra string argument' do
       it 'should be a string or nil' do
-        authentication_string('socketid', 123).should raise_error Pusher::Error
-        authentication_string('socketid', {}).should raise_error Pusher::Error
+        authentication_string('1.1', 123).should raise_error Pusher::Error
+        authentication_string('1.1', {}).should raise_error Pusher::Error
 
-        authentication_string('socketid', 'boom').should_not raise_error
-        authentication_string('socketid', nil).should_not raise_error
+        authentication_string('1.1', 'boom').should_not raise_error
+        authentication_string('1.1', nil).should_not raise_error
       end
 
       it "should return an authentication string given a socket id and custom args" do
-        auth = @channel.authentication_string('socketid', 'foobar')
+        auth = @channel.authentication_string('1.1', 'foobar')
 
-        auth.should == "12345678900000001:#{hmac(@client.secret, "socketid:test_channel:foobar")}"
+        auth.should == "12345678900000001:#{hmac(@client.secret, "1.1:test_channel:foobar")}"
       end
     end
   end
@@ -135,12 +135,34 @@ def authentication_string(*data)
     it 'should return a hash with signature including custom data and data as json string' do
       MultiJson.stub(:encode).with(@custom_data).and_return 'a json string'
 
-      response = @channel.authenticate('socketid', @custom_data)
+      response = @channel.authenticate('1.1', @custom_data)
 
       response.should == {
-        :auth => "12345678900000001:#{hmac(@client.secret, "socketid:test_channel:a json string")}",
+        :auth => "12345678900000001:#{hmac(@client.secret, "1.1:test_channel:a json string")}",
         :channel_data => 'a json string'
       }
     end
+
+    it 'should fail on invalid socket_ids' do
+      lambda {
+        @channel.authenticate('1.1:')
+      }.should raise_error Pusher::Error
+
+      lambda {
+        @channel.authenticate('1.1foo', 'channel')
+      }.should raise_error Pusher::Error
+
+      lambda {
+        @channel.authenticate(':1.1')
+      }.should raise_error Pusher::Error
+
+      lambda {
+        @channel.authenticate('foo1.1', 'channel')
+      }.should raise_error Pusher::Error
+
+      lambda {
+        @channel.authenticate('foo', 'channel')
+      }.should raise_error Pusher::Error
+    end
   end
 end

spec/client_spec.rb

@@ -225,19 +225,19 @@
           lambda {
             @client.trigger((0..11).map{|i| 'mychannel#{i}'},
               'event', {'some' => 'data'}, {
-                :socket_id => "1234"
+                :socket_id => "12.34"
               })}.should raise_error(Pusher::Error)
         end
 
         it "should pass any parameters in the body of the request" do
           @client.trigger(['mychannel', 'c2'], 'event', {'some' => 'data'}, {
-            :socket_id => "1234"
+            :socket_id => "12.34"
           })
           WebMock.should have_requested(:post, @api_path).with { |req|
             parsed = MultiJson.decode(req.body)
             parsed["name"].should == 'event'
             parsed["channels"].should == ["mychannel", "c2"]
-            parsed["socket_id"].should == '1234'
+            parsed["socket_id"].should == '12.34'
           }
         end
 
@@ -277,10 +277,10 @@
         it "should pass any parameters in the body of the request" do
           EM.run {
             @client.trigger_async('mychannel', 'event', {'some' => 'data'}, {
-              :socket_id => "1234"
+              :socket_id => "12.34"
             }).callback {
               WebMock.should have_requested(:post, @api_path).with { |req|
-                MultiJson.decode(req.body)["socket_id"].should == '1234'
+                MultiJson.decode(req.body)["socket_id"].should == '12.34'
               }
               EM.stop
             }