From 58dbc1c2ed0e372d9cae4c9e3baebb679a726dc3 Mon Sep 17 00:00:00 2001 From: Mick Staugaard Date: Thu, 11 Oct 2018 13:47:16 -0700 Subject: [PATCH] Stop trying to reconnect on unauthorized cable connections --- actioncable/CHANGELOG.md | 5 +++++ actioncable/app/assets/javascripts/action_cable.js | 14 +++++++++++++- .../app/javascript/action_cable/connection.js | 5 ++++- actioncable/lib/action_cable.rb | 6 ++++++ actioncable/lib/action_cable/connection/base.rb | 11 ++++++++--- actioncable/lib/action_cable/server/base.rb | 4 +++- actioncable/test/connection/authorization_test.rb | 7 +++++-- actioncable/test/connection/base_test.rb | 2 +- actioncable/test/connection/client_socket_test.rb | 2 +- 9 files changed, 46 insertions(+), 10 deletions(-) diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index 5d2e5d7634..88a74a521f 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,3 +1,8 @@ +* The JavaScript WebSocket client will no longer try to reconnect + when you call `reject_unauthorized_connection` on the connection. + + *Mick Staugaard* + * `ActionCable.Connection#getState` now references the configurable `ActionCable.adapters.WebSocket` property rather than the `WebSocket` global variable, matching the behavior of `ActionCable.Connection#open`. diff --git a/actioncable/app/assets/javascripts/action_cable.js b/actioncable/app/assets/javascripts/action_cable.js index ee11c62eb6..65e32d6c3f 100644 --- a/actioncable/app/assets/javascripts/action_cable.js +++ b/actioncable/app/assets/javascripts/action_cable.js @@ -136,10 +136,16 @@ var INTERNAL = { message_types: { welcome: "welcome", + disconnect: "disconnect", ping: "ping", confirmation: "confirm_subscription", rejection: "reject_subscription" }, + disconnect_reasons: { + unauthorized: "unauthorized", + invalid_request: "invalid_request", + server_restart: "server_restart" + }, default_mount_path: "/cable", protocols: [ "actioncable-v1-json", "actioncable-unsupported" ] }; @@ -251,12 +257,18 @@ if (!this.isProtocolSupported()) { return; } - var _JSON$parse = JSON.parse(event.data), identifier = _JSON$parse.identifier, message = _JSON$parse.message, type = _JSON$parse.type; + var _JSON$parse = JSON.parse(event.data), identifier = _JSON$parse.identifier, message = _JSON$parse.message, reason = _JSON$parse.reason, reconnect = _JSON$parse.reconnect, type = _JSON$parse.type; switch (type) { case message_types.welcome: this.monitor.recordConnect(); return this.subscriptions.reload(); + case message_types.disconnect: + logger.log("Disconnecting. Reason: " + reason); + return this.close({ + allowReconnect: reconnect + }); + case message_types.ping: return this.monitor.recordPing(); diff --git a/actioncable/app/javascript/action_cable/connection.js b/actioncable/app/javascript/action_cable/connection.js index 8aa4fe1266..b2910cb2a6 100644 --- a/actioncable/app/javascript/action_cable/connection.js +++ b/actioncable/app/javascript/action_cable/connection.js @@ -117,11 +117,14 @@ Connection.reopenDelay = 500 Connection.prototype.events = { message(event) { if (!this.isProtocolSupported()) { return } - const {identifier, message, type} = JSON.parse(event.data) + const {identifier, message, reason, reconnect, type} = JSON.parse(event.data) switch (type) { case message_types.welcome: this.monitor.recordConnect() return this.subscriptions.reload() + case message_types.disconnect: + logger.log(`Disconnecting. Reason: ${reason}`) + return this.close({allowReconnect: reconnect}) case message_types.ping: return this.monitor.recordPing() case message_types.confirmation: diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb index d261d4112e..cb9dfa2268 100644 --- a/actioncable/lib/action_cable.rb +++ b/actioncable/lib/action_cable.rb @@ -33,10 +33,16 @@ module ActionCable INTERNAL = { message_types: { welcome: "welcome", + disconnect: "disconnect", ping: "ping", confirmation: "confirm_subscription", rejection: "reject_subscription" }, + disconnect_reasons: { + unauthorized: "unauthorized", + invalid_request: "invalid_request", + server_restart: "server_restart" + }, default_mount_path: "/cable", protocols: ["actioncable-v1-json", "actioncable-unsupported"].freeze } diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index 11a1f1a5e8..0044afad98 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -95,7 +95,12 @@ module ActionCable end # Close the WebSocket connection. - def close + def close(reason: nil, reconnect: true) + transmit( + type: ActionCable::INTERNAL[:message_types][:disconnect], + reason: reason, + reconnect: reconnect + ) websocket.close end @@ -170,7 +175,7 @@ module ActionCable message_buffer.process! server.add_connection(self) rescue ActionCable::Connection::Authorization::UnauthorizedError - respond_to_invalid_request + close(reason: ActionCable::INTERNAL[:disconnect_reasons][:unauthorized], reconnect: false) if websocket.alive? end def handle_close @@ -211,7 +216,7 @@ module ActionCable end def respond_to_invalid_request - close if websocket.alive? + close(reason: ActionCable::INTERNAL[:disconnect_reasons][:invalid_request]) if websocket.alive? logger.error invalid_request_message logger.info finished_request_message diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb index 1ee03f6dfc..2b9e1cba3b 100644 --- a/actioncable/lib/action_cable/server/base.rb +++ b/actioncable/lib/action_cable/server/base.rb @@ -36,7 +36,9 @@ module ActionCable end def restart - connections.each(&:close) + connections.each do |connection| + connection.close(reason: ActionCable::INTERNAL[:disconnect_reasons][:server_restart]) + end @mutex.synchronize do # Shutdown the worker pool diff --git a/actioncable/test/connection/authorization_test.rb b/actioncable/test/connection/authorization_test.rb index f77e543435..ac5c128135 100644 --- a/actioncable/test/connection/authorization_test.rb +++ b/actioncable/test/connection/authorization_test.rb @@ -25,8 +25,11 @@ class ActionCable::Connection::AuthorizationTest < ActionCable::TestCase "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" connection = Connection.new(server, env) - assert_called(connection.websocket, :close) do - connection.process + + assert_called_with(connection.websocket, :transmit, [{ type: "disconnect", reason: "unauthorized", reconnect: false }.to_json]) do + assert_called(connection.websocket, :close) do + connection.process + end end end end diff --git a/actioncable/test/connection/base_test.rb b/actioncable/test/connection/base_test.rb index 6ffa0961bc..299879ad4c 100644 --- a/actioncable/test/connection/base_test.rb +++ b/actioncable/test/connection/base_test.rb @@ -108,7 +108,7 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase connection.process assert_called(connection.websocket, :close) do - connection.close + connection.close(reason: "testing") end end end diff --git a/actioncable/test/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb index a7db32c3e4..1ab3c3b71d 100644 --- a/actioncable/test/connection/client_socket_test.rb +++ b/actioncable/test/connection/client_socket_test.rb @@ -58,7 +58,7 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase client.instance_variable_get("@stream") .instance_variable_get("@rack_hijack_io") .define_singleton_method(:close) { event.set } - connection.close + connection.close(reason: "testing") event.wait end end