Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1378)

Unified Diff: content/browser/frame_host/navigation_controller_impl.cc

Issue 317703004: Simplify AreURLsInPageNavigation (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 6d1c597d7604629df1b636ed8947c099fbc40c5c..a751ccd3da09dd5b6223138a65e5fe20776e9635 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -104,24 +104,30 @@ void ConfigureEntriesForRestore(
}
}
-// See NavigationController::IsURLInPageNavigation for how this works and why.
+// There are two general cases where a navigation is in page:
+// 1. A fragment navigation, in which the url is kept the same except for the
+// reference fragment.
+// 2. A history API navigation (pushState and replaceState). This case is
+// always in-page, but the urls are not guaranteed to match excluding the
+// fragment. The relevant spec allows pushState/replaceState to any URL on
+// the same origin.
+// However, due to reloads, even identical urls are *not* guaranteed to be
+// in-page navigations, we have to trust the renderer almost entirely.
+// The one thing we do know is that cross-origin navigations will *never* be
+// in-page. Therefore, trust the renderer if the URLs are on the same origin,
+// and assume the renderer is malicious if a cross-origin navigation claims to
+// be in-page.
bool AreURLsInPageNavigation(const GURL& existing_url,
const GURL& new_url,
bool renderer_says_in_page,
- NavigationType navigation_type) {
- if (existing_url.GetOrigin() == new_url.GetOrigin())
- return renderer_says_in_page;
-
- if (!new_url.has_ref()) {
- // When going back from the ref URL to the non ref one the navigation type
- // is IN_PAGE.
- return navigation_type == NAVIGATION_TYPE_IN_PAGE;
- }
-
- url::Replacements<char> replacements;
- replacements.ClearRef();
- return existing_url.ReplaceComponents(replacements) ==
- new_url.ReplaceComponents(replacements);
+ RenderFrameHost* rfh) {
+ WebPreferences prefs = rfh->GetRenderViewHost()->GetWebkitPreferences();
+ bool is_same_origin = existing_url.is_empty() ||
+ existing_url.GetOrigin() == new_url.GetOrigin() ||
+ !prefs.web_security_enabled;
+ if (!is_same_origin && renderer_says_in_page)
+ rfh->GetProcess()->ReceivedBadMessage();
+ return is_same_origin && renderer_says_in_page;
}
// Determines whether or not we should be carrying over a user agent override
@@ -766,8 +772,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
details->type = ClassifyNavigation(rfh, params);
// is_in_page must be computed before the entry gets committed.
- details->is_in_page = IsURLInPageNavigation(
- params.url, params.was_within_same_page, details->type);
+ details->is_in_page = AreURLsInPageNavigation(rfh->GetLastCommittedURL(),
+ params.url, params.was_within_same_page, rfh);
switch (details->type) {
case NAVIGATION_TYPE_NEW_PAGE:
@@ -986,8 +992,7 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
// navigations that don't actually navigate, but it can happen when there is
// an encoding override (it always sends a navigation request).
if (AreURLsInPageNavigation(existing_entry->GetURL(), params.url,
- params.was_within_same_page,
- NAVIGATION_TYPE_UNKNOWN)) {
+ params.was_within_same_page, rfh)) {
return NAVIGATION_TYPE_IN_PAGE;
}
@@ -1253,10 +1258,10 @@ int NavigationControllerImpl::GetIndexOfEntry(
bool NavigationControllerImpl::IsURLInPageNavigation(
const GURL& url,
bool renderer_says_in_page,
- NavigationType navigation_type) const {
+ RenderFrameHost* rfh) const {
NavigationEntry* last_committed = GetLastCommittedEntry();
return last_committed && AreURLsInPageNavigation(
- last_committed->GetURL(), url, renderer_says_in_page, navigation_type);
+ last_committed->GetURL(), url, renderer_says_in_page, rfh);
}
void NavigationControllerImpl::CopyStateFrom(

Powered by Google App Engine
This is Rietveld 408576698