[ChromeDriver] Allow POST with empty body in OSS mode
W3C WebDriver spec requires POST request to always have a non-empty
body. However, some legacy clients send empty POST body in some cases.
Updating ChromeDriver to handle such clients.
(cherry picked from commit e705a095439d0658fb81803396d4dcdb4c368c64)
Bug: 973088, chromedriver:2943
Change-Id: If29dd990dbebc0b4fc7c5df35ac25e4a4bc47f30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1648893
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667714}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1653649
Reviewed-by: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#226}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/test/chromedriver/commands.cc b/chrome/test/chromedriver/commands.cc
index eb0ddabb..a21c422 100644
--- a/chrome/test/chromedriver/commands.cc
+++ b/chrome/test/chromedriver/commands.cc
@@ -28,6 +28,7 @@
#include "chrome/test/chromedriver/chrome/status.h"
#include "chrome/test/chromedriver/logging.h"
#include "chrome/test/chromedriver/session.h"
+#include "chrome/test/chromedriver/session_commands.h"
#include "chrome/test/chromedriver/session_thread_map.h"
#include "chrome/test/chromedriver/util.h"
#include "chrome/test/chromedriver/version.h"
@@ -68,8 +69,9 @@
if (new_id.empty())
new_id = GenerateId();
std::unique_ptr<Session> session = std::make_unique<Session>(new_id);
- std::unique_ptr<base::Thread> thread = std::make_unique<base::Thread>(new_id);
- if (!thread->Start()) {
+ std::unique_ptr<SessionThreadInfo> threadInfo =
+ std::make_unique<SessionThreadInfo>(new_id, GetW3CSetting(params));
+ if (!threadInfo->thread()->Start()) {
callback.Run(
Status(kUnknownError, "failed to start a thread for the new session"),
std::unique_ptr<base::Value>(), std::string(),
@@ -77,9 +79,9 @@
return;
}
- thread->task_runner()->PostTask(
+ threadInfo->thread()->task_runner()->PostTask(
FROM_HERE, base::BindOnce(&SetThreadLocalSession, std::move(session)));
- session_thread_map->insert(std::make_pair(new_id, std::move(thread)));
+ session_thread_map->insert(std::make_pair(new_id, std::move(threadInfo)));
init_session_cmd.Run(params, new_id, callback);
}
@@ -335,7 +337,7 @@
callback.Run(status, std::unique_ptr<base::Value>(), session_id,
kW3CDefault);
} else {
- iter->second->task_runner()->PostTask(
+ iter->second->thread()->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&ExecuteSessionCommandOnSessionThread, command_name,
command, w3c_standard_command, return_ok_without_session,
diff --git a/chrome/test/chromedriver/commands_unittest.cc b/chrome/test/chromedriver/commands_unittest.cc
index 99f6524..752d1a6 100644
--- a/chrome/test/chromedriver/commands_unittest.cc
+++ b/chrome/test/chromedriver/commands_unittest.cc
@@ -141,8 +141,8 @@
SessionThreadMap map;
Session session("id");
Session session2("id2");
- map[session.id] = std::make_unique<base::Thread>("1");
- map[session2.id] = std::make_unique<base::Thread>("2");
+ map[session.id] = std::make_unique<SessionThreadInfo>("1", true);
+ map[session2.id] = std::make_unique<SessionThreadInfo>("2", true);
int count = 0;
@@ -186,8 +186,8 @@
SessionThreadMap map;
Session session("id");
Session session2("id2");
- map[session.id] = std::make_unique<base::Thread>("1");
- map[session2.id] = std::make_unique<base::Thread>("2");
+ map[session.id] = std::make_unique<SessionThreadInfo>("1", true);
+ map[session2.id] = std::make_unique<SessionThreadInfo>("2", true);
int count = 0;
Command cmd = base::Bind(&ExecuteStubQuit, &count);
@@ -229,13 +229,14 @@
TEST(CommandsTest, ExecuteSessionCommand) {
SessionThreadMap map;
- auto thread = std::make_unique<base::Thread>("1");
+ auto threadInfo = std::make_unique<SessionThreadInfo>("1", true);
+ base::Thread* thread = threadInfo->thread();
ASSERT_TRUE(thread->Start());
std::string id("id");
thread->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&internal::CreateSessionOnSessionThreadForTesting, id));
- map[id] = std::move(thread);
+ map[id] = std::move(threadInfo);
base::DictionaryValue params;
params.SetInteger("param", 5);
@@ -310,10 +311,10 @@
TEST(CommandsTest, ExecuteSessionCommandOnJustDeletedSession) {
SessionThreadMap map;
- auto thread = std::make_unique<base::Thread>("1");
- ASSERT_TRUE(thread->Start());
+ auto threadInfo = std::make_unique<SessionThreadInfo>("1", true);
+ ASSERT_TRUE(threadInfo->thread()->Start());
std::string id("id");
- map[id] = std::move(thread);
+ map[id] = std::move(threadInfo);
base::test::ScopedTaskEnvironment scoped_task_environment;
base::RunLoop run_loop;
@@ -701,14 +702,15 @@
TEST(CommandsTest, SuccessNotifyingCommandListeners) {
SessionThreadMap map;
- auto thread = std::make_unique<base::Thread>("1");
+ auto threadInfo = std::make_unique<SessionThreadInfo>("1", true);
+ base::Thread* thread = threadInfo->thread();
ASSERT_TRUE(thread->Start());
std::string id("id");
thread->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&internal::CreateSessionOnSessionThreadForTesting, id));
- map[id] = std::move(thread);
+ map[id] = std::move(threadInfo);
base::DictionaryValue params;
auto listener = std::make_unique<MockCommandListener>();
@@ -782,20 +784,20 @@
TEST(CommandsTest, ErrorNotifyingCommandListeners) {
SessionThreadMap map;
- auto thread = std::make_unique<base::Thread>("1");
- base::Thread* thread_ptr = thread.get();
+ auto threadInfo = std::make_unique<SessionThreadInfo>("1", true);
+ base::Thread* thread = threadInfo->thread();
ASSERT_TRUE(thread->Start());
std::string id("id");
thread->task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&internal::CreateSessionOnSessionThreadForTesting, id));
- map[id] = std::move(thread);
+ map[id] = std::move(threadInfo);
// In SuccessNotifyingCommandListenersBeforeCommand, we verified BeforeCommand
// was called before (as opposed to after) command execution. We don't need to
// verify this again, so we can just add |listener| with PostTask.
auto listener = std::make_unique<FailingCommandListener>();
- thread_ptr->task_runner()->PostTask(
+ thread->task_runner()->PostTask(
FROM_HERE, base::BindOnce(&AddListenerToSessionIfSessionExists,
std::move(listener)));
@@ -810,6 +812,6 @@
base::Bind(&OnFailBecauseErrorNotifyingListeners, &run_loop));
run_loop.Run();
- thread_ptr->task_runner()->PostTask(FROM_HERE,
- base::BindOnce(&VerifySessionWasDeleted));
+ thread->task_runner()->PostTask(FROM_HERE,
+ base::BindOnce(&VerifySessionWasDeleted));
}
diff --git a/chrome/test/chromedriver/server/http_handler.cc b/chrome/test/chromedriver/server/http_handler.cc
index 01b96bc..1e85e02 100644
--- a/chrome/test/chromedriver/server/http_handler.cc
+++ b/chrome/test/chromedriver/server/http_handler.cc
@@ -50,6 +50,13 @@
const char kSessionStorage[] = "sessionStorage";
const char kShutdownPath[] = "shutdown";
+bool w3cMode(const std::string& session_id,
+ const SessionThreadMap& session_thread_map) {
+ if (session_id.length() > 0 && session_thread_map.count(session_id) > 0)
+ return session_thread_map.at(session_id)->w3cMode();
+ return kW3CDefault;
+}
+
} // namespace
// WrapperURLLoaderFactory subclasses mojom::URLLoaderFactory as non-mojo, cross
@@ -911,11 +918,11 @@
CommandMap::const_iterator iter = command_map_->begin();
while (true) {
if (iter == command_map_->end()) {
- if (kW3CDefault) {
+ if (w3cMode(session_id, session_thread_map_)) {
PrepareResponse(
trimmed_path, send_response_func,
Status(kUnknownCommand, "unknown command: " + trimmed_path),
- nullptr, session_id, kW3CDefault);
+ nullptr, session_id, true);
} else {
std::unique_ptr<net::HttpServerResponseInfo> response(
new net::HttpServerResponseInfo(net::HTTP_NOT_FOUND));
@@ -936,10 +943,10 @@
std::unique_ptr<base::Value> parsed_body =
base::JSONReader::ReadDeprecated(request.data);
if (!parsed_body || !parsed_body->GetAsDictionary(&body_params)) {
- if (kW3CDefault) {
+ if (w3cMode(session_id, session_thread_map_)) {
PrepareResponse(trimmed_path, send_response_func,
Status(kInvalidArgument, "missing command parameters"),
- nullptr, session_id, kW3CDefault);
+ nullptr, session_id, true);
} else {
std::unique_ptr<net::HttpServerResponseInfo> response(
new net::HttpServerResponseInfo(net::HTTP_BAD_REQUEST));
@@ -949,12 +956,13 @@
return;
}
params.MergeDictionary(body_params);
- } else if (kW3CDefault && iter->method == kPost) {
+ } else if (iter->method == kPost &&
+ w3cMode(session_id, session_thread_map_)) {
// Data in JSON format is required for POST requests. See step 5 of
// https://www.w3.org/TR/2018/REC-webdriver1-20180605/#processing-model.
PrepareResponse(trimmed_path, send_response_func,
Status(kInvalidArgument, "missing command parameters"),
- nullptr, session_id, kW3CDefault);
+ nullptr, session_id, true);
return;
}
diff --git a/chrome/test/chromedriver/session_commands.cc b/chrome/test/chromedriver/session_commands.cc
index fdc007d..2f94cad 100644
--- a/chrome/test/chromedriver/session_commands.cc
+++ b/chrome/test/chromedriver/session_commands.cc
@@ -98,6 +98,41 @@
InitSessionParams::~InitSessionParams() {}
+// Look for W3C mode setting in InitSession command parameters.
+bool GetW3CSetting(const base::DictionaryValue& params) {
+ bool w3c;
+ const base::ListValue* list;
+ const base::DictionaryValue* dict;
+
+ if (params.GetDictionary("capabilities.alwaysMatch", &dict)) {
+ if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
+ dict->GetBoolean("chromeOptions.w3c", &w3c)) {
+ return w3c;
+ }
+ }
+
+ if (params.GetList("capabilities.firstMatch", &list) &&
+ list->GetDictionary(0, &dict)) {
+ if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
+ dict->GetBoolean("chromeOptions.w3c", &w3c)) {
+ return w3c;
+ }
+ }
+
+ if (params.GetDictionary("desiredCapabilities", &dict)) {
+ if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
+ dict->GetBoolean("chromeOptions.w3c", &w3c)) {
+ return w3c;
+ }
+ }
+
+ if (!params.HasKey("capabilities")) {
+ return false;
+ }
+
+ return kW3CDefault;
+}
+
namespace {
// Creates a JSON object (represented by base::DictionaryValue) that contains
@@ -211,41 +246,6 @@
return Status(kOk);
}
-// Look for W3C mode setting in InitSession command parameters.
-bool GetW3CSetting(const base::DictionaryValue& params) {
- bool w3c;
- const base::ListValue* list;
- const base::DictionaryValue* dict;
-
- if (params.GetDictionary("capabilities.alwaysMatch", &dict)) {
- if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
- dict->GetBoolean("chromeOptions.w3c", &w3c)) {
- return w3c;
- }
- }
-
- if (params.GetList("capabilities.firstMatch", &list) &&
- list->GetDictionary(0, &dict)) {
- if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
- dict->GetBoolean("chromeOptions.w3c", &w3c)) {
- return w3c;
- }
- }
-
- if (params.GetDictionary("desiredCapabilities", &dict)) {
- if (dict->GetBoolean("goog:chromeOptions.w3c", &w3c) ||
- dict->GetBoolean("chromeOptions.w3c", &w3c)) {
- return w3c;
- }
- }
-
- if (!params.HasKey("capabilities")) {
- return false;
- }
-
- return kW3CDefault;
-}
-
Status InitSessionHelper(const InitSessionParams& bound_params,
Session* session,
const base::DictionaryValue& params,
diff --git a/chrome/test/chromedriver/session_commands.h b/chrome/test/chromedriver/session_commands.h
index 81c6db190..2fcef40 100644
--- a/chrome/test/chromedriver/session_commands.h
+++ b/chrome/test/chromedriver/session_commands.h
@@ -34,6 +34,8 @@
DeviceManager* device_manager;
};
+bool GetW3CSetting(const base::DictionaryValue& params);
+
bool MergeCapabilities(const base::DictionaryValue* always_match,
const base::DictionaryValue* first_match,
base::DictionaryValue* merged);
diff --git a/chrome/test/chromedriver/session_thread_map.h b/chrome/test/chromedriver/session_thread_map.h
index c0bdaab..972b069d 100644
--- a/chrome/test/chromedriver/session_thread_map.h
+++ b/chrome/test/chromedriver/session_thread_map.h
@@ -11,6 +11,21 @@
#include "base/threading/thread.h"
-using SessionThreadMap = std::map<std::string, std::unique_ptr<base::Thread>>;
+// Info related to session threads, one instance per session. This object should
+// only be accessed on the main thread.
+class SessionThreadInfo {
+ public:
+ SessionThreadInfo(const std::string& name, bool w3c_mode)
+ : thread_(name), w3c_mode_(w3c_mode) {}
+ base::Thread* thread() { return &thread_; }
+ bool w3cMode() const { return w3c_mode_; }
+
+ private:
+ base::Thread thread_;
+ bool w3c_mode_;
+};
+
+using SessionThreadMap =
+ std::map<std::string, std::unique_ptr<SessionThreadInfo>>;
#endif // CHROME_TEST_CHROMEDRIVER_SESSION_THREAD_MAP_H_