Skip to content

Commit

Permalink
Revert "Make mime type sniffing in DevToolsURLLoaderInterceptor consi…
Browse files Browse the repository at this point in the history
…stent with real URLLoader"

This reverts commit 918d412.

Reason for revert:
http/tests/inspector-protocol/fetch/mime-type-sniffing.js consistently failing on linux-trusty-rel. Regression began when this change landed: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-trusty-rel/24163/blamelist

Original change's description:
> Make mime type sniffing in DevToolsURLLoaderInterceptor consistent with real URLLoader
>
> - extract ShouldSniffContent() from url_loader.cc to header_util.{h,cc}
> - reuse it in DevToolsURLLoaderInterceptor
>
> Bug: b/187745024
> Change-Id: I0cd6904e7912c372aa744b0abb038d867c9195d2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2905075
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#884328}

Bug: b/187745024,1210841
Change-Id: I73260f7b6fe85d323eba0946c5386becab4c898c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2910515
Owners-Override: Katie Dektar <katie@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#885087}
  • Loading branch information
Katie Dektar authored and Chromium LUCI CQ committed May 20, 2021
1 parent 19ec52c commit fdee838
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 120 deletions.
28 changes: 10 additions & 18 deletions content/browser/devtools/devtools_url_loader_interceptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "net/url_request/redirect_util.h"
#include "net/url_request/referrer_policy.h"
#include "services/network/public/cpp/cors/cors.h"
#include "services/network/public/cpp/header_util.h"
#include "services/network/public/cpp/resource_request_body.h"
#include "services/network/public/mojom/network_context.mojom.h"
#include "third_party/blink/public/platform/resource_request_blocked_reason.h"
Expand Down Expand Up @@ -1038,22 +1037,15 @@ Response InterceptionJob::ProcessResponseOverride(
base::MakeRefCounted<net::HttpResponseHeaders>(kDummyHeaders);
}
head->headers->GetMimeTypeAndCharset(&head->mime_type, &head->charset);
const GURL& url = create_loader_params_->request.url;
if (create_loader_params_->options &
network::mojom::kURLLoadOptionSniffMimeType) {
if (body_size && network::ShouldSniffContent(url, *head)) {
size_t bytes_to_sniff =
std::min(body_size, static_cast<size_t>(net::kMaxBytesToSniff));
const std::string hint = head->mime_type;
net::SniffMimeType(
base::StringPiece(body->front_as<const char>() + response_body_offset,
bytes_to_sniff),
url, hint, net::ForceSniffFileUrlsForHtml::kDisabled,
&head->mime_type);
head->did_mime_sniff = true;
} else if (head->mime_type.empty()) {
head->mime_type.assign("text/plain");
}
if (head->mime_type.empty() && body_size) {
size_t bytes_to_sniff =
std::min(body_size, static_cast<size_t>(net::kMaxBytesToSniff));
net::SniffMimeType(
base::StringPiece(body->front_as<const char>() + response_body_offset,
bytes_to_sniff),
create_loader_params_->request.url, "",
net::ForceSniffFileUrlsForHtml::kDisabled, &head->mime_type);
head->did_mime_sniff = true;
}
// TODO(caseq): we're cheating here a bit, raw_headers() have \0's
// where real headers would have \r\n, but the sizes here
Expand All @@ -1075,7 +1067,7 @@ Response InterceptionJob::ProcessResponseOverride(
base::OnceClosure continue_after_cookies_set;
std::string location;
if (head->headers->IsRedirect(&location)) {
GURL redirect_url = url.Resolve(location);
GURL redirect_url = create_loader_params_->request.url.Resolve(location);
if (redirect_url.is_valid()) {
continue_after_cookies_set =
base::BindOnce(&InterceptionJob::ProcessRedirectByClient,
Expand Down
1 change: 1 addition & 0 deletions content/browser/devtools/protocol/network_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/containers/queue.h"
#include "base/i18n/i18n_constants.h"
#include "base/i18n/icu_string_conversions.h"
#include "base/json/json_reader.h"
#include "base/process/process_handle.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h"
Expand Down
18 changes: 0 additions & 18 deletions services/network/public/cpp/header_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@
#include "services/network/public/cpp/header_util.h"

#include "base/strings/string_util.h"
#include "net/base/mime_sniffer.h"
#include "net/http/http_request_headers.h"
#include "services/network/public/mojom/url_response_head.mojom.h"
#include "url/gurl.h"

namespace network {

Expand Down Expand Up @@ -96,19 +93,4 @@ bool AreRequestHeadersSafe(const net::HttpRequestHeaders& request_headers) {
return true;
}

bool ShouldSniffContent(const GURL& url,
const mojom::URLResponseHead& response) {
std::string content_type_options;
if (response.headers) {
response.headers->GetNormalizedHeader("x-content-type-options",
&content_type_options);
}
bool sniffing_blocked =
base::LowerCaseEqualsASCII(content_type_options, "nosniff");
bool we_would_like_to_sniff =
net::ShouldSniffMimeType(url, response.mime_type);

return !sniffing_blocked && we_would_like_to_sniff;
}

} // namespace network
10 changes: 0 additions & 10 deletions services/network/public/cpp/header_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,11 @@
#include "base/component_export.h"
#include "base/strings/string_piece.h"

class GURL;
namespace net {
class HttpRequestHeaders;
} // namespace net

namespace network {
namespace mojom {
class URLResponseHead;
} // namespace mojom

// Checks if a single request header is safe to send.
COMPONENT_EXPORT(NETWORK_CPP)
Expand All @@ -29,12 +25,6 @@ bool IsRequestHeaderSafe(const base::StringPiece& key,
COMPONENT_EXPORT(NETWORK_CPP)
bool AreRequestHeadersSafe(const net::HttpRequestHeaders& request_headers);

// Checks whether mime type sniffing should be enabled, considering response
// headers, current mime type and URL scheme.
COMPONENT_EXPORT(NETWORK_CPP)
bool ShouldSniffContent(const GURL& url,
const mojom::URLResponseHead& response);

} // namespace network

#endif // SERVICES_NETWORK_PUBLIC_CPP_HEADER_UTIL_H_
28 changes: 24 additions & 4 deletions services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,29 @@ mojom::HttpRawRequestResponseInfoPtr BuildRawRequestResponseInfo(
return info;
}

bool ShouldSniffContent(net::URLRequest* url_request,
const mojom::URLResponseHead& response) {
const std::string& mime_type = response.mime_type;

std::string content_type_options;
url_request->GetResponseHeaderByName("x-content-type-options",
&content_type_options);

bool sniffing_blocked =
base::LowerCaseEqualsASCII(content_type_options, "nosniff");
bool we_would_like_to_sniff =
net::ShouldSniffMimeType(url_request->url(), mime_type);

if (!sniffing_blocked && we_would_like_to_sniff) {
// We're going to look at the data before deciding what the content type
// is. That means we need to delay sending the response started IPC.
VLOG(1) << "To buffer: " << url_request->url().spec();
return true;
}

return false;
}

// Concerning headers that consumers probably shouldn't be allowed to set.
// Gathering numbers on these before adding them to kUnsafeHeaders.
const struct {
Expand Down Expand Up @@ -1429,10 +1452,7 @@ void URLLoader::ContinueOnResponseStarted() {
}
}
if ((options_ & mojom::kURLLoadOptionSniffMimeType)) {
if (ShouldSniffContent(url_request_->url(), *response_)) {
// We're going to look at the data before deciding what the content type
// is. That means we need to delay sending the response started IPC.
VLOG(1) << "Will sniff content for mime type: " << url_request_->url();
if (ShouldSniffContent(url_request_.get(), *response_)) {
is_more_mime_sniffing_needed_ = true;
} else if (response_->mime_type.empty()) {
// Ugg. The server told us not to sniff the content but didn't give us
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit fdee838

Please sign in to comment.