Commit 76916d1b authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Refactoring: Move mime type calculations closer to the point of 1st use.

This CL moves the call to
  canonical_mime_type_ =
      network::CrossOriginReadBlocking::GetCanonicalMimeType(mime_type);
closer to the point where the value of the |canonical_mime_type_| field
is needed for the 1st time.

The move helps with the following:
- Makes unit tests more robust against shuffling of chucks inside
  ShouldBlockBasedOnHeaders (some shuffling will be needed when
  moving some checks into //services/network).
- Makes the code of ShouldBlockBasedOnHeaders closer to the promise to
  perform less expensive checks first (the GetCanonicalMimeType has
  medium cost - it has to compare the mime type against multiple
  hardcoded strings).

The move necessitates some small follow-up tweaks in unit tests.

Bug: 792546
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ib21add69443dd0748aafd57da89a3cc16ffcdec1
Reviewed-on: https://chromium-review.googlesource.com/957804Reviewed-by: default avatarNick Carter <nick@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542643}
parent 9f32bb51
......@@ -242,6 +242,10 @@ void CrossSiteDocumentResourceHandler::LogBlockedResponse(
MimeType canonical_mime_type,
int http_response_code,
int64_t content_length) {
DCHECK(resource_request_info);
DCHECK_NE(network::CrossOriginReadBlocking::MimeType::kInvalid,
canonical_mime_type);
LogCrossSiteDocumentAction(
needed_sniffing
? CrossSiteDocumentResourceHandler::Action::kBlockedAfterSniffing
......@@ -751,22 +755,6 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
return false;
}
// CORB should look directly at the Content-Type header if one has been
// received from the network. Ignoring |response->head.mime_type| helps avoid
// breaking legitimate websites (which might happen more often when blocking
// would be based on the mime type sniffed by MimeSniffingResourceHandler).
//
// TODO(nick): What if the mime type is omitted? Should that be treated the
// same as text/plain? https://crbug.com/795971
std::string mime_type;
if (response->head.headers)
response->head.headers->GetMimeType(&mime_type);
// Canonicalize the MIME type. Note that even if it doesn't claim to be a
// blockable type (i.e., HTML, XML, JSON, or plain text), it may still fail
// the checks during the SniffForFetchOnlyResource() phase.
canonical_mime_type_ =
network::CrossOriginReadBlocking::GetCanonicalMimeType(mime_type);
// Treat a missing initiator as an empty origin to be safe, though we don't
// expect this to happen. Unfortunately, this requires a copy.
url::Origin initiator;
......@@ -858,6 +846,22 @@ bool CrossSiteDocumentResourceHandler::ShouldBlockBasedOnHeaders(
response->head.headers->GetNormalizedHeader("content-range", &range_header);
bool has_range_header = !range_header.empty();
// CORB should look directly at the Content-Type header if one has been
// received from the network. Ignoring |response->head.mime_type| helps avoid
// breaking legitimate websites (which might happen more often when blocking
// would be based on the mime type sniffed by MimeSniffingResourceHandler).
//
// TODO(nick): What if the mime type is omitted? Should that be treated the
// same as text/plain? https://crbug.com/795971
std::string mime_type;
if (response->head.headers)
response->head.headers->GetMimeType(&mime_type);
// Canonicalize the MIME type. Note that even if it doesn't claim to be a
// blockable type (i.e., HTML, XML, JSON, or plain text), it may still fail
// the checks during the SniffForFetchOnlyResource() phase.
canonical_mime_type_ =
network::CrossOriginReadBlocking::GetCanonicalMimeType(mime_type);
// If this is a partial response, sniffing is not possible, so allow the
// response if it's not a protected mime type.
if (has_range_header && canonical_mime_type_ == MimeType::kOthers) {
......
......@@ -172,7 +172,7 @@ class CONTENT_EXPORT CrossSiteDocumentResourceHandler
// A canonicalization of the specified MIME type, to determine if blocking the
// response is needed, as well as which type of sniffing to perform.
network::CrossOriginReadBlocking::MimeType canonical_mime_type_ =
network::CrossOriginReadBlocking::MimeType::kOthers;
network::CrossOriginReadBlocking::MimeType::kInvalid;
// Indicates whether this request was made by a plugin and was not using CORS.
// Such requests are exempt from blocking, while other plugin requests must be
......
......@@ -206,7 +206,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -221,7 +221,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{")]}',\n[true, true, false, \"user@chromium.org\"]"}, // packets
......@@ -236,7 +236,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/json", // response_mime_type
MimeType::kJson, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{")]}'\n[true, true, false, \"user@chromium.org\"]"}, // packets
......@@ -266,7 +266,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kInclude, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kAllowInitiatorOrigin, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -281,7 +281,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kInclude, // cors_request
"application/rss+xml", // response_mime_type
MimeType::kXml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kAllowAny, // cors_response
{"<?xml version=\"1.0\" encoding=\"UTF-8\" ?>"}, // packets
......@@ -296,7 +296,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kInclude, // cors_request
"text/json", // response_mime_type
MimeType::kJson, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kAllowNull, // cors_response
{"{\"x\" : 3}"}, // packets
......@@ -311,7 +311,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -326,7 +326,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -341,7 +341,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -356,7 +356,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kInclude, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kAllowInitiatorOrigin, // cors_response
{"<html><head>this should sniff as HTML"}, // first_chunk
......@@ -371,7 +371,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kInclude, // cors_request
"application/javascript", // response_mime_type
MimeType::kOthers, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
true, // include_no_sniff_header
AccessControlAllowOriginHeader::kAllowAny, // cors_response
{")]}'\n[true, false]"}, // packets
......@@ -600,7 +600,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -615,7 +615,7 @@ const TestScenario kScenarios[] = {
"http://www.a.com/", // initiator_origin
OriginHeader::kOmit, // cors_request
"text/html", // response_mime_type
MimeType::kHtml, // canonical_mime_type
MimeType::kInvalid, // canonical_mime_type
false, // include_no_sniff_header
AccessControlAllowOriginHeader::kOmit, // cors_response
{"<html><head>this should sniff as HTML"}, // packets
......@@ -1423,8 +1423,11 @@ TEST_P(CrossSiteDocumentResourceHandlerTest, ResponseBlocking) {
case MimeType::kOthers:
bucket = "Others";
break;
default:
NOTREACHED();
case MimeType::kInvalid:
DCHECK_EQ(Verdict::kAllow, scenario.verdict);
DCHECK_EQ(-1, scenario.verdict_packet);
bucket = "No blocking = no bucket";
break;
}
int start_action = static_cast<int>(
CrossSiteDocumentResourceHandler::Action::kResponseStarted);
......
......@@ -30,7 +30,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CrossOriginReadBlocking {
kJson = 2,
kPlain = 3,
kOthers = 4,
kMax,
kInvalid = kMax,
};
// Three conclusions are possible from sniffing a byte sequence:
......
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