[Extensions] Make extensions.getURL behaviour match runtime.getURL

This change makes it so that when passed a fully qualified URL, getURL
does not return the string as-is.

Bug: 984696
Change-Id: Ib82e41b13071bb6ec6fde5f98dbd2eade593d272
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1705579
Commit-Queue: Tim Judkins <tjudkins@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680044}

chrome/renderer/extensions/extension_hooks_delegate.cc

@@ -17,6 +17,7 @@
 #include "extensions/renderer/message_target.h"
 #include "extensions/renderer/messaging_util.h"
 #include "extensions/renderer/native_renderer_messaging_service.h"
+#include "extensions/renderer/runtime_hooks_delegate.h"
 #include "extensions/renderer/script_context.h"
 #include "gin/converter.h"
 #include "gin/dictionary.h"
@@ -238,17 +239,10 @@ RequestResult ExtensionHooksDelegate::HandleSendRequest(
 RequestResult ExtensionHooksDelegate::HandleGetURL(
     ScriptContext* script_context,
     const std::vector<v8::Local<v8::Value>>& arguments) {
-  DCHECK_EQ(1u, arguments.size());
-  DCHECK(arguments[0]->IsString());
-  DCHECK(script_context->extension());
-
-  std::string path = gin::V8ToString(script_context->isolate(), arguments[0]);
-
-  RequestResult result(RequestResult::HANDLED);
-  result.return_value =
-      gin::StringToV8(script_context->isolate(),
-                      script_context->extension()->GetResourceURL(path).spec());
-  return result;
+  // We call a static implementation here rather using an alias due to not being
+  // able to remove the extension.json GetURL entry, as it is used for generated
+  // documentation and api feature lists some other methods refer to.
+  return RuntimeHooksDelegate::GetURL(script_context, arguments);
 }
 
 APIBindingHooks::RequestResult ExtensionHooksDelegate::HandleGetViews(

chrome/renderer/extensions/extension_hooks_delegate_unittest.cc

@@ -237,4 +237,30 @@ TEST_F(ExtensionHooksDelegateTest, RuntimeAliasesCorrupted) {
                       context, 0, nullptr);
 }
 
+// Ensure that HandleGetURL allows extension URLs and doesn't allow arbitrary
+// non-extension URLs. Very similar to RuntimeHooksDeligateTest that tests a
+// similar function.
+TEST_F(ExtensionHooksDelegateTest, GetURL) {
+  v8::HandleScope handle_scope(isolate());
+  v8::Local<v8::Context> context = MainContext();
+
+  auto get_url = [this, context](const char* args, const GURL& expected_url) {
+    SCOPED_TRACE(base::StringPrintf("Args: `%s`", args));
+    constexpr char kGetUrlTemplate[] =
+        "(function() { return chrome.extension.getURL(%s); })";
+    v8::Local<v8::Function> get_url =
+        FunctionFromString(context, base::StringPrintf(kGetUrlTemplate, args));
+    v8::Local<v8::Value> url = RunFunction(get_url, context, 0, nullptr);
+    ASSERT_FALSE(url.IsEmpty());
+    ASSERT_TRUE(url->IsString());
+    EXPECT_EQ(expected_url.spec(), gin::V8ToString(isolate(), url));
+  };
+
+  get_url("''", extension()->url());
+  get_url("'foo'", extension()->GetResourceURL("foo"));
+  get_url("'/foo'", extension()->GetResourceURL("foo"));
+  get_url("'https://www.google.com'",
+          GURL(extension()->url().spec() + "https://www.google.com"));
+}
+
 }  // namespace extensions

extensions/renderer/runtime_hooks_delegate.cc

@@ -89,6 +89,28 @@ RuntimeHooksDelegate::RuntimeHooksDelegate(
     : messaging_service_(messaging_service) {}
 RuntimeHooksDelegate::~RuntimeHooksDelegate() {}
 
+// static
+RequestResult RuntimeHooksDelegate::GetURL(
+    ScriptContext* script_context,
+    const std::vector<v8::Local<v8::Value>>& arguments) {
+  DCHECK_EQ(1u, arguments.size());
+  DCHECK(arguments[0]->IsString());
+  DCHECK(script_context->extension());
+
+  v8::Isolate* isolate = script_context->isolate();
+  std::string path = gin::V8ToString(isolate, arguments[0]);
+
+  RequestResult result(RequestResult::HANDLED);
+  std::string url = base::StringPrintf(
+      "chrome-extension://%s%s%s", script_context->extension()->id().c_str(),
+      !path.empty() && path[0] == '/' ? "" : "/", path.c_str());
+  result.return_value = gin::StringToV8(isolate, url);
+  // TODO(tjudkins): Gather data on how often this is passed a bad URL (i.e. not
+  // a relative file path for the extension), so we can decide if it's fine to
+  // throw an error in those cases.
+  return result;
+}
+
 RequestResult RuntimeHooksDelegate::HandleRequest(
     const std::string& method_name,
     const APISignature* signature,
@@ -174,20 +196,7 @@ RequestResult RuntimeHooksDelegate::HandleGetManifest(
 RequestResult RuntimeHooksDelegate::HandleGetURL(
     ScriptContext* script_context,
     const std::vector<v8::Local<v8::Value>>& arguments) {
-  DCHECK_EQ(1u, arguments.size());
-  DCHECK(arguments[0]->IsString());
-  DCHECK(script_context->extension());
-
-  v8::Isolate* isolate = script_context->isolate();
-  std::string path = gin::V8ToString(isolate, arguments[0]);
-
-  RequestResult result(RequestResult::HANDLED);
-  std::string url = base::StringPrintf(
-      "chrome-extension://%s%s%s", script_context->extension()->id().c_str(),
-      !path.empty() && path[0] == '/' ? "" : "/", path.c_str());
-  result.return_value = gin::StringToV8(isolate, url);
-
-  return result;
+  return GetURL(script_context, arguments);
 }
 
 RequestResult RuntimeHooksDelegate::HandleSendMessage(

extensions/renderer/runtime_hooks_delegate.h

@@ -22,6 +22,14 @@ class RuntimeHooksDelegate : public APIBindingHooksDelegate {
       NativeRendererMessagingService* messaging_service);
   ~RuntimeHooksDelegate() override;
 
+  // Returns an absolute url for a path inside of an extension, as requested
+  // through the getURL API call.
+  // NOTE: Static as the logic is used by both the runtime and extension
+  // hooks.
+  static APIBindingHooks::RequestResult GetURL(
+      ScriptContext* script_context,
+      const std::vector<v8::Local<v8::Value>>& arguments);
+
   // APIBindingHooksDelegate:
   APIBindingHooks::RequestResult HandleRequest(
       const std::string& method_name,

extensions/renderer/runtime_hooks_delegate_unittest.cc

@@ -168,6 +168,8 @@ TEST_F(RuntimeHooksDelegateTest, GetURL) {
   get_url("''", extension()->url());
   get_url("'foo'", extension()->GetResourceURL("foo"));
   get_url("'/foo'", extension()->GetResourceURL("foo"));
+  get_url("'https://www.google.com'",
+          GURL(extension()->url().spec() + "https://www.google.com"));
 }
 
 TEST_F(RuntimeHooksDelegateTest, Connect) {
@@ -253,15 +255,12 @@ TEST_F(RuntimeHooksDelegateTest, SendMessage) {
   tester.TestSendMessage("null, {data: 'hello'}, function() {}",
                          kStandardMessage, self_target, false,
                          SendMessageTester::OPEN);
-  tester.TestSendMessage("null, 'test', function() {}",
-                         R"("test")", self_target, false,
-                         SendMessageTester::OPEN);
-  tester.TestSendMessage("null, 'test'",
-                         R"("test")", self_target, false,
+  tester.TestSendMessage("null, 'test', function() {}", R"("test")",
+                         self_target, false, SendMessageTester::OPEN);
+  tester.TestSendMessage("null, 'test'", R"("test")", self_target, false,
                          SendMessageTester::CLOSED);
-  tester.TestSendMessage("undefined, 'test', function() {}",
-                         R"("test")", self_target, false,
-                         SendMessageTester::OPEN);
+  tester.TestSendMessage("undefined, 'test', function() {}", R"("test")",
+                         self_target, false, SendMessageTester::OPEN);
 
   // Funny case. The only required argument is `message`, which can be any type.
   // This means that if an extension provides a <string, object> pair for the