Skip to content

Commit

Permalink
Don't serve in-memory cached responses when NIK is transient
Browse files Browse the repository at this point in the history
The following two NIKs, one is non-transient and the other is
transient, will generate the same cache key:
* NetworkIsolationKey("foo", "foo", <no nonce>)
* NetworkIsolationKey("foo", "foo", <some nonce>)

The in-memory cache should distinguish them. Don't serve responses
from the in-memory cache when the request's NIK is transient.

Bug: 1339708
Change-Id: Idd24ae6d30a9c11692f0ecab99ff2d095ee853c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3758298
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1024125}
  • Loading branch information
bashi authored and Chromium LUCI CQ committed Jul 14, 2022
1 parent 5ca8427 commit 31e1847
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 4 deletions.
11 changes: 10 additions & 1 deletion net/http/http_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ void HttpCache::OnExternalCacheHit(
if (!disk_cache_.get() || mode_ == DISABLE)
return;

if (IsSplitCacheEnabled() && network_isolation_key.IsTransient())
return;

HttpRequestInfo request_info;
request_info.url = url;
request_info.method = http_method;
Expand Down Expand Up @@ -453,6 +456,9 @@ Error HttpCache::CheckResourceExistence(
if (!disk_cache_)
return ERR_CACHE_MISS;

if (IsSplitCacheEnabled() && network_isolation_key.IsTransient())
return ERR_CACHE_MISS;

HttpRequestInfo request_info;
request_info.url = url;
request_info.method = std::string(method);
Expand Down Expand Up @@ -511,7 +517,7 @@ std::string HttpCache::GenerateCacheKey(
// double-keyed (and makes it an invalid url so that it doesn't get
// confused with a single-keyed entry). Separate the origin and url
// with invalid whitespace character |kDoubleKeySeparator|.
DCHECK(network_isolation_key.IsFullyPopulated());
DCHECK(!network_isolation_key.IsTransient());
std::string subframe_document_resource_prefix =
is_subframe_document_resource ? kSubframeDocumentResourcePrefix : "";
isolation_key =
Expand Down Expand Up @@ -708,6 +714,9 @@ void HttpCache::DoomMainEntryForUrl(const GURL& url,
if (!disk_cache_)
return;

if (IsSplitCacheEnabled() && isolation_key.IsTransient())
return;

HttpRequestInfo temp_info;
temp_info.url = url;
temp_info.method = "GET";
Expand Down
4 changes: 3 additions & 1 deletion services/network/network_service_memory_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,10 @@ absl::optional<std::string> NetworkServiceMemoryCache::CanServe(
// TODO(https://crbug.com/1339708): Support automatically assigned network
// isolation key for request from browsers. See comments in
// CorsURLLoaderFactory::CorsURLLoaderFactory.
if (!network_isolation_key.IsFullyPopulated())
if (net::HttpCache::IsSplitCacheEnabled() &&
network_isolation_key.IsTransient()) {
return absl::nullopt;
}

const GURL& url = resource_request.url;
if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS())
Expand Down
17 changes: 15 additions & 2 deletions services/network/network_service_memory_cache_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,14 @@ class NetworkServiceMemoryCacheTest : public testing::Test {

bool CanServeFromMemoryCache(
const ResourceRequest& request,
net::NetworkIsolationKey& network_isolation_key) {
const net::NetworkIsolationKey& network_isolation_key) {
return CanServeFromMemoryCache(request, network_isolation_key,
CrossOriginEmbedderPolicy());
}

bool CanServeFromMemoryCache(
const ResourceRequest& request,
net::NetworkIsolationKey& network_isolation_key,
const net::NetworkIsolationKey& network_isolation_key,
const CrossOriginEmbedderPolicy& cross_origin_embedder_policy) {
return memory_cache()
.CanServe(request, network_isolation_key, cross_origin_embedder_policy)
Expand Down Expand Up @@ -366,6 +366,19 @@ TEST_F(NetworkServiceMemoryCacheTest, CanServe_Basic) {
ASSERT_TRUE(CanServeFromMemoryCache(request));
}

TEST_F(NetworkServiceMemoryCacheTest, CanServe_NetworkIsolationKeyIsTransient) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(
net::features::kSplitCacheByNetworkIsolationKey);

ResourceRequest request = CreateRequest("/cacheable");
StoreResponseToMemoryCache(request);

ASSERT_TRUE(CanServeFromMemoryCache(request));
ASSERT_FALSE(CanServeFromMemoryCache(
request, net::NetworkIsolationKey::CreateTransient()));
}

TEST_F(NetworkServiceMemoryCacheTest, CanServe_InvalidURL) {
ResourceRequest request;
request.url = GURL();
Expand Down

0 comments on commit 31e1847

Please sign in to comment.