Fix ConnectionToClient to connect stubs only after authentication.

remoting/host/client_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/client_session.h"
 
 #include <algorithm>
 
 #include "base/message_loop/message_loop_proxy.h"
 #include "remoting/base/capabilities.h"
@@ skipping 76 matching lines @@
     const std::vector<HostExtension*>& extensions)
     : event_handler_(event_handler),
       connection_(connection.Pass()),
       client_jid_(connection_->session()->jid()),
       desktop_environment_factory_(desktop_environment_factory),
       input_tracker_(&host_input_filter_),
       remote_input_filter_(&input_tracker_),
       mouse_clamping_filter_(&remote_input_filter_),
       disable_input_filter_(mouse_clamping_filter_.input_filter()),
       disable_clipboard_filter_(clipboard_echo_filter_.host_filter()),
-      auth_input_filter_(&disable_input_filter_),
-      auth_clipboard_filter_(&disable_clipboard_filter_),
       client_clipboard_factory_(clipboard_echo_filter_.client_filter()),
       max_duration_(max_duration),
       audio_task_runner_(audio_task_runner),
       input_task_runner_(input_task_runner),
       video_capture_task_runner_(video_capture_task_runner),
       video_encode_task_runner_(video_encode_task_runner),
       network_task_runner_(network_task_runner),
       ui_task_runner_(ui_task_runner),
       pairing_registry_(pairing_registry),
+      is_authenticated_(false),
       pause_video_(false),
       lossless_video_encode_(false),
       lossless_video_color_(false),
       weak_factory_(this) {
   connection_->SetEventHandler(this);
 
-  // TODO(sergeyu): Currently ConnectionToClient expects stubs to be
-  // set before channels are connected. Make it possible to set stubs
-  // later and set them only when connection is authenticated.
-  connection_->set_clipboard_stub(&auth_clipboard_filter_);
-  connection_->set_host_stub(this);
-  connection_->set_input_stub(&auth_input_filter_);
-
-  // |auth_*_filter_|'s states reflect whether the session is authenticated.
-  auth_input_filter_.set_enabled(false);
-  auth_clipboard_filter_.set_enabled(false);
-
   // Create a manager for the configured extensions, if any.
   extension_manager_.reset(new HostExtensionSessionManager(extensions, this));
 
 #if defined(OS_WIN)
   // LocalInputMonitorWin filters out an echo of the injected input before it
   // reaches |remote_input_filter_|.
   remote_input_filter_.SetExpectLocalEcho(false);
 #endif  // defined(OS_WIN)
 }
 
@@ skipping 147 matching lines @@
 void ClientSession::OnConnectionAuthenticated(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
   DCHECK(!audio_pump_);
   DCHECK(!desktop_environment_);
   DCHECK(!input_injector_);
   DCHECK(!screen_controls_);
   DCHECK(!video_frame_pump_);
 
-  auth_input_filter_.set_enabled(true);
-  auth_clipboard_filter_.set_enabled(true);
-
-  clipboard_echo_filter_.set_client_stub(connection_->client_stub());
-  mouse_clamping_filter_.set_video_stub(connection_->video_stub());
+  is_authenticated_ = true;
 
   if (max_duration_ > base::TimeDelta()) {
     // TODO(simonmorris): Let Disconnect() tell the client that the
     // disconnection was caused by the session exceeding its maximum duration.
     max_duration_timer_.Start(FROM_HERE, max_duration_,
                               this, &ClientSession::DisconnectSession);
   }
 
   // Disconnect the session if the connection was rejected by the host.
   if (!event_handler_->OnSessionAuthenticated(this)) {
     DisconnectSession();
     return;
   }
 
   // Create the desktop environment. Drop the connection if it could not be
   // created for any reason (for instance the curtain could not initialize).
   desktop_environment_ =
       desktop_environment_factory_->Create(weak_factory_.GetWeakPtr());
   if (!desktop_environment_) {
     DisconnectSession();
     return;
   }
 
+  // Connect host stub.
+  connection_->set_host_stub(this);
+
+  // Connect video stub.
+  mouse_clamping_filter_.set_video_stub(connection_->video_stub());
+
   // Collate the set of capabilities to offer the client, if it supports them.
   host_capabilities_ = desktop_environment_->GetCapabilities();
   if (!host_capabilities_.empty())
     host_capabilities_.append(" ");
   host_capabilities_.append(extension_manager_->GetCapabilities());
 
   // Create the object that controls the screen resolution.
   screen_controls_ = desktop_environment_->CreateScreenControls();
 
   // Create the event executor.
   input_injector_ = desktop_environment_->CreateInputInjector();
 
-  // Connect the host clipboard and input stubs.
+  // Connect the host input stubs.
+  connection_->set_input_stub(&disable_input_filter_);
   host_input_filter_.set_input_stub(input_injector_.get());
+
+  // Connect the clipboard stubs.
+  connection_->set_clipboard_stub(&disable_clipboard_filter_);
   clipboard_echo_filter_.set_host_stub(input_injector_.get());
+  clipboard_echo_filter_.set_client_stub(connection_->client_stub());
 
   // Create a GnubbyAuthHandler to proxy gnubbyd messages.
   gnubby_auth_handler_ = desktop_environment_->CreateGnubbyAuthHandler(
       connection_->client_stub());
 }
 
 void ClientSession::OnConnectionChannelsConnected(
     protocol::ConnectionToClient* connection) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
 
   // Negotiate capabilities with the client.
   VLOG(1) << "Host capabilities: " << host_capabilities_;
-
   protocol::Capabilities capabilities;
   capabilities.set_capabilities(host_capabilities_);
   connection_->client_stub()->SetCapabilities(capabilities);
 
   // Start the event executor.
   input_injector_->Start(CreateClipboardProxy());
   SetDisableInputs(false);
 
   // Start recording video.
   ResetVideoPipeline();
@@ skipping 15 matching lines @@
 void ClientSession::OnConnectionClosed(
     protocol::ConnectionToClient* connection,
     protocol::ErrorCode error) {
   DCHECK(CalledOnValidThread());
   DCHECK_EQ(connection_.get(), connection);
 
   // Ignore any further callbacks.
   weak_factory_.InvalidateWeakPtrs();
 
   // If the client never authenticated then the session failed.
-  if (!auth_input_filter_.enabled())
+  if (!is_authenticated_)
     event_handler_->OnSessionAuthenticationFailed(this);
 
-  // Block any further input events from the client.
-  // TODO(wez): Fix ChromotingHost::OnSessionClosed not to check our
-  // is_authenticated(), so that we can disable |auth_*_filter_| here.
-  disable_input_filter_.set_enabled(false);
-  disable_clipboard_filter_.set_enabled(false);

[Wez, 2015/02/07 00:40:43]

Why remove these?  The ensure that we don't process further input even if the
connection somehow manages to generate more from the event channel.

[Sergey Ulanov, 2015/02/13 19:40:20]

InputInjector is deleted 5 lines later so these are not really useful.

-
   // Ensure that any pressed keys or buttons are released.
   input_tracker_.ReleaseAll();
 
   // Stop components access the client, audio or video stubs, which are no
   // longer valid once ConnectionToClient calls OnConnectionClosed().
   audio_pump_.reset();
   video_frame_pump_.reset();
   mouse_shape_pump_.reset();
   client_clipboard_factory_.InvalidateWeakPtrs();
   input_injector_.reset();
@@ skipping 101 matching lines @@
 
 scoped_ptr<protocol::ClipboardStub> ClientSession::CreateClipboardProxy() {
   DCHECK(CalledOnValidThread());
 
   return make_scoped_ptr(
       new protocol::ClipboardThreadProxy(client_clipboard_factory_.GetWeakPtr(),
                                          base::MessageLoopProxy::current()));
 }
 
 }  // namespace remoting