Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(88)

Unified Diff: chrome/browser/devtools/devtools_ui_bindings.cc

Issue 2607833002: DevTools: move front-end URL handling to DevToolsUIBindingds (Closed)
Patch Set: review comments addressed Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/devtools/devtools_ui_bindings.cc
diff --git a/chrome/browser/devtools/devtools_ui_bindings.cc b/chrome/browser/devtools/devtools_ui_bindings.cc
index 4c20eb075419620072d9b34c633245d027c39eda..97fd7941a846c731941d44d7d27f64985427f1b7 100644
--- a/chrome/browser/devtools/devtools_ui_bindings.cc
+++ b/chrome/browser/devtools/devtools_ui_bindings.cc
@@ -17,6 +17,7 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
@@ -26,6 +27,7 @@
#include "chrome/browser/devtools/devtools_file_watcher.h"
#include "chrome/browser/devtools/devtools_protocol.h"
#include "chrome/browser/devtools/global_confirm_info_bar.h"
+#include "chrome/browser/devtools/url_constants.h"
#include "chrome/browser/extensions/chrome_extension_web_contents_observer.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/profiles/profile.h"
@@ -62,8 +64,10 @@
#include "extensions/common/constants.h"
#include "extensions/common/permissions/permissions_data.h"
#include "ipc/ipc_channel.h"
+#include "net/base/escape.h"
#include "net/base/io_buffer.h"
#include "net/base/net_errors.h"
+#include "net/base/url_util.h"
#include "net/cert/x509_certificate.h"
#include "net/http/http_response_headers.h"
#include "net/url_request/url_fetcher.h"
@@ -299,6 +303,132 @@ int ResponseWriter::Finish(int net_error,
return net::OK;
}
+GURL SanitizeFrontendURL(
+ const GURL& url,
+ const std::string& scheme,
+ const std::string& host,
+ const std::string& path,
+ bool allow_query);
+
+std::string SanitizeRevision(const std::string& revision) {
+ for (size_t i = 0; i < revision.length(); i++) {
+ if (!(revision[i] == '@' && i == 0)
+ && !(revision[i] >= '0' && revision[i] <= '9')
+ && !(revision[i] >= 'a' && revision[i] <= 'z')
+ && !(revision[i] >= 'A' && revision[i] <= 'Z')) {
+ return std::string();
+ }
+ }
+ return revision;
+}
+
+std::string SanitizeFrontendPath(const std::string& path) {
+ for (size_t i = 0; i < path.length(); i++) {
+ if (path[i] != '/' && path[i] != '-' && path[i] != '_'
+ && path[i] != '.' && path[i] != '@'
+ && !(path[i] >= '0' && path[i] <= '9')
+ && !(path[i] >= 'a' && path[i] <= 'z')
+ && !(path[i] >= 'A' && path[i] <= 'Z')) {
+ return std::string();
+ }
+ }
+ return path;
+}
+
+std::string SanitizeEndpoint(const std::string& value) {
+ if (value.find('&') != std::string::npos
+ || value.find('?') != std::string::npos)
+ return std::string();
+ return value;
+}
+
+std::string SanitizeRemoteBase(const std::string& value) {
+ GURL url(value);
+ std::string path = url.path();
+ std::vector<std::string> parts = base::SplitString(
+ path, "/", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+ std::string revision = parts.size() > 2 ? parts[2] : "";
+ revision = SanitizeRevision(revision);
+ path = base::StringPrintf("/%s/%s/", kRemoteFrontendPath, revision.c_str());
+ return SanitizeFrontendURL(url, url::kHttpsScheme,
+ kRemoteFrontendDomain, path, false).spec();
+}
+
+std::string SanitizeRemoteFrontendURL(const std::string& value) {
+ GURL url(net::UnescapeURLComponent(value,
+ net::UnescapeRule::SPACES | net::UnescapeRule::PATH_SEPARATORS |
+ net::UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS |
+ net::UnescapeRule::REPLACE_PLUS_WITH_SPACE));
+ std::string path = url.path();
+ std::vector<std::string> parts = base::SplitString(
+ path, "/", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+ std::string revision = parts.size() > 2 ? parts[2] : "";
+ revision = SanitizeRevision(revision);
+ std::string filename = parts.size() ? parts[parts.size() - 1] : "";
+ if (filename != "devtools.html")
+ filename = "inspector.html";
+ path = base::StringPrintf("/serve_rev/%s/%s",
+ revision.c_str(), filename.c_str());
+ std::string sanitized = SanitizeFrontendURL(url, url::kHttpsScheme,
+ kRemoteFrontendDomain, path, true).spec();
+ return net::EscapeQueryParamValue(sanitized, false);
+}
+
+std::string SanitizeFrontendQueryParam(
+ const std::string& key,
+ const std::string& value) {
+ // Convert boolean flags to true.
+ if (key == "can_dock" || key == "debugFrontend" || key == "experiments" ||
+ key == "isSharedWorker" || key == "v8only" || key == "remoteFrontend")
+ return "true";
+
+ // Pass connection endpoints as is.
+ if (key == "ws" || key == "service-backend")
+ return SanitizeEndpoint(value);
+
+ // Only support undocked for old frontends.
+ if (key == "dockSide" && value == "undocked")
+ return value;
+
+ if (key == "panel" && (value == "elements" || value == "console"))
+ return value;
+
+ if (key == "remoteBase")
+ return SanitizeRemoteBase(value);
+
+ if (key == "remoteFrontendUrl")
+ return SanitizeRemoteFrontendURL(value);
+
+ return std::string();
+}
+
+GURL SanitizeFrontendURL(
+ const GURL& url,
+ const std::string& scheme,
+ const std::string& host,
+ const std::string& path,
+ bool allow_query) {
+ std::vector<std::string> query_parts;
+ if (allow_query) {
+ for (net::QueryIterator it(url); !it.IsAtEnd(); it.Advance()) {
+ std::string value = SanitizeFrontendQueryParam(it.GetKey(),
+ it.GetValue());
+ if (!value.empty()) {
+ query_parts.push_back(
+ base::StringPrintf("%s=%s", it.GetKey().c_str(), value.c_str()));
+ }
+ }
+ }
+ std::string query =
+ query_parts.empty() ? "" : "?" + base::JoinString(query_parts, "&");
+ std::string constructed = base::StringPrintf("%s://%s%s%s",
+ scheme.c_str(), host.c_str(), path.c_str(), query.c_str());
+ GURL result = GURL(constructed);
+ if (!result.is_valid())
+ return GURL();
+ return result;
+}
+
} // namespace
// DevToolsUIBindings::FrontendWebContentsObserver ----------------------------
@@ -335,6 +465,12 @@ DevToolsUIBindings::FrontendWebContentsObserver::
~FrontendWebContentsObserver() {
}
+// static
+GURL DevToolsUIBindings::SanitizeFrontendURL(const GURL& url) {
+ return ::SanitizeFrontendURL(url, content::kChromeDevToolsScheme,
+ chrome::kChromeUIDevToolsHost, SanitizeFrontendPath(url.path()), true);
+}
+
void DevToolsUIBindings::FrontendWebContentsObserver::RenderProcessGone(
base::TerminationStatus status) {
bool crashed = true;
@@ -359,11 +495,7 @@ void DevToolsUIBindings::FrontendWebContentsObserver::RenderProcessGone(
void DevToolsUIBindings::FrontendWebContentsObserver::
DidStartNavigationToPendingEntry(const GURL& url,
content::ReloadType reload_type) {
- devtools_bindings_->frontend_host_.reset(
- content::DevToolsFrontendHost::Create(
- web_contents()->GetMainFrame(),
- base::Bind(&DevToolsUIBindings::HandleMessageFromDevToolsFrontend,
- base::Unretained(devtools_bindings_))));
+ devtools_bindings_->UpdateFrontendHost();
}
void DevToolsUIBindings::FrontendWebContentsObserver::
@@ -418,11 +550,7 @@ DevToolsUIBindings::DevToolsUIBindings(content::WebContents* web_contents)
// Register on-load actions.
embedder_message_dispatcher_.reset(
DevToolsEmbedderMessageDispatcher::CreateForDevToolsFrontend(this));
-
- frontend_host_.reset(content::DevToolsFrontendHost::Create(
- web_contents_->GetMainFrame(),
- base::Bind(&DevToolsUIBindings::HandleMessageFromDevToolsFrontend,
- base::Unretained(this))));
+ UpdateFrontendHost();
}
DevToolsUIBindings::~DevToolsUIBindings() {
@@ -883,6 +1011,8 @@ void DevToolsUIBindings::DispatchProtocolMessageFromDevToolsFrontend(
void DevToolsUIBindings::RecordEnumeratedHistogram(const std::string& name,
int sample,
int boundary_value) {
+ if (!frontend_host_)
+ return;
if (!(boundary_value >= 0 && boundary_value <= 100 && sample >= 0 &&
sample < boundary_value)) {
// TODO(nick): Replace with chrome::bad_message::ReceivedBadMessage().
@@ -1068,6 +1198,20 @@ void DevToolsUIBindings::ShowDevToolsConfirmInfoBar(
GlobalConfirmInfoBar::Show(std::move(delegate));
}
+void DevToolsUIBindings::UpdateFrontendHost() {
+ GURL url = web_contents_->GetVisibleURL();
+ if (url.spec() != SanitizeFrontendURL(url).spec()) {
+ LOG(ERROR) << "Attempt to navigate to an invalid DevTools front-end URL: "
+ << url.spec();
+ frontend_host_.reset();
+ return;
+ }
+ frontend_host_.reset(content::DevToolsFrontendHost::Create(
+ web_contents_->GetMainFrame(),
+ base::Bind(&DevToolsUIBindings::HandleMessageFromDevToolsFrontend,
+ base::Unretained(this))));
+}
+
void DevToolsUIBindings::AddDevToolsExtensionsToClient() {
const extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(profile_->GetOriginalProfile());
@@ -1141,6 +1285,9 @@ void DevToolsUIBindings::CallClientFunction(const std::string& function_name,
const base::Value* arg3) {
if (!web_contents_->GetURL().SchemeIs(content::kChromeDevToolsScheme))
return;
+ // If we're not exposing bindings, we shouldn't call functions either.
+ if (!frontend_host_)
+ return;
std::string javascript = function_name + "(";
if (arg1) {
std::string json;
« no previous file with comments | « chrome/browser/devtools/devtools_ui_bindings.h ('k') | chrome/browser/devtools/devtools_ui_bindings_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698