Fix Self-Referencing OOPIF Infinite Loop

content/browser/frame_host/navigation_handle_impl.cc

 // Copyright 2015 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/navigation_handle_impl.h"
 
 #include "base/debug/dump_without_crashing.h"
 #include "base/logging.h"
 #include "content/browser/appcache/appcache_navigation_handle.h"
 #include "content/browser/appcache/appcache_service_impl.h"
@@ skipping 420 matching lines @@
   if (method_ == "POST")
     resource_request_body_ = resource_request_body;
   sanitized_referrer_ = sanitized_referrer;
   has_user_gesture_ = has_user_gesture;
   transition_ = transition;
   is_external_protocol_ = is_external_protocol;
   request_context_type_ = request_context_type;
   state_ = WILL_SEND_REQUEST;
   complete_callback_ = callback;
 
+  if (IsSelfReferentialURL()) {
+    state_ = CANCELING;
+    RunCompleteCallback(NavigationThrottle::CANCEL);
+    return;
+  }
+
   RegisterNavigationThrottles();
 
   if (IsBrowserSideNavigationEnabled())
     navigation_ui_data_ = GetDelegate()->GetNavigationUIData(this);
 
   // Notify each throttle of the request.
   NavigationThrottle::ThrottleCheckResult result = CheckWillStartRequest();
 
   // If the navigation is not deferred, run the callback.
   if (result != NavigationThrottle::DEFER)
@@ skipping 17 matching lines @@
   response_headers_ = response_headers;
   connection_info_ = connection_info;
   was_redirected_ = true;
   redirect_chain_.push_back(new_url);
   if (new_method != "POST")
     resource_request_body_ = nullptr;
 
   state_ = WILL_REDIRECT_REQUEST;
   complete_callback_ = callback;
 
+  if (IsSelfReferentialURL()) {
+    state_ = CANCELING;
+    RunCompleteCallback(NavigationThrottle::CANCEL);
+    return;
+  }
+
   // Notify each throttle of the request.
   NavigationThrottle::ThrottleCheckResult result = CheckWillRedirectRequest();
 
   // If the navigation is not deferred, run the callback.
   if (result != NavigationThrottle::DEFER)
     RunCompleteCallback(result);
 }
 
 void NavigationHandleImpl::WillProcessResponse(
     RenderFrameHostImpl* render_frame_host,
@@ skipping 312 matching lines @@
   if (ancestor_throttle)
     throttles_.push_back(std::move(ancestor_throttle));
 
   if (throttles_to_register.size() > 0) {
     throttles_.insert(throttles_.begin(), throttles_to_register.begin(),
                       throttles_to_register.end());
     throttles_to_register.weak_clear();
   }
 }
 
+bool NavigationHandleImpl::IsSelfReferentialURL() {
+  // about: URLs should be exempted since they are reserved for other purposes
+  // and cannot be the source of infinite recursion. BUG=341858

[alexmos, 2017/01/06 02:27:29]

nit: For referencing bug numbers, it's more common to use "See
https://crbug.com/341858."

[davidsac (gone - try alexmos), 2017/01/19 18:26:23]

Done.

+  if (url_.SchemeIs("about"))
+    return false;
+
+  // Browser-triggered navigations should be exempted.
+  if (!is_renderer_initiated_)
+    return false;
+
+  // We allow one level of self-reference because some sites depend on that,
+  // but we don't allow more than one.
+  bool found_self_reference = false;
+  for (const FrameTreeNode* node = frame_tree_node_->parent(); node;
+       node = node->parent()) {
+    url::Replacements<char> replacements;

[alexmos, 2017/01/06 02:27:29]

I think this might become more readable if you declared a helper (e.g.,
EqualIgnoringFragmentIdentifier(url1, url2)) in the anonymous namespace, similar
to the one we had in Blink, and used it here.

[davidsac (gone - try alexmos), 2017/01/19 18:26:23]

Done.

+    replacements.ClearRef();
+    replacements.ClearUsername();

[alexmos, 2017/01/06 02:27:29]

Why do you clear out username and password as well?  Just clearing the # should
be sufficient.

[davidsac (gone - try alexmos), 2017/01/19 18:26:23]

Done.

+    replacements.ClearPassword();
+    if (node->current_url().ReplaceComponents(replacements) ==
+        url_.ReplaceComponents(replacements)) {
+      if (found_self_reference) {

[alexmos, 2017/01/06 02:27:29]

nit: { not necessary

[davidsac (gone - try alexmos), 2017/01/19 18:26:23]

Done.

+        return true;
+      }
+      found_self_reference = true;
+    }
+  }
+  return false;
+}
+
 }  // namespace content