Validate debuggee.targetId before use in chrome.debugger
And refactored the tests to make sure that the debugger is detached upon
returning from RunAttachFunction. Previously, if the debugger
unexpectedly succeeded in attaching, the method would return (because
empty error != some error), causing the attached debugger to not be
detached.
R=rdevlin.cronin@chromium.org
BUG=456841
TEST=browser_tests DebuggerApiTest.DebuggerNotAllowedOnOtherExtensionPages
Review URL: https://codereview.chromium.org/910053002
Cr-Commit-Position: refs/heads/master@{#315675}
diff --git a/chrome/browser/extensions/api/debugger/debugger_api.cc b/chrome/browser/extensions/api/debugger/debugger_api.cc
index f927fcb..c2f701f 100644
--- a/chrome/browser/extensions/api/debugger/debugger_api.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_api.cc
@@ -529,6 +529,15 @@
}
} else if (debuggee_.target_id) {
agent_host_ = DevToolsAgentHost::GetForId(*debuggee_.target_id);
+ if (agent_host_.get()) {
+ if (PermissionsData::IsRestrictedUrl(agent_host_->GetURL(),
+ agent_host_->GetURL(),
+ extension(),
+ &error_)) {
+ agent_host_ = nullptr;
+ return false;
+ }
+ }
} else {
error_ = keys::kInvalidTargetError;
return false;
diff --git a/chrome/browser/extensions/api/debugger/debugger_apitest.cc b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
index 64dbc12..0d81858 100644
--- a/chrome/browser/extensions/api/debugger/debugger_apitest.cc
+++ b/chrome/browser/extensions/api/debugger/debugger_apitest.cc
@@ -42,6 +42,9 @@
base::CommandLine* command_line() const { return command_line_; }
private:
+ testing::AssertionResult RunAttachFunctionOnTarget(
+ const std::string& debuggee_target, const std::string& expected_error);
+
// The command-line for the test process, preserved in order to modify
// mid-test.
base::CommandLine* command_line_;
@@ -72,41 +75,75 @@
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents();
+
+ // Attach by tabId.
int tab_id = SessionTabHelper::IdForTab(web_contents);
+ std::string debugee_by_tab = base::StringPrintf("{\"tabId\": %d}", tab_id);
+ testing::AssertionResult result =
+ RunAttachFunctionOnTarget(debugee_by_tab, expected_error);
+ if (!result)
+ return result;
+
+ // Attach by targetId.
+ scoped_refptr<DebuggerGetTargetsFunction> get_targets_function =
+ new DebuggerGetTargetsFunction();
+ scoped_ptr<base::Value> value(
+ extension_function_test_utils::RunFunctionAndReturnSingleResult(
+ get_targets_function.get(), "[]", browser()));
+ base::ListValue* targets = nullptr;
+ EXPECT_TRUE(value->GetAsList(&targets));
+
+ std::string debugger_target_id;
+ for (size_t i = 0; i < targets->GetSize(); ++i) {
+ base::DictionaryValue* target_dict = nullptr;
+ EXPECT_TRUE(targets->GetDictionary(i, &target_dict));
+ int id = -1;
+ if (target_dict->GetInteger("tabId", &id) && id == tab_id) {
+ EXPECT_TRUE(target_dict->GetString("id", &debugger_target_id));
+ break;
+ }
+ }
+ EXPECT_TRUE(!debugger_target_id.empty());
+
+ std::string debugee_by_target_id =
+ base::StringPrintf("{\"targetId\": \"%s\"}", debugger_target_id.c_str());
+ return RunAttachFunctionOnTarget(debugee_by_target_id, expected_error);
+}
+
+testing::AssertionResult DebuggerApiTest::RunAttachFunctionOnTarget(
+ const std::string& debuggee_target, const std::string& expected_error) {
scoped_refptr<DebuggerAttachFunction> attach_function =
new DebuggerAttachFunction();
attach_function->set_extension(extension_.get());
- std::string args = base::StringPrintf("[{\"tabId\": %d}, \"1.1\"]", tab_id);
- if (!expected_error.empty()) {
- std::string actual_error =
- extension_function_test_utils::RunFunctionAndReturnError(
- attach_function.get(), args, browser());
- if (actual_error != expected_error) {
- return testing::AssertionFailure() << "Did not get correct error: "
- << "expected: " << expected_error << ", found: " << actual_error;
- }
+ std::string actual_error;
+ if (!RunFunction(attach_function.get(),
+ base::StringPrintf("[%s, \"1.1\"]", debuggee_target.c_str()),
+ browser(),
+ extension_function_test_utils::NONE)) {
+ actual_error = attach_function->GetError();
} else {
- if (!RunFunction(attach_function.get(),
- args,
- browser(),
- extension_function_test_utils::NONE)) {
- return testing::AssertionFailure() << "Could not run function: "
- << attach_function->GetError();
- }
-
// Clean up and detach.
scoped_refptr<DebuggerDetachFunction> detach_function =
new DebuggerDetachFunction();
detach_function->set_extension(extension_.get());
if (!RunFunction(detach_function.get(),
- base::StringPrintf("[{\"tabId\": %d}]", tab_id),
+ base::StringPrintf("[%s]", debuggee_target.c_str()),
browser(),
extension_function_test_utils::NONE)) {
- return testing::AssertionFailure() << "Could not detach: "
- << detach_function->GetError();
+ return testing::AssertionFailure() << "Could not detach from "
+ << debuggee_target << " : " << detach_function->GetError();
}
}
+
+ if (expected_error.empty() && !actual_error.empty()) {
+ return testing::AssertionFailure() << "Could not attach to "
+ << debuggee_target << " : " << actual_error;
+ } else if (actual_error != expected_error) {
+ return testing::AssertionFailure() << "Did not get correct error upon "
+ << "attach to " << debuggee_target << " : "
+ << "expected: " << expected_error << ", found: " << actual_error;
+ }
return testing::AssertionSuccess();
}
