Skip to content

GitLab

T
tangled
Commit cae1f823 authored by Takashi Toyoshima's avatar Takashi Toyoshima Committed by Commit Bot
Browse files
Options

OOR-CORS: Remove BlinkCORS related tests from ExtensionWebRequestApiTest 

This patch modifies tests that see kOutOfBlinkCors feature flag, so
to make them run only with OOR-CORS enabled.

BlinkCors has already been deprecated and this is part of the work
to remove the kOutOfBlinkCors feature work.

Bug: 1053866
Change-Id: I59d3b9c42e61641053cc74aacc99493261ad51fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368642
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800959}
parent 61543f93
Show whitespace changes
Inline Side-by-side
Showing with 68 additions and 182 deletions
chrome/browser/extensions/api/web_request/web_request_apitest.cc
View file @ cae1f823
......@@ -60,7 +60,6 @@
#include "chromeos/login/login_state/scoped_test_public_session_login_state.h"
#include "components/embedder_support/switches.h"
#include "components/google/core/common/google_switches.h"
#include "components/policy/core/browser/browser_policy_connector.h"
#include "components/policy/core/common/mock_configuration_policy_provider.h"
#include "components/policy/policy_constants.h"
#include "components/prefs/pref_service.h"
......@@ -628,65 +627,12 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
<< message_;
}
enum class CorsMode {
kOutOfBlinkCors,
kBlinkCors,
};
class ExtensionWebRequestApiPolicyTest
: public ExtensionWebRequestApiTest,
public ::testing::WithParamInterface<CorsMode> {
public:
const std::string& test_name() { return test_name_; }
private:
void SetUpInProcessBrowserTestFixture() override {
EXPECT_CALL(provider_, IsInitializationComplete(testing::_))
.WillRepeatedly(testing::Return(true));
policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_);
switch (GetParam()) {
case CorsMode::kOutOfBlinkCors:
feature_list_.InitAndEnableFeature(network::features::kOutOfBlinkCors);
test_name_ += "?cors_mode=network_service";
break;
case CorsMode::kBlinkCors:
feature_list_.InitAndDisableFeature(network::features::kOutOfBlinkCors);
test_name_ += "?cors_mode=blink";
break;
}
ExtensionWebRequestApiTest::SetUpInProcessBrowserTestFixture();
}
void UpdatePolicy(const std::string& policy, base::Value value) {
policy::PolicyMap policy_map;
policy_map.Set(policy, policy::POLICY_LEVEL_MANDATORY,
policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD,
std::move(value), nullptr);
provider_.UpdateChromePolicy(policy_map);
}
private:
base::test::ScopedFeatureList feature_list_;
policy::MockConfigurationPolicyProvider provider_;
std::string test_name_ = "test_cors.html";
};
IN_PROC_BROWSER_TEST_P(ExtensionWebRequestApiPolicyTest,
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
WebRequestCORSWithExtraHeaders) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtest("webrequest", test_name())) << message_;
ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_cors.html")) << message_;
}
INSTANTIATE_TEST_SUITE_P(Enabled,
ExtensionWebRequestApiPolicyTest,
testing::Values(CorsMode::kOutOfBlinkCors));
INSTANTIATE_TEST_SUITE_P(Disabled,
ExtensionWebRequestApiPolicyTest,
testing::Values(CorsMode::kBlinkCors));
IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestRedirects) {
ASSERT_TRUE(StartEmbeddedTestServer());
ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_redirects.html"))
......
chrome/test/data/extensions/api_test/webrequest/test_cors.js
View file @ cae1f823
......@@ -7,18 +7,8 @@ const listeningUrlPattern = '*://cors.example.com/*';
const params = (new URL(location.href)).searchParams;
const BASE = 'extensions/api_test/webrequest/cors/';
function getCorsMode() {
const name = 'cors_mode';
chrome.test.assertTrue(params.has(name));
const mode = params.get(name);
chrome.test.assertTrue(mode == 'blink' || mode == 'network_service');
return mode;
}
function setExpectationsForNonObservablePreflight() {
// In this case the preflight request is not observable.
chrome.test.assertTrue(getCorsMode() == 'network_service');
const url = getServerURL(BASE + 'accept', 'cors.example.com');
const method = 'GET';
const initiator = getServerURL('').slice(0, -1);
......@@ -146,15 +136,11 @@ function setExpectationsForObservablePreflight(extraInfoSpec) {
// should arrive, but we are not sure which comes first - that is essentially
// racy, so we cannot have an expecation here.
let events;
let eventsOrder;
if (getCorsMode() == 'network_service') {
// When the CORS module is in the network process, onBeforeRequest is called
// for the actual request first, and then the preflight request is made.
// As there is no 'access-control-allow-headers' header in the preflight
// response, the actual request fails whereas the preflight request
// succeeds.
events = [
// First, onBeforeRequest is called for the actual request, and then the
// preflight request is made. As there is no 'access-control-allow-headers'
// header in the preflight response, the actual request fails whereas the
// preflight request succeeds.
let events = [
{ label: 'onBeforeRequest',
event: 'onBeforeRequest',
details: {
......@@ -166,18 +152,11 @@ function setExpectationsForObservablePreflight(extraInfoSpec) {
},
},
].concat(eventsForPreflight);
eventOrder = ['onBeforeRequest'].concat(eventOrderForPreflight);
let eventOrder = ['onBeforeRequest'].concat(eventOrderForPreflight);
// We should see the cancellation of the actual request, but we cannot
// have that expecation here because we don't have an expecation on
// the completion of the preflight request. See above.
} else {
// In this case, the preflight request is made first, and blink will not
// make the actual request because of the lack of an
// 'access-control-allow-headers' header in the preflight response.
events = eventsForPreflight;
eventOrder = eventOrderForPreflight;
}
expect(
events,
......@@ -217,15 +196,14 @@ function registerRequestHeaderInjectionListeners(extraInfoSpec) {
beforeSendHeadersListener, {urls: [listeningUrlPattern]}, extraInfoSpec);
// If the 'x-foo' header is injected by |beforeSendHeadersListener| without
// 'extraHeaders' and with OOR-CORS being enabled, it triggers CORS
// preflight, and the response for the preflight OPTIONS request is expected
// to have the 'Access-Control-Allow-Headers: x-foo' header to pass the
// security checks. Since the mock-http-headers for the target URL does not
// provide the required header, the request fails in the CORS preflight.
// Otherwises, modified headers are not observed by CORS implementations, and
// do not trigger the CORS preflight.
const triggerPreflight = !extraInfoSpec.includes('extraHeaders') &&
getCorsMode() == 'network_service';
// 'extraHeaders', it triggers CORS preflight, and the response for the
// preflight OPTIONS request is expected to have the
// 'Access-Control-Allow-Headers: x-foo' header to pass the security checks.
// Since the mock-http-headers for the target URL does not provide the
// required header, the request fails in the CORS preflight. Otherwises,
// modified headers are not observed by CORS implementations, and do not
// trigger the CORS preflight.
const triggerPreflight = !extraInfoSpec.includes('extraHeaders');
const event = triggerPreflight ? chrome.webRequest.onErrorOccurred :
chrome.webRequest.onCompleted;
......@@ -248,11 +226,10 @@ function registerResponseHeaderInjectionListeners(extraInfoSpec) {
chrome.webRequest.onHeadersReceived.addListener(
headersReceivedListener, {urls: [listeningUrlPattern]}, extraInfoSpec);
// If the 'extraHeaders' is not specified and OOR-CORS is enabled, Chrome
// detects CORS failures before |headerReceivedListener| is called and injects
// fake headers to deceive the CORS checks.
const canInjectFakeCorsResponse = extraInfoSpec.includes('extraHeaders') ||
getCorsMode() == 'blink';
// If the 'extraHeaders' is not specified, Chrome detects CORS failures
// before |headerReceivedListener| is called and injects fake headers to
// deceive the CORS checks.
const canInjectFakeCorsResponse = extraInfoSpec.includes('extraHeaders');
const event = canInjectFakeCorsResponse ? chrome.webRequest.onCompleted :
chrome.webRequest.onErrorOccurred;
......@@ -420,9 +397,7 @@ function setExpectationsForSuccessfulPreflight() {
},
},
];
let eventOrder;
if (getCorsMode() == 'network_service') {
eventOrder = [
let eventOrder = [
'onBeforeRequest',
'onBeforeRequest-P',
'onBeforeSendHeaders-P',
......@@ -436,22 +411,6 @@ function setExpectationsForSuccessfulPreflight() {
'onResponseStarted',
'onCompleted',
];
} else {
eventOrder = [
'onBeforeRequest-P',
'onBeforeSendHeaders-P',
'onSendHeaders-P',
'onHeadersReceived-P',
'onResponseStarted-P',
'onCompleted-P',
'onBeforeRequest',
'onBeforeSendHeaders',
'onSendHeaders',
'onHeadersReceived',
'onResponseStarted',
'onCompleted',
];
}
expect(
events,
[eventOrder],
......@@ -485,10 +444,7 @@ function registerPreflightBlockingListener() {
hasSeenPreflightError = true;
}
// We see an error event for the actual request only when OOR-CORS
// is enabled; otherwise the CORS module in blink doesn't make a network
// request for the actual request.
if (details.method === 'GET' || getCorsMode() == 'blink') {
if (details.method === 'GET') {
chrome.webRequest.onErrorOccurred.removeListener(onErrorOccurred);
chrome.test.assertTrue(hasSeenPreflightError);
done();
......@@ -512,9 +468,7 @@ function registerPreflightRedirectingListener() {
}
}, {urls: [url]}, ['blocking', 'extraHeaders']);
if (getCorsMode() == 'network_service') {
// When CORS is implemented in the network service, we see failures on both
// the preflight and the actual request.
// We see failures on both the preflight and the actual request.
const done = callbackPass(() => {});
let hasSeenPreflightError = false;
chrome.webRequest.onErrorOccurred.addListener(
......@@ -529,12 +483,6 @@ function registerPreflightRedirectingListener() {
done();
}
}, {urls: [url]});
} else {
// In this case we see no completion events nor error events - The renderer
// cancels the preflight request in the redirect handling logic, and
// WebRequestProxyingURLLoaderFactory suppresses events in such a case.
// See https://crbug.com/1014816.
}
}
function registerOnBeforeRequestAndOnErrorOcurredListeners() {
......@@ -542,7 +490,7 @@ function registerOnBeforeRequestAndOnErrorOcurredListeners() {
const onBeforeRequestCalledForPreflight = callbackPass(() => {});
// onBeforeRequest doesn't have "extraHeaders", but it sees a preflight
// even when OOR-CORS is enabled, because onErrorOccurred has "extraHeaders".
// because onErrorOccurred has "extraHeaders".
chrome.webRequest.onBeforeRequest.addListener((details) => {
if (details.method === 'OPTIONS') {
onBeforeRequestCalledForPreflight();
......@@ -557,13 +505,9 @@ function registerOnBeforeRequestAndOnErrorOcurredListeners() {
runTests([
function testOriginHeader() {
// Register two sets of listener. One with extraHeaders and the second one
// without it.
// If OOR-CORS is enabled, the Origin header is invisible if the
// extraHeaders is not specified.
if (getCorsMode() == 'network_service')
// without it. The Origin header is invisible if the extraHeaders is not
// specified.
registerOriginListeners([], ['origin'], ['requestHeaders']);
else
registerOriginListeners(['origin'], [], ['requestHeaders']);
registerOriginListeners(['origin'], [], ['requestHeaders', 'extraHeaders']);
// Wait for the navigation to complete.
......@@ -602,11 +546,7 @@ runTests([
'extensions/api_test/webrequest/cors/fetch.html?path=reject'));
},
function testCorsPreflightWithoutExtraHeaders() {
if (getCorsMode() == 'network_service') {
setExpectationsForNonObservablePreflight();
} else {
setExpectationsForObservablePreflight([]);
}
navigateAndWait(getServerURL(
BASE + 'fetch.html?path=accept&with-preflight'));
},
......
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