Fix web accessible resource checks in ShouldAllowOpenURL

content/browser/frame_host/navigator_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigator_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 717 matching lines @@
       render_frame_host->frame_tree_node()->current_frame_host()) {
     return;
   }
 
   SiteInstance* current_site_instance = render_frame_host->GetSiteInstance();
 
   // TODO(creis): Pass the redirect_chain into this method to support client
   // redirects.  http://crbug.com/311721.
   std::vector<GURL> redirect_chain;
 
+  // Note that unlike RequestTransferURL, this uses the navigating
+  // RenderFrameHost's current SiteInstance, as that's where this navigation
+  // originated.
   GURL dest_url(url);
   if (!GetContentClient()->browser()->ShouldAllowOpenURL(
           current_site_instance, url)) {
     dest_url = GURL(url::kAboutBlankURL);
   }
 
   int frame_tree_node_id = -1;
 
   // Send the navigation to the current FrameTreeNode if it's destined for a
   // subframe in the current tab.  We'll assume it's for the main frame
@@ skipping 67 matching lines @@
 
   // Allow the delegate to cancel the transfer.
   if (!delegate_->ShouldTransferNavigation(
           render_frame_host->frame_tree_node()->IsMainFrame()))
     return;
 
   GURL dest_url(url);
   Referrer referrer_to_use(referrer);
   FrameTreeNode* node = render_frame_host->frame_tree_node();
   SiteInstance* current_site_instance = render_frame_host->GetSiteInstance();
-  if (!GetContentClient()->browser()->ShouldAllowOpenURL(current_site_instance,
-                                                         url)) {
-    dest_url = GURL(url::kAboutBlankURL);
+  // It is important to pass in the source_site_instance if it is available
+  // (such as when navigating a proxy).  See https://crbug.com/656752.
+  if (!GetContentClient()->browser()->ShouldAllowOpenURL(
+          source_site_instance ? source_site_instance : current_site_instance,
+          url)) {
+    // It is important to return here, rather than rewrite the dest_url to
+    // about:blank.  The latter won't actually have any effect when
+    // transferring, as NavigateToEntry will think that the transfer is to the
+    // same RFH that started the navigation and let the existing navigation
+    // (for the disallowed URL) proceed.
+    return;
   }
 
   // TODO(creis): Determine if this transfer started as a browser-initiated
   // navigation.  See https://crbug.com/495161.
   bool is_renderer_initiated = true;
   if (render_frame_host->web_ui()) {
     // Web UI pages sometimes want to override the page transition type for
     // link clicks (e.g., so the new tab page can specify AUTO_BOOKMARK for
     // automatically generated suggestions).  We don't override other types
     // like TYPED because they have different implications (e.g., autocomplete).
@@ skipping 423 matching lines @@
     if (navigation_handle)
       navigation_handle->update_entry_id_for_transfer(entry->GetUniqueID());
 
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content