Commit 531e3d90 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

Prevent overwriting SSLStatus on non-network navigations.

This CL removes a now-erroneous overwrite of a valid SSLStatus when
a user performs a same-document navigation that does not result in a
network request.

Bug: 877618
Change-Id: I495b9e43fe11d2dc6650e7f2a4d776d5cf804c5d
Reviewed-on: https://chromium-review.googlesource.com/c/1311126
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: default avatarChristopher Thompson <cthomp@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605613}
parent 11021fbd
...@@ -5886,6 +5886,32 @@ IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentHasSSLState) { ...@@ -5886,6 +5886,32 @@ IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentHasSSLState) {
CheckAuthenticatedState(tab, AuthState::NONE); CheckAuthenticatedState(tab, AuthState::NONE);
} }
// Simulate the user revisiting a page without triggering a reload (e.g., when
// clicking a bookmark with an anchor hash twice). As this is a same document
// navigation, the SSL state should be left intact despite not triggering a
// network request. Regression test for https://crbug.com/877618.
IN_PROC_BROWSER_TEST_P(SSLUITest, SameDocumentHasSSLStateNoLoad) {
ASSERT_TRUE(https_server_.Start());
WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
GURL start_url(https_server_.GetURL("/ssl/google.html#foo"));
ui_test_utils::NavigateToURL(browser(), start_url);
// Simulate clicking on a bookmark.
{
content::WindowedNotificationObserver observer(
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
NavigateParams navigate_params(browser(), start_url,
ui::PAGE_TRANSITION_AUTO_BOOKMARK);
Navigate(&navigate_params);
observer.Wait();
}
CheckAuthenticatedState(tab, AuthState::NONE);
}
// Checks that if a client redirect occurs while the page is loading, the SSL // Checks that if a client redirect occurs while the page is loading, the SSL
// state reflects the final URL. // state reflects the final URL.
IN_PROC_BROWSER_TEST_P(SSLUITest, ClientRedirectSSLState) { IN_PROC_BROWSER_TEST_P(SSLUITest, ClientRedirectSSLState) {
......
...@@ -965,7 +965,8 @@ bool NavigationControllerImpl::RendererDidNavigate( ...@@ -965,7 +965,8 @@ bool NavigationControllerImpl::RendererDidNavigate(
was_restored, navigation_handle); was_restored, navigation_handle);
break; break;
case NAVIGATION_TYPE_SAME_PAGE: case NAVIGATION_TYPE_SAME_PAGE:
RendererDidNavigateToSamePage(rfh, params, navigation_handle); RendererDidNavigateToSamePage(rfh, params, details->is_same_document,
navigation_handle);
break; break;
case NAVIGATION_TYPE_NEW_SUBFRAME: case NAVIGATION_TYPE_NEW_SUBFRAME:
RendererDidNavigateNewSubframe(rfh, params, details->is_same_document, RendererDidNavigateNewSubframe(rfh, params, details->is_same_document,
...@@ -1533,6 +1534,7 @@ void NavigationControllerImpl::RendererDidNavigateToExistingPage( ...@@ -1533,6 +1534,7 @@ void NavigationControllerImpl::RendererDidNavigateToExistingPage(
void NavigationControllerImpl::RendererDidNavigateToSamePage( void NavigationControllerImpl::RendererDidNavigateToSamePage(
RenderFrameHostImpl* rfh, RenderFrameHostImpl* rfh,
const FrameHostMsg_DidCommitProvisionalLoad_Params& params, const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
bool is_same_document,
NavigationHandleImpl* handle) { NavigationHandleImpl* handle) {
// This classification says that we have a pending entry that's the same as // This classification says that we have a pending entry that's the same as
// the last committed entry. This entry is guaranteed to exist by // the last committed entry. This entry is guaranteed to exist by
...@@ -1556,9 +1558,13 @@ void NavigationControllerImpl::RendererDidNavigateToSamePage( ...@@ -1556,9 +1558,13 @@ void NavigationControllerImpl::RendererDidNavigateToSamePage(
existing_entry->SetURL(params.url); existing_entry->SetURL(params.url);
// If a user presses enter in the omnibox and the server redirects, the URL // If a user presses enter in the omnibox and the server redirects, the URL
// might change (but it's still considered a SAME_PAGE navigation). So we must // might change (but it's still considered a SAME_PAGE navigation), so we must
// update the SSL status. // update the SSL status if we perform a network request (e.g. a
existing_entry->GetSSL() = SSLStatus(handle->GetSSLInfo()); // non-same-document navigation). Requests that don't result in a network
// request do not have a valid SSL status, but since the document didn't
// change, the previous SSLStatus is still valid.
if (!is_same_document)
existing_entry->GetSSL() = SSLStatus(handle->GetSSLInfo());
if (existing_entry->GetURL().SchemeIs(url::kHttpsScheme) && if (existing_entry->GetURL().SchemeIs(url::kHttpsScheme) &&
!rfh->GetParent() && handle->GetNetErrorCode() == net::OK) { !rfh->GetParent() && handle->GetNetErrorCode() == net::OK) {
......
...@@ -375,6 +375,7 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController { ...@@ -375,6 +375,7 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
void RendererDidNavigateToSamePage( void RendererDidNavigateToSamePage(
RenderFrameHostImpl* rfh, RenderFrameHostImpl* rfh,
const FrameHostMsg_DidCommitProvisionalLoad_Params& params, const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
bool is_same_document,
NavigationHandleImpl* handle); NavigationHandleImpl* handle);
void RendererDidNavigateNewSubframe( void RendererDidNavigateNewSubframe(
RenderFrameHostImpl* rfh, RenderFrameHostImpl* rfh,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment