Commit b06677fe authored by Lucas Gadani's avatar Lucas Gadani Committed by Commit Bot

CSP: Fix DCHECK when processing a redirect.

CSP is not enforced while processing a redirect, so the DCHECK needs to
reflect that we are doing a redirect (not a response check) or that we
are not enforcing frame-ancestors in the browser process.

Bug: 1041711
Change-Id: Ib4872e0f8d96c944f14b5b6ed26b9b07008697dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2011744
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733605}
parent 490c944c
......@@ -239,8 +239,8 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
}
std::string header_value;
HeaderDisposition disposition =
ParseHeader(request->GetResponseHeaders(), &header_value);
HeaderDisposition disposition = ParseHeader(request->GetResponseHeaders(),
&header_value, is_response_check);
switch (disposition) {
case HeaderDisposition::CONFLICT:
......@@ -363,7 +363,8 @@ void AncestorThrottle::ConsoleError(HeaderDisposition disposition) {
AncestorThrottle::HeaderDisposition AncestorThrottle::ParseHeader(
const net::HttpResponseHeaders* headers,
std::string* header_value) {
std::string* header_value,
bool is_response_check) {
DCHECK(header_value);
if (!headers)
return HeaderDisposition::NONE;
......@@ -406,8 +407,9 @@ AncestorThrottle::HeaderDisposition AncestorThrottle::ParseHeader(
if (result != HeaderDisposition::NONE &&
result != HeaderDisposition::ALLOWALL &&
HeadersContainFrameAncestorsCSP(headers)) {
DCHECK(!base::FeatureList::IsEnabled(
network::features::kOutOfBlinkFrameAncestors));
DCHECK(!is_response_check ||
!base::FeatureList::IsEnabled(
network::features::kOutOfBlinkFrameAncestors));
// TODO(mkwst): 'frame-ancestors' is currently handled in Blink. We should
// handle it here instead. Until then, don't block the request, and let
// Blink handle it. https://crbug.com/555418
......
......@@ -61,7 +61,8 @@ class CONTENT_EXPORT AncestorThrottle : public NavigationThrottle {
// or INVALID, |header_value| will be populated with the value which caused
// the parse error.
HeaderDisposition ParseHeader(const net::HttpResponseHeaders* headers,
std::string* header_value);
std::string* header_value,
bool is_response_check);
DISALLOW_COPY_AND_ASSIGN(AncestorThrottle);
};
......
......@@ -89,7 +89,7 @@ TEST_F(AncestorThrottleTest, ParsingXFrameOptions) {
GetAncestorHeaders(test.header, nullptr);
std::string header_value;
EXPECT_EQ(test.expected,
throttle.ParseHeader(headers.get(), &header_value));
throttle.ParseHeader(headers.get(), &header_value, true));
EXPECT_EQ(test.value, header_value);
}
}
......@@ -124,7 +124,7 @@ TEST_F(AncestorThrottleTest, ErrorsParsingXFrameOptions) {
GetAncestorHeaders(test.header, nullptr);
std::string header_value;
EXPECT_EQ(test.expected,
throttle.ParseHeader(headers.get(), &header_value));
throttle.ParseHeader(headers.get(), &header_value, true));
EXPECT_EQ(test.failure, header_value);
}
}
......@@ -184,7 +184,7 @@ TEST_F(AncestorThrottleTest, IgnoreWhenFrameAncestorsPresent) {
GetAncestorHeaders("DENY", test.csp);
std::string header_value;
EXPECT_EQ(test.expected,
throttle.ParseHeader(headers.get(), &header_value));
throttle.ParseHeader(headers.get(), &header_value, true));
EXPECT_EQ("DENY", header_value);
}
}
......
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