Drop navigations to NavigationEntry with invalid virtual URLs.

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 284 matching lines @@
   if (reload_type == ReloadType::ORIGINAL_REQUEST_URL &&
       entry.GetOriginalRequestURL().is_valid() && !entry.GetHasPostData()) {
     // We may have been redirected when navigating to the current URL.
     // Use the URL the user originally intended to visit, if it's valid and if a
     // POST wasn't involved; the latter case avoids issues with sending data to
     // the wrong page.
     dest_url = entry.GetOriginalRequestURL();
     dest_referrer = Referrer();
   }
 
+  // Don't attempt to navigate if the virtual URL is non-empty and invalid.
+  if (frame_tree_node->IsMainFrame()) {
+    GURL virtual_url = entry.GetVirtualURL();

[Charlie Reis, 2016/10/26 23:07:09]

const ref?

[nasko, 2016/10/26 23:37:47]

Done.

+    if (!virtual_url.is_valid() && !virtual_url.is_empty()) {
+      LOG(WARNING) << "Refusing to load for invalid virtual URL: "
+                   << virtual_url.possibly_invalid_spec();
+      return false;
+    }
+  }
+
   // Don't attempt to navigate to non-empty invalid URLs.
   if (!dest_url.is_valid() && !dest_url.is_empty()) {
     LOG(WARNING) << "Refusing to load invalid URL: "
                  << dest_url.possibly_invalid_spec();
     return false;
   }
 
   // The renderer will reject IPC messages with URLs longer than
   // this limit, so don't attempt to navigate with a longer URL.
   if (dest_url.spec().size() > url::kMaxURLChars) {
@@ skipping 908 matching lines @@
   // in deleting it, which leads to inconsistent state.
   bool has_transient_entry = !!controller_->GetTransientEntry();
 
   if (!has_browser_initiated_pending_entry && !has_transient_entry) {
     std::unique_ptr<NavigationEntryImpl> entry =
         NavigationEntryImpl::FromNavigationEntry(
             controller_->CreateNavigationEntry(
                 url, content::Referrer(), ui::PAGE_TRANSITION_LINK,
                 true /* is_renderer_initiated */, std::string(),
                 controller_->GetBrowserContext()));
+
+    // If the creation of NavigationEntry failed, just return early and
+    // don't set a pending entry.
+    if (!entry.get())

[Charlie Reis, 2016/10/26 23:07:09]

Is this stale?  I don't see any changes to CreateNavigationEntry that would
allow it to return null.

[nasko, 2016/10/26 23:37:47]

Oops, indeed. This is leftover from my previous try and I missed it.

+      return;
+
     entry->set_site_instance(site_instance);
     // TODO(creis): If there's a pending entry already, find a safe way to
     // update it instead of replacing it and copying over things like this.
     // That will allow us to skip the NavigationHandle update below as well.
     if (pending_entry) {
       entry->set_transferred_global_request_id(
           pending_entry->transferred_global_request_id());
       entry->set_should_replace_entry(pending_entry->should_replace_entry());
       entry->SetRedirectChain(pending_entry->GetRedirectChain());
     }
 
     // If there's a current NavigationHandle, update its pending NavEntry ID.
     // This is necessary for transfer navigations.  The handle may be null in
     // PlzNavigate.
     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