Only allow downloading in response to real keyboard modifiers

BUG=848531 Change-Id: I97554c8d312243b55647f1376945aee32dbd95bf Reviewed-on: https://chromium-review.googlesource.com/1082216 Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/heads/master@{#564051}
marcosholgado · Jun 4, 2018 · 4379a7f · 4379a7f
1 parent cbc5dfe
commit 4379a7f
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 30 deletions.
diff --git a/third_party/WebKit/LayoutTests/fast/events/download-on-alt-click-expected.txt b/third_party/WebKit/LayoutTests/fast/events/download-on-alt-click-expected.txt
@@ -0,0 +1,4 @@
+Download started
+Tests that hitting alt-click results in downloading link. Test passes if a download is observed.
+
+link
diff --git a/third_party/WebKit/LayoutTests/fast/events/download-on-alt-click.html b/third_party/WebKit/LayoutTests/fast/events/download-on-alt-click.html
@@ -0,0 +1,20 @@
+<!doctype html>
+<script>
+function test()
+{
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+        testRunner.waitUntilExternalURLLoad();
+
+        var a = document.querySelector("#link");
+        eventSender.mouseMoveTo(a.offsetLeft + 10, a.offsetTop + 10);
+        eventSender.mouseDown(0, ['altKey']);
+        eventSender.mouseUp(0, ['altKey']);
+    }
+}
+</script>
+<body onload="test()">
+<p>Tests that hitting alt-click results in downloading link. Test passes if a download is observed.</p>
+<a href="resources/notify-done.html" id="link">link</a>
+</body>
diff --git a/third_party/WebKit/LayoutTests/fast/events/download-on-synthesized-alt-click-expected.txt b/third_party/WebKit/LayoutTests/fast/events/download-on-synthesized-alt-click-expected.txt
@@ -0,0 +1 @@
+
diff --git a/third_party/WebKit/LayoutTests/fast/events/download-on-synthesized-alt-click.html b/third_party/WebKit/LayoutTests/fast/events/download-on-synthesized-alt-click.html
@@ -0,0 +1,19 @@
+<!doctype html>
+<script>
+function test()
+{
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+        testRunner.waitUntilExternalURLLoad();
+
+        var a = document.querySelector("#link");
+        var evt = new MouseEvent("click", { altKey: true });
+        a.dispatchEvent(evt);
+    }
+}
+</script>
+<body onload="test()">
+<p>Tests that synthesizing alt-click does not result in downloading link. Test passes if no download is observed.</p>
+<a href="resources/notify-done.html" id="link">link</a>
+</body>
diff --git a/third_party/WebKit/LayoutTests/fast/events/resources/notify-done.html b/third_party/WebKit/LayoutTests/fast/events/resources/notify-done.html
@@ -0,0 +1,5 @@
+<!doctype html>
+<script>
+  if (window.testRunner)
+    testRunner.notifyDone();
+</script>
diff --git a/third_party/blink/renderer/core/loader/frame_loader.cc b/third_party/blink/renderer/core/loader/frame_loader.cc
@@ -51,6 +51,7 @@
 #include "third_party/blink/renderer/core/dom/element.h"
 #include "third_party/blink/renderer/core/dom/events/event.h"
 #include "third_party/blink/renderer/core/dom/viewport_description.h"
+#include "third_party/blink/renderer/core/events/current_input_event.h"
 #include "third_party/blink/renderer/core/events/gesture_event.h"
 #include "third_party/blink/renderer/core/events/keyboard_event.h"
 #include "third_party/blink/renderer/core/events/mouse_event.h"
@@ -805,16 +806,8 @@ static WebURLRequest::RequestContext DetermineRequestContextFromNavigationType(
   return WebURLRequest::kRequestContextHyperlink;
 }
 
-static NavigationPolicy NavigationPolicyForRequest(
-    const FrameLoadRequest& request) {
+static NavigationPolicy NavigationPolicyForEvent(Event* event) {
   NavigationPolicy policy = kNavigationPolicyCurrentTab;
-  Event* event = request.TriggeringEvent();
-  if (!event)
-    return policy;
-
-  if (request.Form() && event->UnderlyingEvent())
-    event = event->UnderlyingEvent();
-
   if (event->IsMouseEvent()) {
     MouseEvent* mouse_event = ToMouseEvent(event);
     NavigationPolicyFromMouseEvent(
@@ -836,6 +829,27 @@ static NavigationPolicy NavigationPolicyForRequest(
   return policy;
 }
 
+static NavigationPolicy NavigationPolicyForRequest(
+    const FrameLoadRequest& request) {
+  NavigationPolicy policy = kNavigationPolicyCurrentTab;
+  Event* event = request.TriggeringEvent();
+  if (!event)
+    return policy;
+
+  if (request.Form() && event->UnderlyingEvent())
+    event = event->UnderlyingEvent();
+
+  policy = NavigationPolicyForEvent(event);
+
+  if (policy == kNavigationPolicyDownload &&
+      EffectiveNavigationPolicy(policy, CurrentInputEvent::Get(),
+                                WebWindowFeatures()) !=
+          kNavigationPolicyDownload) {
+    return kNavigationPolicyCurrentTab;
+  }
+  return policy;
+}
+
 void FrameLoader::StartNavigation(const FrameLoadRequest& passed_request,
                                   FrameLoadType frame_load_type,
                                   HistoryItem* history_item) {

diff --git a/third_party/blink/renderer/core/page/create_window.cc b/third_party/blink/renderer/core/page/create_window.cc
@@ -98,12 +98,6 @@ void UpdatePolicyForEvent(const WebInputEvent* input_event,
   NavigationPolicyFromMouseEvent(button_number, ctrl, shift, alt, meta,
                                  &user_policy);
 
-  // When the input event suggests a download, but the navigation was initiated
-  // by script, we should not override it.
-  if (user_policy == kNavigationPolicyDownload &&
-      *policy != kNavigationPolicyIgnore)
-    return;
-
   // User and app agree that we want a new window; let the app override the
   // decorations.
   if (user_policy == kNavigationPolicyNewWindow &&
@@ -112,33 +106,48 @@ void UpdatePolicyForEvent(const WebInputEvent* input_event,
   *policy = user_policy;
 }
 
-NavigationPolicy GetNavigationPolicy(const WebInputEvent* current_event,
-                                     const WebWindowFeatures& features) {
+}  // anonymous namespace
+
+// Check that the desired NavigationPolicy |policy| is compatible with the
+// observed input event |current_event|.
+NavigationPolicy EffectiveNavigationPolicy(NavigationPolicy policy,
+                                           const WebInputEvent* current_event,
+                                           const WebWindowFeatures& features) {
   // If our default configuration was modified by a script or wasn't
   // created by a user gesture, then show as a popup. Else, let this
   // new window be opened as a toplevel window.
   bool as_popup = !features.tool_bar_visible || !features.status_bar_visible ||
                   !features.scrollbars_visible || !features.menu_bar_visible ||
                   !features.resizable;
-  NavigationPolicy policy =
+  NavigationPolicy user_policy =
       as_popup ? kNavigationPolicyNewPopup : kNavigationPolicyNewForegroundTab;
-  UpdatePolicyForEvent(current_event, &policy);
-  return policy;
-}
-
-}  // anonymous namespace
+  UpdatePolicyForEvent(current_event, &user_policy);
+
+  if (policy == kNavigationPolicyIgnore) {
+    // When the input event suggests a download, but the navigation was
+    // initiated by script, we should not override it.
+    if (user_policy == kNavigationPolicyDownload) {
+      return as_popup ? kNavigationPolicyNewPopup
+                      : kNavigationPolicyNewForegroundTab;
+    }
+    return user_policy;
+  }
 
-NavigationPolicy EffectiveNavigationPolicy(NavigationPolicy policy,
-                                           const WebInputEvent* current_event,
-                                           const WebWindowFeatures& features) {
-  if (policy == kNavigationPolicyIgnore)
-    return GetNavigationPolicy(current_event, features);
   if (policy == kNavigationPolicyNewBackgroundTab &&
-      GetNavigationPolicy(current_event, features) !=
-          kNavigationPolicyNewBackgroundTab &&
+      user_policy != kNavigationPolicyNewBackgroundTab &&
       !UIEventWithKeyState::NewTabModifierSetFromIsolatedWorld()) {
+    // Don't allow background tabs to be opened via script setting the
+    // event modifiers.
     return kNavigationPolicyNewForegroundTab;
   }
+
+  if (policy == kNavigationPolicyDownload &&
+      user_policy != kNavigationPolicyDownload) {
+    // Don't allow downloads to be triggered via script setting the event
+    // modifiers.
+    return kNavigationPolicyNewForegroundTab;
+  }
+
   return policy;
 }