diff options
| -rw-r--r-- | actioncable/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | actioncable/lib/action_cable/connection/subscriptions.rb | 25 | ||||
| -rw-r--r-- | actioncable/test/connection/subscriptions_test.rb | 4 | ||||
| -rw-r--r-- | actioncable/test/test_helper.rb | 1 |
4 files changed, 27 insertions, 8 deletions
diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 946fdfb3fc..5c505d5dde 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,3 +1,8 @@ +* Allow channel identifiers with no backslahes/escaping to be accepted + by the subscription storer. + + *Jon Moss* + * Safely support autoloading and class unloading, by preventing concurrent loads, and disconnecting all cables during reload. diff --git a/actioncable/lib/action_cable/connection/subscriptions.rb b/actioncable/lib/action_cable/connection/subscriptions.rb index 3742f248d1..5aa907c2d3 100644 --- a/actioncable/lib/action_cable/connection/subscriptions.rb +++ b/actioncable/lib/action_cable/connection/subscriptions.rb @@ -23,13 +23,13 @@ module ActionCable end def add(data) - id_key = data['identifier'] - id_options = ActiveSupport::JSON.decode(id_key).with_indifferent_access + id_options = decode_hash(data['identifier']) + identifier = normalize_identifier(id_options) subscription_klass = connection.server.channel_classes[id_options[:channel]] if subscription_klass - subscriptions[id_key] ||= subscription_klass.new(connection, id_key, id_options) + subscriptions[identifier] ||= subscription_klass.new(connection, identifier, id_options) else logger.error "Subscription class not found (#{data.inspect})" end @@ -37,7 +37,7 @@ module ActionCable def remove(data) logger.info "Unsubscribing from channel: #{data['identifier']}" - remove_subscription subscriptions[data['identifier']] + remove_subscription subscriptions[normalize_identifier(data['identifier'])] end def remove_subscription(subscription) @@ -46,7 +46,7 @@ module ActionCable end def perform_action(data) - find(data).perform_action ActiveSupport::JSON.decode(data['data']) + find(data).perform_action(decode_hash(data['data'])) end def identifiers @@ -63,8 +63,21 @@ module ActionCable private delegate :logger, to: :connection + def normalize_identifier(identifier) + identifier = ActiveSupport::JSON.encode(identifier) if identifier.is_a?(Hash) + identifier + end + + # If `data` is a Hash, this means that the original JSON + # sent by the client had no backslashes in it, and does + # not need to be decoded again. + def decode_hash(data) + data = ActiveSupport::JSON.decode(data) unless data.is_a?(Hash) + data.with_indifferent_access + end + def find(data) - if subscription = subscriptions[data['identifier']] + if subscription = subscriptions[normalize_identifier(data['identifier'])] subscription else raise "Unable to find subscription with identifier: #{data['identifier']}" diff --git a/actioncable/test/connection/subscriptions_test.rb b/actioncable/test/connection/subscriptions_test.rb index 62e41484fe..68a79c6e4c 100644 --- a/actioncable/test/connection/subscriptions_test.rb +++ b/actioncable/test/connection/subscriptions_test.rb @@ -82,13 +82,13 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase end end - test "unsubscrib from all" do + test "unsubscribe from all" do run_in_eventmachine do setup_connection channel1 = subscribe_to_chat_channel - channel2_id = ActiveSupport::JSON.encode(id: 2, channel: 'ActionCable::Connection::SubscriptionsTest::ChatChannel') + channel2_id = ActiveSupport::JSON.encode({ id: 2, channel: 'ActionCable::Connection::SubscriptionsTest::ChatChannel' }) channel2 = subscribe_to_chat_channel(channel2_id) channel1.expects(:unsubscribe_from_channel) diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index 8b157c9b46..797e7786d1 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -6,6 +6,7 @@ require 'puma' require 'mocha/setup' require 'rack/mock' +require 'active_support/core_ext/hash/indifferent_access' # Require all the stubs and models Dir[File.dirname(__FILE__) + '/stubs/*.rb'].each {|file| require file } |