Skip to content

Commit

Permalink
Trust Tokens: Give ERR_TRUST_TOKEN_OPERATION_CACHE_HIT more generic name
Browse files Browse the repository at this point in the history
Trust Tokens is a new web feature that extends the fetch API to allow
specifying executing certain cryptographic operations alongside outgoing
requests: the requests gain new HTTP request headers bearing outbound
cryptographic payloads, and the corresponding responses from servers
usually contain corresponding cryptographic responses; the browser
processes and strips the responses, updating local state.

Sometimes, these operations' results can be cached: when we encounter a
cached result from a previous operation, we fail the request with
ERR_TRUST_TOKEN_OPERATION_CACHE_HIT. This is because we assume that the
main reason for sending a request with an associated Trust Tokens
operation is executing the Trust Tokens operation (rather than sending
whatever's in the request body and obtaining whatever's in the response
body), so the caller wouldn't want to incur the resource consumption
caused by the request if the Trust Tokens operation's result is already
stored locally.

While failing a request with ERR_TRUST_TOKEN_OPERATION_CACHE_HIT follows
the ResourceError code path in Blink and causes fetch() calls
to reject, these requests:
- do not trigger additional "request failed" error prints in the
DevTools console (assuming that the caller catches the exception that
the fetch Promise rejects with); and
- show up as "cache hits" (black, in contrast to red errors) in the
DevTools network panel.

We're adding a new code path ("platform-provided trust tokens";
crbug.com/1130248) where we can have a Trust Tokens operation
complete by calling out to a system service, instead of sending a HTTP
request to a server. This is pretty similar to the existing "cache hit"
case in that we discover we don't need to send anything to a server
after all, so we bail early by "failing" the request (since it is messy
to populate an empty "success" response) in this case, too.

To make the error handling for Trust Tokens operations that are
successfully fulfilled locally have the same developer-facing
characteristics as cache hits (no console error; not a failure in the
network panel), we want to achieve similar error handling. The easiest
way to do this seemed to be making the existing net error more generic,
to the effect of "we didn't need to send a request to execute the
desired Trust Tokens operation, so there's no response to provide" and
setting the new error code for both cache hits and locally fulfilled
Trust Tokens operations.

This CL renames the error code; a follow-up CL will update URLLoader to
set this net error for locally fulfilled Trust Tokens operations in
addition to, as currently, cache hits.

R=mmenke

Bug: 1154847
Change-Id: I33e80515c77f2f8e0fc442ccbe9cc98c9b693738
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568205
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Auto-Submit: David Van Cleve <davidvc@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833371}
  • Loading branch information
David Van Cleve authored and Chromium LUCI CQ committed Dec 3, 2020
1 parent 1c09f5f commit a836ee9
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 12 deletions.
3 changes: 2 additions & 1 deletion content/renderer/loader/web_url_loader_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,8 @@ WebURLError WebURLLoaderImpl::PopulateURLError(

if (status.trust_token_operation_status !=
network::mojom::TrustTokenOperationStatus::kOk) {
DCHECK(status.error_code == net::ERR_TRUST_TOKEN_OPERATION_CACHE_HIT ||
DCHECK(status.error_code ==
net::ERR_TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST ||
status.error_code == net::ERR_TRUST_TOKEN_OPERATION_FAILED)
<< "Unexpected error code on Trust Token operation failure (or cache "
"hit): "
Expand Down
8 changes: 5 additions & 3 deletions net/base/net_error_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -874,9 +874,11 @@ NET_ERROR(INVALID_WEB_BUNDLE, -505)
NET_ERROR(TRUST_TOKEN_OPERATION_FAILED, -506)

// When handling a Trust Tokens protocol operation-executing request, the system
// found that the request's desired Trust Tokens results were already present in
// a local cache; as a result, the main request was cancelled.
NET_ERROR(TRUST_TOKEN_OPERATION_CACHE_HIT, -507)
// was able to execute the request's Trust Tokens operation without sending the
// request to its destination: for instance, the results could have been present
// in a local cache (for redemption) or the operation could have been diverted
// to a local provider (for "platform-provided" issuance).
NET_ERROR(TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST, -507)

// *** Code -600 is reserved (was FTP_PASV_COMMAND_FAILED). ***

Expand Down
7 changes: 4 additions & 3 deletions services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -888,9 +888,10 @@ void URLLoader::OnDoneBeginningTrustTokenOperation(
// Here and below, defer calling NotifyCompleted to make sure the URLLoader
// finishes initializing before getting deleted.
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&URLLoader::NotifyCompleted,
weak_ptr_factory_.GetWeakPtr(),
net::ERR_TRUST_TOKEN_OPERATION_CACHE_HIT));
FROM_HERE,
base::BindOnce(
&URLLoader::NotifyCompleted, weak_ptr_factory_.GetWeakPtr(),
net::ERR_TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST));
} else {
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&URLLoader::NotifyCompleted,
Expand Down
2 changes: 1 addition & 1 deletion services/network/url_loader_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5852,7 +5852,7 @@ TEST_P(URLLoaderSyncOrAsyncTrustTokenOperationTest,
delete_run_loop.Run();

EXPECT_EQ(client()->completion_status().error_code,
net::ERR_TRUST_TOKEN_OPERATION_CACHE_HIT);
net::ERR_TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST);
EXPECT_EQ(client()->completion_status().trust_token_operation_status,
mojom::TrustTokenOperationStatus::kAlreadyExists);
// Verify the DevTools event was fired and it has the right status.
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/public/platform/web_url_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ struct WebURLError {

// More detailed reason for failing the response with
// net::ERR_TRUST_TOKEN_OPERATION_FAILED or
// net::ERR_TRUST_TOKEN_OPERATION_CACHE_HIT.
// net::ERR_TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST.
//
// A value of kOk means that this request failed for a reason other than a
// Trust Tokens operation failure. This does not necessarily mean that a Trust
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ bool ResourceError::IsCancellation() const {
}

bool ResourceError::IsTrustTokenCacheHit() const {
return error_code_ == net::ERR_TRUST_TOKEN_OPERATION_CACHE_HIT;
return error_code_ ==
net::ERR_TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST;
}

bool ResourceError::IsUnactionableTrustTokensStatus() const {
Expand Down
6 changes: 4 additions & 2 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11334,7 +11334,8 @@ Called by update_net_error_codes.py.-->
<int value="-603" label="FTP_TRANSFER_ABORTED"/>
<int value="-602" label="FTP_SERVICE_UNAVAILABLE"/>
<int value="-601" label="FTP_FAILED"/>
<int value="-507" label="TRUST_TOKEN_OPERATION_CACHE_HIT"/>
<int value="-507"
label="TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST"/>
<int value="-506" label="TRUST_TOKEN_OPERATION_FAILED"/>
<int value="-505" label="INVALID_WEB_BUNDLE"/>
<int value="-504" label="INVALID_SIGNED_EXCHANGE"/>
Expand Down Expand Up @@ -50694,7 +50695,8 @@ Called by update_net_error_codes.py.-->
<int value="504" label="INVALID_SIGNED_EXCHANGE"/>
<int value="505" label="INVALID_WEB_BUNDLE"/>
<int value="506" label="TRUST_TOKEN_OPERATION_FAILED"/>
<int value="507" label="TRUST_TOKEN_OPERATION_CACHE_HIT"/>
<int value="507"
label="TRUST_TOKEN_OPERATION_SUCCESS_WITHOUT_SENDING_REQUEST"/>
<int value="601" label="FTP_FAILED"/>
<int value="602" label="FTP_SERVICE_UNAVAILABLE"/>
<int value="603" label="FTP_TRANSFER_ABORTED"/>
Expand Down

0 comments on commit a836ee9

Please sign in to comment.