Fix to restrict W3C commands while in W3C mode

By default, all commands are accessible. However, once user specifies
session in W3C mode, all JWP commands will return as "Unknown Command".
Other commands such as "unknown origin", "ChromeDriver extentions"
won't be affected by the PR since they are used by Selenium binding
and internal python tests.


Bug: chromedriver:2060
Change-Id: I1e09fd6f06e290f374ffde5bc3507defe9fb52cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1504977Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: Long Ly <loly@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#638186}

chrome/test/chromedriver/commands.cc

@@ -205,6 +205,7 @@ void TerminateSessionThreadOnCommandThread(SessionThreadMap* session_thread_map,
 void ExecuteSessionCommandOnSessionThread(
     const char* command_name,
     const SessionCommand& command,
+    bool w3c_standard_command,
     bool return_ok_without_session,
     std::unique_ptr<base::DictionaryValue> params,
     scoped_refptr<base::SingleThreadTaskRunner> cmd_task_runner,
@@ -237,65 +238,73 @@ void ExecuteSessionCommandOnSessionThread(
   // Will mark |session| for deletion if an error is encountered.
   Status status = NotifyCommandListenersBeforeCommand(session, command_name);
 
-  // Only run the command if we were able to notify all listeners successfully.
-  // Otherwise, pass error to callback, delete |session|, and do not continue.
   std::unique_ptr<base::Value> value;
-  if (status.IsError()) {
-    LOG(ERROR) << status.message();
+  if (session->w3c_compliant && !w3c_standard_command) {
+    status = Status(kUnknownCommand,
+                    "Cannot call non W3C standard command while in W3C mode");
   } else {
-    status = command.Run(session, *params, &value);
-
-    if (status.IsError() && session->chrome) {
-      if (!session->quit && session->chrome->HasCrashedWebView()) {
-        session->quit = true;
-        std::string message("session deleted because of page crash");
-        if (!session->detach) {
-          Status quit_status = session->chrome->Quit();
-          if (quit_status.IsError())
-            message += ", but failed to kill browser:" + quit_status.message();
+    // Only run the command if we were able to notify all listeners
+    // successfully.
+    // Otherwise, pass error to callback, delete |session|, and do not continue.
+    if (status.IsError()) {
+      LOG(ERROR) << status.message();
+    } else {
+      status = command.Run(session, *params, &value);
+
+      if (status.IsError() && session->chrome) {
+        if (!session->quit && session->chrome->HasCrashedWebView()) {
+          session->quit = true;
+          std::string message("session deleted because of page crash");
+          if (!session->detach) {
+            Status quit_status = session->chrome->Quit();
+            if (quit_status.IsError())
+              message +=
+                  ", but failed to kill browser:" + quit_status.message();
+          }
+          status = Status(kUnknownError, message, status);
+        } else if (status.code() == kDisconnected) {
+          // Some commands, like clicking a button or link which closes the
+          // window, may result in a kDisconnected error code.
+          std::list<std::string> web_view_ids;
+          Status status_tmp = session->chrome->GetWebViewIds(
+              &web_view_ids, session->w3c_compliant);
+          if (status_tmp.IsError() &&
+              status_tmp.code() != kChromeNotReachable) {
+            status.AddDetails("failed to check if window was closed: " +
+                              status_tmp.message());
+          } else if (!base::ContainsValue(web_view_ids, session->window)) {
+            status = Status(kOk);
+          }
         }
-        status = Status(kUnknownError, message, status);
-      } else if (status.code() == kDisconnected) {
-        // Some commands, like clicking a button or link which closes the
-        // window, may result in a kDisconnected error code.
-        std::list<std::string> web_view_ids;
-        Status status_tmp = session->chrome->GetWebViewIds(
-          &web_view_ids, session->w3c_compliant);
-        if (status_tmp.IsError() && status_tmp.code() != kChromeNotReachable) {
-          status.AddDetails(
-              "failed to check if window was closed: " + status_tmp.message());
-        } else if (!base::ContainsValue(web_view_ids, session->window)) {
-          status = Status(kOk);
+        if (status.IsError()) {
+          const BrowserInfo* browser_info = session->chrome->GetBrowserInfo();
+          status.AddDetails("Session info: " + browser_info->browser_name +
+                            "=" + browser_info->browser_version);
         }
       }
-      if (status.IsError()) {
-        const BrowserInfo* browser_info = session->chrome->GetBrowserInfo();
-        status.AddDetails("Session info: " + browser_info->browser_name + "=" +
-                          browser_info->browser_version);
-      }
-    }
 
-    if (IsVLogOn(0)) {
-      std::string result;
-      if (status.IsError()) {
-        result = "ERROR " + status.message();
-      } else if (value) {
-        result = FormatValueForDisplay(*value);
-      }
-      if (!session->driver_log ||
-          session->driver_log->min_level() != Log::Level::kOff) {
-        // Note: ChromeDriver log-replay depends on the format of this logging.
-        // see chromedriver/log_replay/client_replay.py
-        VLOG(0) << "[" << session->id << "] "
-                << "RESPONSE " << command_name
-                << (result.length() ? " " + result : "");
+      if (IsVLogOn(0)) {
+        std::string result;
+        if (status.IsError()) {
+          result = "ERROR " + status.message();
+        } else if (value) {
+          result = FormatValueForDisplay(*value);
+        }
+        if (!session->driver_log ||
+            session->driver_log->min_level() != Log::Level::kOff) {
+          // Note: ChromeDriver log-replay depends on the format of this
+          // logging. see chromedriver/log_replay/client_replay.py
+          VLOG(0) << "[" << session->id << "] "
+                  << "RESPONSE " << command_name
+                  << (result.length() ? " " + result : "");
+        }
       }
-    }
 
-    if (status.IsOk() && session->auto_reporting_enabled) {
-      std::string message = session->GetFirstBrowserError();
-      if (!message.empty())
-        status = Status(kUnknownError, message);
+      if (status.IsOk() && session->auto_reporting_enabled) {
+        std::string message = session->GetFirstBrowserError();
+        if (!message.empty())
+          status = Status(kUnknownError, message);
+      }
     }
   }
 
@@ -312,14 +321,14 @@ void ExecuteSessionCommandOnSessionThread(
 
 }  // namespace
 
-void ExecuteSessionCommand(
-    SessionThreadMap* session_thread_map,
-    const char* command_name,
-    const SessionCommand& command,
-    bool return_ok_without_session,
-    const base::DictionaryValue& params,
-    const std::string& session_id,
-    const CommandCallback& callback) {
+void ExecuteSessionCommand(SessionThreadMap* session_thread_map,
+                           const char* command_name,
+                           const SessionCommand& command,
+                           bool w3c_standard_command,
+                           bool return_ok_without_session,
+                           const base::DictionaryValue& params,
+                           const std::string& session_id,
+                           const CommandCallback& callback) {
   auto iter = session_thread_map->find(session_id);
   if (iter == session_thread_map->end()) {
     Status status(return_ok_without_session ? kOk : kInvalidSessionId);
@@ -329,7 +338,7 @@ void ExecuteSessionCommand(
     iter->second->task_runner()->PostTask(
         FROM_HERE,
         base::BindOnce(&ExecuteSessionCommandOnSessionThread, command_name,
-                       command, return_ok_without_session,
+                       command, w3c_standard_command, return_ok_without_session,
                        base::WrapUnique(params.DeepCopy()),
                        base::ThreadTaskRunnerHandle::Get(), callback,
                        base::Bind(&TerminateSessionThreadOnCommandThread,

chrome/test/chromedriver/commands.h

@@ -58,14 +58,14 @@ typedef base::Callback<Status(Session* session,
 
 // Executes a given session command, after acquiring access to the appropriate
 // session.
-void ExecuteSessionCommand(
-    SessionThreadMap* session_thread_map,
-    const char* command_name,
-    const SessionCommand& command,
-    bool return_ok_without_session,
-    const base::DictionaryValue& params,
-    const std::string& session_id,
-    const CommandCallback& callback);
+void ExecuteSessionCommand(SessionThreadMap* session_thread_map,
+                           const char* command_name,
+                           const SessionCommand& command,
+                           bool w3c_standard_command,
+                           bool return_ok_without_session,
+                           const base::DictionaryValue& params,
+                           const std::string& session_id,
+                           const CommandCallback& callback);
 
 namespace internal {
 void CreateSessionOnSessionThreadForTesting(const std::string& id);

chrome/test/chromedriver/commands_unittest.cc

@@ -246,12 +246,7 @@ TEST(CommandsTest, ExecuteSessionCommand) {
   base::test::ScopedTaskEnvironment scoped_task_environment;
   base::RunLoop run_loop;
   ExecuteSessionCommand(
-      &map,
-      "cmd",
-      cmd,
-      false,
-      params,
-      id,
+      &map, "cmd", cmd, true /*w3c_standard_command*/, false, params, id,
       base::Bind(&OnSimpleCommand, &run_loop, id, &expected_value));
   run_loop.Run();
 }
@@ -286,24 +281,16 @@ void OnNoSuchSessionIsOk(const Status& status,
 TEST(CommandsTest, ExecuteSessionCommandOnNoSuchSession) {
   SessionThreadMap map;
   base::DictionaryValue params;
-  ExecuteSessionCommand(&map,
-                        "cmd",
-                        base::Bind(&ShouldNotBeCalled),
-                        false,
-                        params,
-                        "session",
+  ExecuteSessionCommand(&map, "cmd", base::Bind(&ShouldNotBeCalled),
+                        true /*w3c_standard_command*/, false, params, "session",
                         base::Bind(&OnNoSuchSession));
 }
 
 TEST(CommandsTest, ExecuteSessionCommandOnNoSuchSessionWhenItExpectsOk) {
   SessionThreadMap map;
   base::DictionaryValue params;
-  ExecuteSessionCommand(&map,
-                        "cmd",
-                        base::Bind(&ShouldNotBeCalled),
-                        true,
-                        params,
-                        "session",
+  ExecuteSessionCommand(&map, "cmd", base::Bind(&ShouldNotBeCalled),
+                        true /*w3c_standard_command*/, true, params, "session",
                         base::Bind(&OnNoSuchSessionIsOk));
 }
 
@@ -330,12 +317,9 @@ TEST(CommandsTest, ExecuteSessionCommandOnJustDeletedSession) {
 
   base::test::ScopedTaskEnvironment scoped_task_environment;
   base::RunLoop run_loop;
-  ExecuteSessionCommand(&map,
-                        "cmd",
-                        base::Bind(&ShouldNotBeCalled),
-                        false,
-                        base::DictionaryValue(),
-                        "session",
+  ExecuteSessionCommand(&map, "cmd", base::Bind(&ShouldNotBeCalled),
+                        true /*w3c_standard_command*/, false,
+                        base::DictionaryValue(), "session",
                         base::Bind(&OnNoSuchSessionAndQuit, &run_loop));
   run_loop.Run();
 }
@@ -740,14 +724,9 @@ TEST(CommandsTest, SuccessNotifyingCommandListeners) {
   // |CommandListener|s are notified immediately before commands are run.
   // Here, the command adds |listener| to the session, so |listener|
   // should not be notified since it will not have been added yet.
-  ExecuteSessionCommand(
-      &map,
-      "cmd",
-      cmd,
-      false,
-      params,
-      id,
-      base::Bind(&OnSessionCommand, &run_loop_addlistener));
+  ExecuteSessionCommand(&map, "cmd", cmd, true /*w3c_standard_command*/, false,
+                        params, id,
+                        base::Bind(&OnSessionCommand, &run_loop_addlistener));
   run_loop_addlistener.Run();
 
   listener->VerifyNotCalled();
@@ -757,14 +736,9 @@ TEST(CommandsTest, SuccessNotifyingCommandListeners) {
 
   // |listener| was added to |session| by ExecuteAddListenerToSessionCommand
   // and should be notified before the next command, ExecuteQuitSessionCommand.
-  ExecuteSessionCommand(
-      &map,
-      "cmd",
-      cmd,
-      false,
-      params,
-      id,
-      base::Bind(&OnSessionCommand, &run_loop_testlistener));
+  ExecuteSessionCommand(&map, "cmd", cmd, true /*w3c_standard_command*/, false,
+                        params, id,
+                        base::Bind(&OnSessionCommand, &run_loop_testlistener));
   run_loop_testlistener.Run();
 
   listener->VerifyCalled();
@@ -832,12 +806,7 @@ TEST(CommandsTest, ErrorNotifyingCommandListeners) {
   base::RunLoop run_loop;
 
   ExecuteSessionCommand(
-      &map,
-      "cmd",
-      cmd,
-      false,
-      params,
-      id,
+      &map, "cmd", cmd, true /*w3c_standard_command*/, false, params, id,
       base::Bind(&OnFailBecauseErrorNotifyingListeners, &run_loop));
   run_loop.Run();
 

chrome/test/chromedriver/server/http_handler.h

@@ -85,10 +85,14 @@ class HttpHandler {
   typedef std::vector<CommandMapping> CommandMap;
 
   Command WrapToCommand(const char* name,
-                        const SessionCommand& session_command);
-  Command WrapToCommand(const char* name, const WindowCommand& window_command);
+                        const SessionCommand& session_command,
+                        bool w3c_standard_command = true);
   Command WrapToCommand(const char* name,
-                        const ElementCommand& element_command);
+                        const WindowCommand& window_command,
+                        bool w3c_standard_command = true);
+  Command WrapToCommand(const char* name,
+                        const ElementCommand& element_command,
+                        bool w3c_standard_command = true);
   void HandleCommand(const net::HttpServerRequestInfo& request,
                      const std::string& trimmed_path,
                      const HttpResponseSenderFunc& send_response_func);