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

Side by Side Diff: content/browser/frame_host/navigation_handle_impl.cc

Issue 2973433003: Block redirects to renderer-debug urls. (Closed)
Patch Set: Addressed comments. Add CanRequestURL again. Created 3 years, 5 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/browser/frame_host/navigation_handle_impl.h" 5 #include "content/browser/frame_host/navigation_handle_impl.h"
6 6
7 #include <iterator> 7 #include <iterator>
8 8
9 #include "base/debug/dump_without_crashing.h"
10 #include "base/logging.h" 9 #include "base/logging.h"
11 #include "base/metrics/histogram_macros.h" 10 #include "base/metrics/histogram_macros.h"
12 #include "content/browser/appcache/appcache_navigation_handle.h" 11 #include "content/browser/appcache/appcache_navigation_handle.h"
13 #include "content/browser/appcache/appcache_service_impl.h" 12 #include "content/browser/appcache/appcache_service_impl.h"
14 #include "content/browser/child_process_security_policy_impl.h" 13 #include "content/browser/child_process_security_policy_impl.h"
15 #include "content/browser/devtools/render_frame_devtools_agent_host.h" 14 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
16 #include "content/browser/frame_host/ancestor_throttle.h" 15 #include "content/browser/frame_host/ancestor_throttle.h"
17 #include "content/browser/frame_host/data_url_navigation_throttle.h" 16 #include "content/browser/frame_host/data_url_navigation_throttle.h"
18 #include "content/browser/frame_host/debug_urls.h" 17 #include "content/browser/frame_host/debug_urls.h"
19 #include "content/browser/frame_host/form_submission_throttle.h" 18 #include "content/browser/frame_host/form_submission_throttle.h"
(...skipping 97 matching lines...) Expand 10 before | Expand all | Expand 10 after
117 is_download_(false), 116 is_download_(false),
118 is_stream_(false), 117 is_stream_(false),
119 started_from_context_menu_(started_from_context_menu), 118 started_from_context_menu_(started_from_context_menu),
120 reload_type_(ReloadType::NONE), 119 reload_type_(ReloadType::NONE),
121 restore_type_(RestoreType::NONE), 120 restore_type_(RestoreType::NONE),
122 navigation_type_(NAVIGATION_TYPE_UNKNOWN), 121 navigation_type_(NAVIGATION_TYPE_UNKNOWN),
123 should_check_main_world_csp_(should_check_main_world_csp), 122 should_check_main_world_csp_(should_check_main_world_csp),
124 is_form_submission_(is_form_submission), 123 is_form_submission_(is_form_submission),
125 expected_render_process_host_id_(ChildProcessHost::kInvalidUniqueID), 124 expected_render_process_host_id_(ChildProcessHost::kInvalidUniqueID),
126 weak_factory_(this) { 125 weak_factory_(this) {
127 is_in_constructor = true;
128 TRACE_EVENT_ASYNC_BEGIN2("navigation", "NavigationHandle", this, 126 TRACE_EVENT_ASYNC_BEGIN2("navigation", "NavigationHandle", this,
129 "frame_tree_node", 127 "frame_tree_node",
130 frame_tree_node_->frame_tree_node_id(), "url", 128 frame_tree_node_->frame_tree_node_id(), "url",
131 url_.possibly_invalid_spec()); 129 url_.possibly_invalid_spec());
132 DCHECK(!navigation_start.is_null()); 130 DCHECK(!navigation_start.is_null());
133 131
134 site_url_ = SiteInstance::GetSiteForURL(frame_tree_node_->current_frame_host() 132 site_url_ = SiteInstance::GetSiteForURL(frame_tree_node_->current_frame_host()
135 ->GetSiteInstance() 133 ->GetSiteInstance()
136 ->GetBrowserContext(), 134 ->GetBrowserContext(),
137 url_); 135 url_);
(...skipping 33 matching lines...) Expand 10 before | Expand all | Expand 10 after
171 if (IsInMainFrame()) { 169 if (IsInMainFrame()) {
172 TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1( 170 TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
173 "navigation", "Navigation StartToCommit", this, 171 "navigation", "Navigation StartToCommit", this,
174 navigation_start, "Initial URL", url_.spec()); 172 navigation_start, "Initial URL", url_.spec());
175 } 173 }
176 174
177 if (is_same_document_) { 175 if (is_same_document_) {
178 TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationHandle", this, 176 TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationHandle", this,
179 "Same document"); 177 "Same document");
180 } 178 }
181 is_in_constructor = false;
182 } 179 }
183 180
184 NavigationHandleImpl::~NavigationHandleImpl() { 181 NavigationHandleImpl::~NavigationHandleImpl() {
185 // TODO(arthursonzogni): Remove this when we understand the root cause behind
186 // crbug.com/704892.
187 if (is_in_constructor)
188 base::debug::DumpWithoutCrashing();
189
190 // Inform the RenderProcessHost to no longer expect a navigation. 182 // Inform the RenderProcessHost to no longer expect a navigation.
191 if (expected_render_process_host_id_ != ChildProcessHost::kInvalidUniqueID) { 183 if (expected_render_process_host_id_ != ChildProcessHost::kInvalidUniqueID) {
192 RenderProcessHost* process = 184 RenderProcessHost* process =
193 RenderProcessHost::FromID(expected_render_process_host_id_); 185 RenderProcessHost::FromID(expected_render_process_host_id_);
194 if (process) { 186 if (process) {
195 RenderProcessHostImpl::RemoveExpectedNavigationToSite( 187 RenderProcessHostImpl::RemoveExpectedNavigationToSite(
196 frame_tree_node_->navigator()->GetController()->GetBrowserContext(), 188 frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
197 process, site_url_); 189 process, site_url_);
198 } 190 }
199 } 191 }
(...skipping 430 matching lines...) Expand 10 before | Expand all | Expand 10 after
630 const GURL& new_referrer_url, 622 const GURL& new_referrer_url,
631 bool new_is_external_protocol, 623 bool new_is_external_protocol,
632 scoped_refptr<net::HttpResponseHeaders> response_headers, 624 scoped_refptr<net::HttpResponseHeaders> response_headers,
633 net::HttpResponseInfo::ConnectionInfo connection_info, 625 net::HttpResponseInfo::ConnectionInfo connection_info,
634 RenderProcessHost* post_redirect_process, 626 RenderProcessHost* post_redirect_process,
635 const ThrottleChecksFinishedCallback& callback) { 627 const ThrottleChecksFinishedCallback& callback) {
636 TRACE_EVENT_ASYNC_STEP_INTO1("navigation", "NavigationHandle", this, 628 TRACE_EVENT_ASYNC_STEP_INTO1("navigation", "NavigationHandle", this,
637 "WillRedirectRequest", "url", 629 "WillRedirectRequest", "url",
638 new_url.possibly_invalid_spec()); 630 new_url.possibly_invalid_spec());
639 631
640 // TODO(arthursonzogni): Remove this when we understand the root cause behind 632 // |new_url| is not expected to be a "renderer debug" url. It should be
641 // crbug.com/704892 and crbug.com/736658 633 // blocked in NavigationRequest::OnRequestRedirected or in
642 if (IsRendererDebugURL(new_url)) 634 // ResourceLoader::OnReceivedRedirect. If it is not the case,
643 base::debug::DumpWithoutCrashing(); 635 // DidFinishNavigation will not be called. It could confuse some
636 // WebContentsObserver because DidStartNavigation was called.
637 // See https://crbug.com/728398
638 CHECK(!IsRendererDebugURL(new_url));
644 639
645 // Update the navigation parameters. 640 // Update the navigation parameters.
646 url_ = new_url; 641 url_ = new_url;
647 method_ = new_method; 642 method_ = new_method;
648 UpdateSiteURL(post_redirect_process); 643 UpdateSiteURL(post_redirect_process);
649 644
650 if (!(transition_ & ui::PAGE_TRANSITION_CLIENT_REDIRECT)) { 645 if (!(transition_ & ui::PAGE_TRANSITION_CLIENT_REDIRECT)) {
651 sanitized_referrer_.url = new_referrer_url; 646 sanitized_referrer_.url = new_referrer_url;
652 sanitized_referrer_ = 647 sanitized_referrer_ =
653 Referrer::SanitizeForRequest(url_, sanitized_referrer_); 648 Referrer::SanitizeForRequest(url_, sanitized_referrer_);
(...skipping 537 matching lines...) Expand 10 before | Expand all | Expand 10 after
1191 // Stop expecting a navigation to the current site URL in the current expected 1186 // Stop expecting a navigation to the current site URL in the current expected
1192 // process. 1187 // process.
1193 SetExpectedProcess(nullptr); 1188 SetExpectedProcess(nullptr);
1194 1189
1195 // Update the site URL and the expected process. 1190 // Update the site URL and the expected process.
1196 site_url_ = new_site_url; 1191 site_url_ = new_site_url;
1197 SetExpectedProcess(post_redirect_process); 1192 SetExpectedProcess(post_redirect_process);
1198 } 1193 }
1199 1194
1200 } // namespace content 1195 } // namespace content
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698