Re: [whatwg/fetch] COEP:credentialless and the HTTP cache. (#1253)

I took a look at the specification, chromium and Firefox.

[Spec](https://fetch.spec.whatwg.org/#network-partition-keys):
```
To determine the network partition key,
==============================
given an environment settings object settings, run these steps:

1. Let topLevelOrigin be settings’s top-level origin.
2. If topLevelOrigin is null, then set topLevelOrigin to settings’s top-level creation URL’s origin.
3. Assert: topLevelOrigin is an origin.
4. Let topLevelSite be the result of obtaining a site, given topLevelOrigin.
5 .Let secondKey be null or an implementation-defined value.
6. Return (topLevelSite, secondKey).

To determine the network partition key
=============================,
given request, run these steps:
1. If request’s reserved client is non-null, then return the result of determining the network partition key given request’s reserved client.
2. If request’s client is non-null, then return the result of determining the network partition key given request’s client.

Return null.
```

[Firefox](https://github.com/mozilla/gecko-dev/blob/b31b78eea683b0eb341c676adb422cd129909fe9/netwerk/protocol/http/nsHttpChannel.cpp#L4113):
```cpp
// Assembles a cache-key from the given pieces of information and |mLoadFlags|
void nsHttpChannel::AssembleCacheKey(const char* spec, uint32_t postID,
                                     nsACString& cacheKey) {
  cacheKey.Truncate();

  if (mLoadFlags & LOAD_ANONYMOUS) {
    cacheKey.AssignLiteral("anon&");
  }

  if (postID) {
    char buf[32];
    SprintfLiteral(buf, "id=%x&", postID);
    cacheKey.Append(buf);
  }

  if (!cacheKey.IsEmpty()) {
    cacheKey.AppendLiteral("uri=");
  }

  // Strip any trailing #ref from the URL before using it as the key
  const char* p = strchr(spec, '#');
  if (p) {
    cacheKey.Append(spec, p - spec);
  } else {
    cacheKey.Append(spec);
  }
}
```

[Chrome](https://source.chromium.org/chromium/chromium/src/+/main:net/http/http_cache.cc;l=570;drc=38b4657edd4e8d0d2accdb8964c35f5ee843a75c;bpv=0;bpt=1):
```cpp
// static
// Generate a key that can be used inside the cache.
std::string HttpCache::GenerateCacheKey(const HttpRequestInfo* request) {
  std::string isolation_key;

  if (IsSplitCacheEnabled()) {
    // Prepend the key with |kDoubleKeyPrefix| = "_dk_" to mark it as
    // 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(request->network_isolation_key.IsFullyPopulated());
    std::string subframe_document_resource_prefix =
        request->is_subframe_document_resource ? kSubframeDocumentResourcePrefix
                                               : "";
    isolation_key = base::StrCat(
        {kDoubleKeyPrefix, subframe_document_resource_prefix,
         request->network_isolation_key.ToString(), kDoubleKeySeparator});
  }

  // Strip out the reference, username, and password sections of the URL and
  // concatenate with the network isolation key if we are splitting the cache.
  std::string url = isolation_key + HttpUtil::SpecForRequest(request->url);

  // No valid URL can begin with numerals, so we should not have to worry
  // about collisions with normal URLs.
  if (request->upload_data_stream &&
      request->upload_data_stream->identifier()) {
    url.insert(0,
               base::StringPrintf("%" PRId64 "/",
                                  request->upload_data_stream->identifier()));
  }

  return url;
}
```

Two observations:
1. Both Chrome and Firefox are keying by form ID. The specification doesn't. If you are happy, I can make a PR to 'officialise' the behavior of both implementers.
2. Firefox is keying the cache by whether the request was make with credentials:
```cpp
  if (mLoadFlags & LOAD_ANONYMOUS) {
    cacheKey.AssignLiteral("anon&");
  }
```
It means Firefox is already implementing the behavior I need. It means I can both fix the issue AND align Chrome and Firefox at the same time. Wonderful! If you are happy, I can make a PR to update the specification and Chrome.
My current Chrome prototype and Firefox are alike two peas in a pod:
```cpp
  if (request->load_flags & LOAD_DO_NOT_SAVE_COOKIES) {
     [...]
  }
```

+CC @kinu FYI (who warned me |includeCredentials| might not be part of the HTTP cache key on Chrome, thanks!)

@yutakahirano, @annevk, @mikewest: I need this, so I would be happy to work on it. Would you have some comments?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/issues/1253#issuecomment-861639711

Received on Tuesday, 15 June 2021 16:21:46 UTC