Skip to content

Commit

Permalink
Validate socket IDs
Browse files Browse the repository at this point in the history
  • Loading branch information
danielwaterworth committed May 8, 2015
1 parent 7df47f8 commit 46fb1e6
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 20 deletions.
22 changes: 17 additions & 5 deletions lib/pusher/channel.rb
Expand Up @@ -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

This comment has been minimized.

Copy link
@zimbatm

zimbatm May 10, 2015

Contributor

If validate_socket_id was returning the socket_id both of these lines could be merged

end
@client.trigger_async(name, event_name, data, params)
end

Expand All @@ -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

This comment has been minimized.

Copy link
@zimbatm

zimbatm May 10, 2015

Contributor

If validate_socket_id was returning the socket_id both of these lines could be merged

end
@client.trigger(name, event_name, data, params)
end

Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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 && /\d+\.\d+$/.match(socket_id)
raise Pusher::Error, "Invalid socket ID #{socket_id}"

This comment has been minimized.

Copy link
@zimbatm

zimbatm May 10, 2015

Contributor

Using socket_id.inspect will make it easier for the user to detect that is was nil

end
end
end
end
34 changes: 24 additions & 10 deletions spec/channel_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -135,12 +135,26 @@ 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('foo', 'channel')
}.should raise_error Pusher::Error

This comment has been minimized.

Copy link
@zimbatm

zimbatm May 10, 2015

Contributor

What about socket ID :1.1 ?

This comment has been minimized.

Copy link
@danielwaterworth

danielwaterworth May 11, 2015

Author Contributor

There will always be cases that I'll miss. That's the nature of testing.

end
end
end
10 changes: 5 additions & 5 deletions spec/client_spec.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 46fb1e6

Please sign in to comment.