Skip to content

GitLab

T
tangled
Commit fee51c38 authored by Yutaka Hirano's avatar Yutaka Hirano Committed by Commit Bot
Browse files
Options

Remove obsolete logic for main resource from SecurityFilterPeer 

SecurityFilterPeer generates response body when it intercepts a
frame request. Thanks to recent refactoring efforts this code path is now
dead - SecurityFilterPeer cannot intercept navigation requests. Let's
remove the obsolete logic.

Bug: 894819
Change-Id: Ib089acbbd27af0a298b9ae471b55fb18bef564a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1546802Reviewed-by: default avatarDmitry Gozman <dgozman@chromium.org>
Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646664}
parent 712a788b
Hide whitespace changes
Inline Side-by-side
Showing with 11 additions and 57 deletions
chrome/renderer/chrome_render_thread_observer.cc
View file @ fee51c38
...@@ -87,7 +87,6 @@ class RendererResourceDelegate : public content::ResourceDispatcherDelegate { ...@@ -87,7 +87,6 @@ class RendererResourceDelegate : public content::ResourceDispatcherDelegate {
std::unique_ptr<content::RequestPeer> OnRequestComplete( std::unique_ptr<content::RequestPeer> OnRequestComplete(
std::unique_ptr<content::RequestPeer> current_peer, std::unique_ptr<content::RequestPeer> current_peer,
content::ResourceType resource_type,
int error_code) override { int error_code) override {
// Update the browser about our cache. // Update the browser about our cache.
// Rate limit informing the host of our cache stats. // Rate limit informing the host of our cache stats.
...@@ -105,7 +104,7 @@ class RendererResourceDelegate : public content::ResourceDispatcherDelegate { ...@@ -105,7 +104,7 @@ class RendererResourceDelegate : public content::ResourceDispatcherDelegate {
// Resource canceled with a specific error are filtered. // Resource canceled with a specific error are filtered.
return SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest( return SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest(
resource_type, std::move(current_peer), error_code); std::move(current_peer), error_code);
} }
std::unique_ptr<content::RequestPeer> OnReceivedResponse( std::unique_ptr<content::RequestPeer> OnReceivedResponse(
......
chrome/renderer/security_filter_peer.cc
View file @ fee51c38
...@@ -9,26 +9,18 @@ ...@@ -9,26 +9,18 @@
#include <utility> #include <utility>
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/strings/stringprintf.h"
#include "chrome/grit/generated_resources.h"
#include "content/public/renderer/fixed_received_data.h"
#include "net/base/escape.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "net/http/http_response_headers.h" #include "net/http/http_response_headers.h"
#include "services/network/public/cpp/url_loader_completion_status.h" #include "services/network/public/cpp/url_loader_completion_status.h"
#include "ui/base/l10n/l10n_util.h"
SecurityFilterPeer::SecurityFilterPeer( SecurityFilterPeer::SecurityFilterPeer(
std::unique_ptr<content::RequestPeer> peer, std::unique_ptr<content::RequestPeer> peer)
const std::string& mime_type, : original_peer_(std::move(peer)) {}
const std::string& data)
: original_peer_(std::move(peer)), mime_type_(mime_type), data_(data) {}
SecurityFilterPeer::~SecurityFilterPeer() {} SecurityFilterPeer::~SecurityFilterPeer() {}
// static // static
std::unique_ptr<content::RequestPeer> std::unique_ptr<content::RequestPeer>
SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest( SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest(
content::ResourceType resource_type,
std::unique_ptr<content::RequestPeer> peer, std::unique_ptr<content::RequestPeer> peer,
int os_error) { int os_error) {
// Create a filter for SSL and CERT errors. // Create a filter for SSL and CERT errors.
...@@ -46,24 +38,8 @@ SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest( ...@@ -46,24 +38,8 @@ SecurityFilterPeer::CreateSecurityFilterPeerForDeniedRequest(
case net::ERR_CERT_WEAK_KEY: case net::ERR_CERT_WEAK_KEY:
case net::ERR_CERT_NAME_CONSTRAINT_VIOLATION: case net::ERR_CERT_NAME_CONSTRAINT_VIOLATION:
case net::ERR_INSECURE_RESPONSE: case net::ERR_INSECURE_RESPONSE:
case net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN: { case net::ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN:
std::string mime_type; return base::WrapUnique(new SecurityFilterPeer(std::move(peer)));
std::string data;
if (content::IsResourceTypeFrame(resource_type)) {
// TODO(jcampan): use a different message when getting a
// phishing/malware error.
data = base::StringPrintf(
"<html><meta charset='UTF-8'>"
"<body style='background-color:#990000;color:white;'>"
"%s</body></html>",
net::EscapeForHTML(
l10n_util::GetStringUTF8(IDS_UNSAFE_FRAME_MESSAGE))
.c_str());
mime_type = "text/html";
}
return base::WrapUnique(
new SecurityFilterPeer(std::move(peer), mime_type, data));
}
default: default:
// For other errors, we use our normal error handling. // For other errors, we use our normal error handling.
return peer; return peer;
...@@ -102,14 +78,9 @@ void SecurityFilterPeer::OnTransferSizeUpdated(int transfer_size_diff) { ...@@ -102,14 +78,9 @@ void SecurityFilterPeer::OnTransferSizeUpdated(int transfer_size_diff) {
void SecurityFilterPeer::OnCompletedRequest( void SecurityFilterPeer::OnCompletedRequest(
const network::URLLoaderCompletionStatus& status) { const network::URLLoaderCompletionStatus& status) {
network::ResourceResponseInfo info; network::ResourceResponseInfo info;
info.mime_type = mime_type_; info.headers = CreateHeaders();
info.headers = CreateHeaders(mime_type_); info.content_length = 0;
info.content_length = static_cast<int>(data_.size());
original_peer_->OnReceivedResponse(info); original_peer_->OnReceivedResponse(info);
if (!data_.empty()) {
original_peer_->OnReceivedData(std::make_unique<content::FixedReceivedData>(
data_.data(), data_.size()));
}
network::URLLoaderCompletionStatus ok_status(status); network::URLLoaderCompletionStatus ok_status(status);
ok_status.error_code = net::OK; ok_status.error_code = net::OK;
original_peer_->OnCompletedRequest(ok_status); original_peer_->OnCompletedRequest(ok_status);
...@@ -119,8 +90,7 @@ scoped_refptr<base::TaskRunner> SecurityFilterPeer::GetTaskRunner() { ...@@ -119,8 +90,7 @@ scoped_refptr<base::TaskRunner> SecurityFilterPeer::GetTaskRunner() {
return original_peer_->GetTaskRunner(); return original_peer_->GetTaskRunner();
} }
scoped_refptr<net::HttpResponseHeaders> SecurityFilterPeer::CreateHeaders( scoped_refptr<net::HttpResponseHeaders> SecurityFilterPeer::CreateHeaders() {
const std::string& mime_type) {
std::string raw_headers; std::string raw_headers;
raw_headers.append("HTTP/1.1 200 OK"); raw_headers.append("HTTP/1.1 200 OK");
raw_headers.push_back('\0'); raw_headers.push_back('\0');
...@@ -130,11 +100,6 @@ scoped_refptr<net::HttpResponseHeaders> SecurityFilterPeer::CreateHeaders( ...@@ -130,11 +100,6 @@ scoped_refptr<net::HttpResponseHeaders> SecurityFilterPeer::CreateHeaders(
// resource). // resource).
raw_headers.append("cache-control: no-cache"); raw_headers.append("cache-control: no-cache");
raw_headers.push_back('\0'); raw_headers.push_back('\0');
if (!mime_type.empty()) {
raw_headers.append("content-type: ");
raw_headers.append(mime_type);
raw_headers.push_back('\0');
}
raw_headers.push_back('\0'); raw_headers.push_back('\0');
return base::MakeRefCounted<net::HttpResponseHeaders>(raw_headers); return base::MakeRefCounted<net::HttpResponseHeaders>(raw_headers);
} }
chrome/renderer/security_filter_peer.h
View file @ fee51c38
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include <string> #include <string>
#include "base/macros.h" #include "base/macros.h"
#include "content/public/common/resource_type.h"
#include "content/public/renderer/request_peer.h" #include "content/public/renderer/request_peer.h"
#include "services/network/public/cpp/resource_response_info.h" #include "services/network/public/cpp/resource_response_info.h"
...@@ -29,7 +28,6 @@ class SecurityFilterPeer final : public content::RequestPeer { ...@@ -29,7 +28,6 @@ class SecurityFilterPeer final : public content::RequestPeer {
static std::unique_ptr<content::RequestPeer> static std::unique_ptr<content::RequestPeer>
CreateSecurityFilterPeerForDeniedRequest( CreateSecurityFilterPeerForDeniedRequest(
content::ResourceType resource_type,
std::unique_ptr<content::RequestPeer> peer, std::unique_ptr<content::RequestPeer> peer,
int os_error); int os_error);
...@@ -47,16 +45,11 @@ class SecurityFilterPeer final : public content::RequestPeer { ...@@ -47,16 +45,11 @@ class SecurityFilterPeer final : public content::RequestPeer {
scoped_refptr<base::TaskRunner> GetTaskRunner() override; scoped_refptr<base::TaskRunner> GetTaskRunner() override;
private: private:
SecurityFilterPeer(std::unique_ptr<content::RequestPeer> peer, explicit SecurityFilterPeer(std::unique_ptr<content::RequestPeer> peer);
const std::string& mime_type,
const std::string& data);
static scoped_refptr<net::HttpResponseHeaders> CreateHeaders( static scoped_refptr<net::HttpResponseHeaders> CreateHeaders();
const std::string& mime_type);
std::unique_ptr<content::RequestPeer> original_peer_; std::unique_ptr<content::RequestPeer> original_peer_;
std::string mime_type_;
std::string data_;
DISALLOW_COPY_AND_ASSIGN(SecurityFilterPeer); DISALLOW_COPY_AND_ASSIGN(SecurityFilterPeer);
}; };
......
content/public/renderer/resource_dispatcher_delegate.h
View file @ fee51c38
...@@ -26,7 +26,6 @@ class CONTENT_EXPORT ResourceDispatcherDelegate { ...@@ -26,7 +26,6 @@ class CONTENT_EXPORT ResourceDispatcherDelegate {
virtual std::unique_ptr<RequestPeer> OnRequestComplete( virtual std::unique_ptr<RequestPeer> OnRequestComplete(
std::unique_ptr<RequestPeer> current_peer, std::unique_ptr<RequestPeer> current_peer,
ResourceType resource_type,
int error_code) = 0; int error_code) = 0;
// Note that |url|, |referrer| and |method| are the final values (e.g. after // Note that |url|, |referrer| and |method| are the final values (e.g. after
......
content/renderer/loader/resource_dispatcher.cc
View file @ fee51c38
...@@ -284,8 +284,7 @@ void ResourceDispatcher::OnRequestComplete( ...@@ -284,8 +284,7 @@ void ResourceDispatcher::OnRequestComplete(
if (delegate_) { if (delegate_) {
std::unique_ptr<RequestPeer> new_peer = delegate_->OnRequestComplete( std::unique_ptr<RequestPeer> new_peer = delegate_->OnRequestComplete(
std::move(request_info->peer), request_info->resource_type, std::move(request_info->peer), status.error_code);
status.error_code);
DCHECK(new_peer); DCHECK(new_peer);
request_info->peer = std::move(new_peer); request_info->peer = std::move(new_peer);
} }
......
content/renderer/loader/resource_dispatcher_unittest.cc
View file @ fee51c38
...@@ -182,7 +182,6 @@ class TestResourceDispatcherDelegate : public ResourceDispatcherDelegate { ...@@ -182,7 +182,6 @@ class TestResourceDispatcherDelegate : public ResourceDispatcherDelegate {
std::unique_ptr<RequestPeer> OnRequestComplete( std::unique_ptr<RequestPeer> OnRequestComplete(
std::unique_ptr<RequestPeer> current_peer, std::unique_ptr<RequestPeer> current_peer,
ResourceType resource_type,
int error_code) override { int error_code) override {
return current_peer; return current_peer;
} }
......
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