Commit 9f7d1ca5 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Remove the "RelaxIsolatedWorldCorsInFileUrlLoaderFactory" feature.

The "RelaxIsolatedWorldCorsInFileUrlLoaderFactory" feature was
introduced in r740899 in Q1 2020 just in case more extensions encounter
https://crbug.com/1049604 forcing us to revert to the old behavior.
This never happened and the feature was never enabled (and we don't have
any plans of using this feature in the future - quite the opposite, we
want to keep the feature disabled).

Based on the above this CL removes the feature (~reverting r740899).

Bug: 1049604
Change-Id: I4e2c2b61965661e61c75fd8cf7878715da4882d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466511Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817309}
parent cd61f872
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/command_line.h" #include "base/command_line.h"
#include "base/feature_list.h"
#include "base/files/file.h" #include "base/files/file.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/files/file_util.h" #include "base/files/file_util.h"
...@@ -36,7 +35,6 @@ ...@@ -36,7 +35,6 @@
#include "content/public/browser/file_url_loader.h" #include "content/public/browser/file_url_loader.h"
#include "content/public/browser/shared_cors_origin_access_list.h" #include "content/public/browser/shared_cors_origin_access_list.h"
#include "content/public/common/content_client.h" #include "content/public/common/content_client.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h" #include "content/public/common/content_switches.h"
#include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/remote.h"
...@@ -56,7 +54,6 @@ ...@@ -56,7 +54,6 @@
#include "services/network/public/cpp/cors/cors.h" #include "services/network/public/cpp/cors/cors.h"
#include "services/network/public/cpp/cors/cors_error_status.h" #include "services/network/public/cpp/cors/cors_error_status.h"
#include "services/network/public/cpp/cors/origin_access_list.h" #include "services/network/public/cpp/cors/origin_access_list.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/request_mode.h" #include "services/network/public/cpp/request_mode.h"
#include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/resource_request.h"
#include "services/network/public/mojom/cors.mojom-shared.h" #include "services/network/public/mojom/cors.mojom-shared.h"
...@@ -770,19 +767,6 @@ class FileURLLoader : public network::mojom::URLLoader { ...@@ -770,19 +767,6 @@ class FileURLLoader : public network::mojom::URLLoader {
DISALLOW_COPY_AND_ASSIGN(FileURLLoader); DISALLOW_COPY_AND_ASSIGN(FileURLLoader);
}; };
const url::Origin& GetCorsOrigin(const network::ResourceRequest& request) {
// Presence of |request_initiator| needs to be verified/ensured by the caller.
DCHECK(request.request_initiator.has_value());
if (request.isolated_world_origin.has_value() &&
base::FeatureList::IsEnabled(
features::kRelaxIsolatedWorldCorsInFileUrlLoaderFactory)) {
return request.isolated_world_origin.value();
}
return request.request_initiator.value();
}
} // namespace } // namespace
FileURLLoaderFactory::FileURLLoaderFactory( FileURLLoaderFactory::FileURLLoaderFactory(
...@@ -838,7 +822,7 @@ void FileURLLoaderFactory::CreateLoaderAndStart( ...@@ -838,7 +822,7 @@ void FileURLLoaderFactory::CreateLoaderAndStart(
url::Origin::Create(request.url)) || url::Origin::Create(request.url)) ||
(shared_cors_origin_access_list_ && (shared_cors_origin_access_list_ &&
shared_cors_origin_access_list_->GetOriginAccessList() shared_cors_origin_access_list_->GetOriginAccessList()
.CheckAccessState(GetCorsOrigin(request), request.url) == .CheckAccessState(*request.request_initiator, request.url) ==
network::cors::OriginAccessList::AccessState::kAllowed))); network::cors::OriginAccessList::AccessState::kAllowed)));
network::mojom::FetchResponseType response_type = network::mojom::FetchResponseType response_type =
......
...@@ -8,11 +8,9 @@ ...@@ -8,11 +8,9 @@
#include <string> #include <string>
#include "base/path_service.h" #include "base/path_service.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
#include "content/public/browser/shared_cors_origin_access_list.h" #include "content/public/browser/shared_cors_origin_access_list.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_paths.h" #include "content/public/common/content_paths.h"
#include "content/public/test/simple_url_loader_test_helper.h" #include "content/public/test/simple_url_loader_test_helper.h"
#include "net/base/filename_util.h" #include "net/base/filename_util.h"
...@@ -155,35 +153,10 @@ TEST_F(FileURLLoaderFactoryTest, MissedRequestInitiator) { ...@@ -155,35 +153,10 @@ TEST_F(FileURLLoaderFactoryTest, MissedRequestInitiator) {
network::mojom::RequestMode::kCorsWithForcedPreflight))); network::mojom::RequestMode::kCorsWithForcedPreflight)));
} }
// Test whether FileURLLoaderFactory can fetch/XHR files based on the
// request_initiator and isolated_world_origin.
//
// The behavior for isolated_world_origin depends on a feature, and the test is
// parameterized to cover both enabled and disabled cases.
class FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds
: public FileURLLoaderFactoryTest,
public testing::WithParamInterface<bool> {
public:
FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds() {
const base::Feature& feature =
features::kRelaxIsolatedWorldCorsInFileUrlLoaderFactory;
if (ShouldRelaxCorsForIsolatedWorlds()) {
feature_list_.InitAndEnableFeature(feature);
} else {
feature_list_.InitAndDisableFeature(feature);
}
}
bool ShouldRelaxCorsForIsolatedWorlds() { return GetParam(); }
private:
base::test::ScopedFeatureList feature_list_;
};
// Verify that FileURLLoaderFactory takes OriginAccessList into account when // Verify that FileURLLoaderFactory takes OriginAccessList into account when
// deciding whether to exempt a request from CORS. See also // deciding whether to exempt a request from CORS. See also
// https://crbug.com/1049604. // https://crbug.com/1049604.
TEST_P(FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds, Test) { TEST_F(FileURLLoaderFactoryTest, Allowlist) {
const url::Origin not_permitted_origin = const url::Origin not_permitted_origin =
url::Origin::Create(GURL("https://www.example.com")); url::Origin::Create(GURL("https://www.example.com"));
...@@ -197,20 +170,12 @@ TEST_P(FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds, Test) { ...@@ -197,20 +170,12 @@ TEST_P(FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds, Test) {
EXPECT_EQ(net::OK, CreateLoaderAndRun(CreateCorsRequestWithInitiator( EXPECT_EQ(net::OK, CreateLoaderAndRun(CreateCorsRequestWithInitiator(
GetPermittedSourceOrigin()))); GetPermittedSourceOrigin())));
// In the long-term, isolated world origin should *not* be used to check the // Isolated world origin should *not* be used to check the permission.
// permission. In the short-term we allow consulting isolated world origin if
// the RelaxIsolatedWorldCorsInFileUrlLoaderFactory feature is enabled (see
// https://crbug.com/1049604 for more discussion and details).
auto request = CreateCorsRequestWithInitiator(not_permitted_origin); auto request = CreateCorsRequestWithInitiator(not_permitted_origin);
request->isolated_world_origin = GetPermittedSourceOrigin(); request->isolated_world_origin = GetPermittedSourceOrigin();
EXPECT_EQ(ShouldRelaxCorsForIsolatedWorlds() ? net::OK : net::ERR_FAILED, EXPECT_EQ(net::ERR_FAILED, CreateLoaderAndRun(std::move(request)));
CreateLoaderAndRun(std::move(request)));
} }
INSTANTIATE_TEST_SUITE_P(FeatureState,
FileURLLoaderFactoryTestWithRelaxedIsolatedWorlds,
testing::Values(true, false));
} // namespace } // namespace
} // namespace content } // namespace content
...@@ -511,16 +511,6 @@ const base::Feature kPushSubscriptionChangeEvent{ ...@@ -511,16 +511,6 @@ const base::Feature kPushSubscriptionChangeEvent{
const base::Feature kDirectSockets{"DirectSockets", const base::Feature kDirectSockets{"DirectSockets",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
// Controls whether FileURLLoaderFactory can fetch additional files based on the
// isolated world's origin. This feature is disabled by default because we want
// content scripts to have the same permissions as the page they are injected
// into. This feature makes it possible to quickly revert to earlier permissive
// behavior if significant regressions are detected. See
// https://crbug.com/1049604.
const base::Feature kRelaxIsolatedWorldCorsInFileUrlLoaderFactory = {
"RelaxIsolatedWorldCorsInFileUrlLoaderFactory",
base::FEATURE_DISABLED_BY_DEFAULT};
// Causes hidden tabs with crashed subframes to be marked for reload, meaning // Causes hidden tabs with crashed subframes to be marked for reload, meaning
// that if a user later switches to that tab, the current page will be // that if a user later switches to that tab, the current page will be
// reloaded. This will hide crashed subframes from the user at the cost of // reloaded. This will hide crashed subframes from the user at the cost of
......
...@@ -111,8 +111,6 @@ CONTENT_EXPORT extern const base::Feature ...@@ -111,8 +111,6 @@ CONTENT_EXPORT extern const base::Feature
kProcessSharingWithStrictSiteInstances; kProcessSharingWithStrictSiteInstances;
CONTENT_EXPORT extern const base::Feature kPushSubscriptionChangeEvent; CONTENT_EXPORT extern const base::Feature kPushSubscriptionChangeEvent;
CONTENT_EXPORT extern const base::Feature kDirectSockets; CONTENT_EXPORT extern const base::Feature kDirectSockets;
CONTENT_EXPORT extern const base::Feature
kRelaxIsolatedWorldCorsInFileUrlLoaderFactory;
CONTENT_EXPORT extern const base::Feature kReloadHiddenTabsWithCrashedSubframes; CONTENT_EXPORT extern const base::Feature kReloadHiddenTabsWithCrashedSubframes;
CONTENT_EXPORT extern const base::Feature kRenderDocument; CONTENT_EXPORT extern const base::Feature kRenderDocument;
CONTENT_EXPORT extern const base::Feature kRequestUnbufferedDispatch; CONTENT_EXPORT extern const base::Feature kRequestUnbufferedDispatch;
......
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