Commit 1454e0ce authored by mek's avatar mek Committed by Commit bot

Reject connection attempts without prompt if target app doesn't handle the event anyway.

If a connection is not going to succeed, the user shouldn't be prompted to allow it.

BUG=442497

Review URL: https://codereview.chromium.org/800853006

Cr-Commit-Position: refs/heads/master@{#309569}
parent 37edfbac
......@@ -30,6 +30,7 @@
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/site_instance.h"
#include "content/public/browser/web_contents.h"
#include "extensions/browser/event_router.h"
#include "extensions/browser/extension_host.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_system.h"
......@@ -343,8 +344,25 @@ void MessageService::OpenChannelToExtension(
// enabling in incognito. In practice this means platform apps only.
if (!is_web_connection || IncognitoInfo::IsSplitMode(target_extension) ||
target_extension->can_be_incognito_enabled()) {
DispatchOnDisconnect(source, receiver_port_id,
kReceivingEndDoesntExistError);
OnOpenChannelAllowed(params.Pass(), false);
return;
}
// If the target extension isn't even listening for connect/message events,
// there is no need to go any further and the connection should be
// rejected without showing a prompt. See http://crbug.com/442497
EventRouter* event_router = EventRouter::Get(context);
const char* const events[] = {"runtime.onConnectExternal",
"runtime.onMessageExternal",
"extension.onRequestExternal",
nullptr};
bool has_event_listener = false;
for (const char* const* event = events; *event; event++) {
has_event_listener |=
event_router->ExtensionHasEventListener(target_extension_id, *event);
}
if (!has_event_listener) {
OnOpenChannelAllowed(params.Pass(), false);
return;
}
......
......@@ -306,7 +306,8 @@ class ExternallyConnectableMessagingTest : public ExtensionApiTest {
return extension;
}
scoped_refptr<const Extension> LoadChromiumConnectableApp() {
scoped_refptr<const Extension> LoadChromiumConnectableApp(
bool with_event_handlers = true) {
scoped_refptr<const Extension> extension =
LoadExtensionIntoDir(&web_connectable_dir_,
"{"
......@@ -321,7 +322,8 @@ class ExternallyConnectableMessagingTest : public ExtensionApiTest {
" \"manifest_version\": 2,"
" \"name\": \"app_connectable\","
" \"version\": \"1.0\""
"}");
"}",
with_event_handlers);
CHECK(extension.get());
return extension;
}
......@@ -382,26 +384,33 @@ class ExternallyConnectableMessagingTest : public ExtensionApiTest {
private:
scoped_refptr<const Extension> LoadExtensionIntoDir(
TestExtensionDir* dir,
const std::string& manifest) {
const std::string& manifest,
bool with_event_handlers = true) {
dir->WriteManifest(manifest);
dir->WriteFile(FILE_PATH_LITERAL("background.js"),
base::StringPrintf(
"function maybeClose(message) {\n"
" if (message.indexOf('%s') >= 0)\n"
" window.setTimeout(function() { window.close() }, 0);\n"
"}\n"
"chrome.runtime.onMessageExternal.addListener(\n"
" function(message, sender, reply) {\n"
" reply({ message: message, sender: sender });\n"
" maybeClose(message);\n"
"});\n"
"chrome.runtime.onConnectExternal.addListener(function(port) {\n"
" port.onMessage.addListener(function(message) {\n"
" port.postMessage({ message: message, sender: port.sender });\n"
" maybeClose(message);\n"
" });\n"
"});\n",
close_background_message()));
if (with_event_handlers) {
dir->WriteFile(
FILE_PATH_LITERAL("background.js"),
base::StringPrintf(
"function maybeClose(message) {\n"
" if (message.indexOf('%s') >= 0)\n"
" window.setTimeout(function() { window.close() }, 0);\n"
"}\n"
"chrome.runtime.onMessageExternal.addListener(\n"
" function(message, sender, reply) {\n"
" reply({ message: message, sender: sender });\n"
" maybeClose(message);\n"
"});\n"
"chrome.runtime.onConnectExternal.addListener(function(port) {\n"
" port.onMessage.addListener(function(message) {\n"
" port.postMessage({ message: message, sender: port.sender "
"});\n"
" maybeClose(message);\n"
" });\n"
"});\n",
close_background_message()));
} else {
dir->WriteFile(FILE_PATH_LITERAL("background.js"), "");
}
return LoadExtension(dir->unpacked_path());
}
......@@ -735,6 +744,36 @@ IN_PROC_BROWSER_TEST_F(ExternallyConnectableMessagingTest,
CanConnectAndSendMessagesToFrame(incognito_frame, extension.get(), NULL));
}
// Tests connection from incognito tabs when the extension doesn't have an event
// handler for the connection event.
IN_PROC_BROWSER_TEST_F(ExternallyConnectableMessagingTest,
FromIncognitoNoEventHandlerInApp) {
InitializeTestServer();
scoped_refptr<const Extension> app = LoadChromiumConnectableApp(false);
ASSERT_TRUE(app->is_platform_app());
Browser* incognito_browser = ui_test_utils::OpenURLOffTheRecord(
profile()->GetOffTheRecordProfile(), chromium_org_url());
content::RenderFrameHost* incognito_frame =
incognito_browser->tab_strip_model()
->GetActiveWebContents()
->GetMainFrame();
{
IncognitoConnectability::ScopedAlertTracker alert_tracker(
IncognitoConnectability::ScopedAlertTracker::ALWAYS_ALLOW);
// No connection because incognito-enabled hasn't been set for the app, and
// the app hasn't installed event handlers.
EXPECT_EQ(
COULD_NOT_ESTABLISH_CONNECTION_ERROR,
CanConnectAndSendMessagesToFrame(incognito_frame, app.get(), NULL));
// No dialog should have been shown.
EXPECT_EQ(0, alert_tracker.GetAndResetAlertCount());
}
}
// Tests connection from incognito tabs when the user accepts the connection
// request. Spanning mode only. Separate tests for apps and extensions.
//
......
......@@ -203,9 +203,6 @@
"name": "onRequestExternal",
"deprecated": "Please use $(ref:runtime.onMessageExternal).",
"type": "function",
"options": {
"unmanaged": true
},
"description": "Fired when a request is sent from another extension.",
"parameters": [
{"name": "request", "type": "any", "optional": true, "description": "The request sent by the calling script."},
......
......@@ -424,9 +424,6 @@
"name": "onConnectExternal",
"type": "function",
"nocompile": true,
"options": {
"unmanaged": true
},
"description": "Fired when a connection is made from another extension.",
"parameters": [
{"$ref": "Port", "name": "port"}
......@@ -435,10 +432,10 @@
{
"name": "onMessage",
"type": "function",
"nocompile": true,
"options": {
"unmanaged": true
},
"nocompile": true,
"description": "Fired when a message is sent from either an extension process or a content script.",
"parameters": [
{"name": "message", "type": "any", "optional": true, "description": "The message sent by the calling script."},
......@@ -454,9 +451,6 @@
{
"name": "onMessageExternal",
"type": "function",
"options": {
"unmanaged": true
},
"nocompile": true,
"description": "Fired when a message is sent from another extension/app. Cannot be used in a content script.",
"parameters": [
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment