aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--actioncable/CHANGELOG.md5
-rw-r--r--actioncable/app/assets/javascripts/action_cable/connection.coffee5
-rw-r--r--actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee6
-rw-r--r--actioncable/app/assets/javascripts/action_cable/subscriptions.coffee6
-rw-r--r--actioncable/lib/action_cable.rb5
-rw-r--r--actioncable/lib/action_cable/connection/base.rb10
-rw-r--r--actioncable/lib/action_cable/connection/subscriptions.rb25
-rw-r--r--actioncable/test/client_test.rb9
-rw-r--r--actioncable/test/connection/base_test.rb2
-rw-r--r--actioncable/test/connection/subscriptions_test.rb4
-rw-r--r--actioncable/test/test_helper.rb1
-rw-r--r--actionpack/test/dispatch/routing/inspector_test.rb18
-rw-r--r--tools/test.rb5
13 files changed, 64 insertions, 37 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/app/assets/javascripts/action_cable/connection.coffee b/actioncable/app/assets/javascripts/action_cable/connection.coffee
index 4244322a1e..e8c9ae6bd0 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee
@@ -73,8 +73,11 @@ class ActionCable.Connection
events:
message: (event) ->
{identifier, message, type} = JSON.parse(event.data)
-
switch type
+ when message_types.welcome
+ @consumer.connectionMonitor.connected()
+ when message_types.ping
+ @consumer.connectionMonitor.ping()
when message_types.confirmation
@consumer.subscriptions.notify(identifier, "connected")
when message_types.rejection
diff --git a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
index 75a6f1fb07..740e86643e 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
@@ -7,10 +7,7 @@ class ActionCable.ConnectionMonitor
@staleThreshold: 6 # Server::Connections::BEAT_INTERVAL * 2 (missed two pings)
- identifier: ActionCable.INTERNAL.identifiers.ping
-
constructor: (@consumer) ->
- @consumer.subscriptions.add(this)
@start()
connected: ->
@@ -22,11 +19,12 @@ class ActionCable.ConnectionMonitor
disconnected: ->
@disconnectedAt = now()
- received: ->
+ ping: ->
@pingedAt = now()
reset: ->
@reconnectAttempts = 0
+ @consumer.connection.isOpen()
start: ->
@reset()
diff --git a/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee b/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
index ae041ffa2b..2443bca14a 100644
--- a/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
@@ -58,7 +58,5 @@ class ActionCable.Subscriptions
sendCommand: (subscription, command) ->
{identifier} = subscription
- if identifier is ActionCable.INTERNAL.identifiers.ping
- @consumer.connection.isOpen()
- else
- @consumer.send({command, identifier})
+ @consumer.send({command, identifier})
+
diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb
index 1dc66ef3ad..a8e4d1cb25 100644
--- a/actioncable/lib/action_cable.rb
+++ b/actioncable/lib/action_cable.rb
@@ -29,10 +29,9 @@ module ActionCable
extend ActiveSupport::Autoload
INTERNAL = {
- identifiers: {
- ping: '_ping'.freeze
- },
message_types: {
+ welcome: 'welcome'.freeze,
+ ping: 'ping'.freeze,
confirmation: 'confirm_subscription'.freeze,
rejection: 'reject_subscription'.freeze
}
diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb
index afe0d958d7..f34f5eb109 100644
--- a/actioncable/lib/action_cable/connection/base.rb
+++ b/actioncable/lib/action_cable/connection/base.rb
@@ -115,7 +115,7 @@ module ActionCable
end
def beat
- transmit ActiveSupport::JSON.encode(identifier: ActionCable::INTERNAL[:identifiers][:ping], message: Time.now.to_i)
+ transmit ActiveSupport::JSON.encode(type: ActionCable::INTERNAL[:message_types][:ping], message: Time.now.to_i)
end
def on_open # :nodoc:
@@ -155,7 +155,7 @@ module ActionCable
def handle_open
connect if respond_to?(:connect)
subscribe_to_internal_channel
- confirm_connection_monitor_subscription
+ send_welcome_message
message_buffer.process!
server.add_connection(self)
@@ -174,11 +174,11 @@ module ActionCable
disconnect if respond_to?(:disconnect)
end
- def confirm_connection_monitor_subscription
- # Send confirmation message to the internal connection monitor channel.
+ def send_welcome_message
+ # Send welcome message to the internal connection monitor channel.
# This ensures the connection monitor state is reset after a successful
# websocket connection.
- transmit ActiveSupport::JSON.encode(identifier: ActionCable::INTERNAL[:identifiers][:ping], type: ActionCable::INTERNAL[:message_types][:confirmation])
+ transmit ActiveSupport::JSON.encode(type: ActionCable::INTERNAL[:message_types][:welcome])
end
def allow_request_origin?
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/client_test.rb b/actioncable/test/client_test.rb
index a6619d3bd2..75545993da 100644
--- a/actioncable/test/client_test.rb
+++ b/actioncable/test/client_test.rb
@@ -75,7 +75,7 @@ class ClientTest < ActionCable::TestCase
@ws.on(:message) do |event|
hash = JSON.parse(event.data)
- if hash['identifier'] == '_ping'
+ if hash['type'] == 'ping'
@pings += 1
else
@messages << hash
@@ -146,6 +146,7 @@ class ClientTest < ActionCable::TestCase
def test_single_client
with_puma_server do |port|
c = faye_client(port)
+ assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -162,6 +163,7 @@ class ClientTest < ActionCable::TestCase
barrier_2 = Concurrent::CyclicBarrier.new(clients.size)
clients.map {|c| Concurrent::Future.execute {
+ assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -181,6 +183,7 @@ class ClientTest < ActionCable::TestCase
clients = 100.times.map { faye_client(port) }
clients.map {|c| Concurrent::Future.execute {
+ assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -194,12 +197,14 @@ class ClientTest < ActionCable::TestCase
def test_disappearing_client
with_puma_server do |port|
c = faye_client(port)
+ assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'delay', message: 'hello')
c.close # disappear before write
c = faye_client(port)
+ assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -214,6 +219,7 @@ class ClientTest < ActionCable::TestCase
identifier = JSON.dump(channel: 'EchoChannel')
c = faye_client(port)
+ assert_equal({"type" => "welcome"}, c.read_message)
c.send_message command: 'subscribe', identifier: identifier
assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
assert_equal(1, app.connections.count)
@@ -232,6 +238,7 @@ class ClientTest < ActionCable::TestCase
def test_server_restart
with_puma_server do |port|
c = faye_client(port)
+ assert_equal({"type" => "welcome"}, c.read_message)
c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
diff --git a/actioncable/test/connection/base_test.rb b/actioncable/test/connection/base_test.rb
index fb11f9be64..fb83bd7d77 100644
--- a/actioncable/test/connection/base_test.rb
+++ b/actioncable/test/connection/base_test.rb
@@ -56,7 +56,7 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase
run_in_eventmachine do
connection = open_connection
- connection.websocket.expects(:transmit).with({ identifier: "_ping", type: "confirm_subscription" }.to_json)
+ connection.websocket.expects(:transmit).with({ type: "welcome" }.to_json)
connection.message_buffer.expects(:process!)
connection.process
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 }
diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb
index d8cffa425f..9d0d23d6de 100644
--- a/actionpack/test/dispatch/routing/inspector_test.rb
+++ b/actionpack/test/dispatch/routing/inspector_test.rb
@@ -170,7 +170,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_route_with_defaults
+ def test_rails_routes_shows_route_with_defaults
output = draw do
get 'photos/:id' => 'photos#show', :defaults => {:format => 'jpg'}
end
@@ -181,7 +181,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_route_with_constraints
+ def test_rails_routes_shows_route_with_constraints
output = draw do
get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/
end
@@ -192,7 +192,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_routes_with_dashes
+ def test_rails_routes_shows_routes_with_dashes
output = draw do
get 'about-us' => 'pages#about_us'
get 'our-work/latest'
@@ -215,7 +215,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_route_with_rack_app
+ def test_rails_routes_shows_route_with_rack_app
output = draw do
get 'foo/:id' => MountedRackApp, :id => /[A-Z]\d{5}/
end
@@ -226,7 +226,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_named_route_with_mounted_rack_app
+ def test_rails_routes_shows_named_route_with_mounted_rack_app
output = draw do
mount MountedRackApp => '/foo'
end
@@ -237,7 +237,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_overridden_named_route_with_mounted_rack_app_with_name
+ def test_rails_routes_shows_overridden_named_route_with_mounted_rack_app_with_name
output = draw do
mount MountedRackApp => '/foo', as: 'blog'
end
@@ -248,7 +248,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_shows_route_with_rack_app_nested_with_dynamic_constraints
+ def test_rails_routes_shows_route_with_rack_app_nested_with_dynamic_constraints
constraint = Class.new do
def inspect
"( my custom constraint )"
@@ -267,7 +267,7 @@ module ActionDispatch
], output
end
- def test_rake_routes_dont_show_app_mounted_in_assets_prefix
+ def test_rails_routes_dont_show_app_mounted_in_assets_prefix
output = draw do
get '/sprockets' => MountedRackApp
end
@@ -275,7 +275,7 @@ module ActionDispatch
assert_no_match(/\/sprockets/, output.first)
end
- def test_rake_routes_shows_route_defined_in_under_assets_prefix
+ def test_rails_routes_shows_route_defined_in_under_assets_prefix
output = draw do
scope '/sprockets' do
get '/foo' => 'foo#bar'
diff --git a/tools/test.rb b/tools/test.rb
index 70f295b554..62e0faa3db 100644
--- a/tools/test.rb
+++ b/tools/test.rb
@@ -1,5 +1,8 @@
$: << File.expand_path("test", COMPONENT_ROOT)
-require File.expand_path("../../load_paths", __FILE__)
+
+require 'bundler'
+Bundler.setup
+
require "rails/test_unit/minitest_plugin"
module Rails
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-15 23:27:47 +0000