Reporting: Refactor ReportingCache's storage of clients and endpoints
See https://docs.google.com/document/d/19vDLCRVP-WvWxA_YNZfhkkW6fS5A2Ea9ulJ8i8AcPAM/edit?usp=sharing
for details.
This CL refactors the client side of the ReportingCacheImpl. Clients,
endpoint groups, and endpoints are now represented by separate structs.
The cache stores each in separate maps. Endpoints are evicted if an
origin exceeds the per-origin endpoint limit, or if the cache exceeds
the global endpoint limit.
This fixes an incorrect assumption made by the previous implementation
that a given endpoint URL cannot be in multiple groups for the same
origin.
Bug: 912951
Change-Id: I0c548c3cf20ac57a9f17482300dd9a57a14e53b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1453109
Commit-Queue: Lily Chen <[email protected]>
Reviewed-by: Joshua Bell <[email protected]>
Reviewed-by: Eric Orth <[email protected]>
Cr-Commit-Position: refs/heads/master@{#652467}diff --git a/net/reporting/reporting_header_parser.cc b/net/reporting/reporting_header_parser.cc
index 6d77afa..775cc79 100644
--- a/net/reporting/reporting_header_parser.cc
+++ b/net/reporting/reporting_header_parser.cc
@@ -5,12 +5,13 @@
#include "net/reporting/reporting_header_parser.h"
#include <string>
+#include <utility>
+#include <vector>
#include "base/bind.h"
#include "base/json/json_reader.h"
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
-#include "base/time/tick_clock.h"
#include "base/time/time.h"
#include "base/values.h"
#include "net/reporting/reporting_cache.h"
@@ -38,12 +39,6 @@
HeaderEndpointGroupOutcome::MAX);
}
-bool EndpointParsedSuccessfully(HeaderEndpointOutcome outcome) {
- return outcome == HeaderEndpointOutcome::REMOVED ||
- outcome == HeaderEndpointOutcome::SET_REJECTED_BY_DELEGATE ||
- outcome == HeaderEndpointOutcome::SET;
-}
-
void RecordHeaderEndpointOutcome(HeaderEndpointOutcome outcome) {
UMA_HISTOGRAM_ENUMERATION(
ReportingHeaderParser::kHeaderEndpointOutcomeHistogram, outcome,
@@ -54,7 +49,7 @@
const char kIncludeSubdomainsKey[] = "include_subdomains";
const char kEndpointsKey[] = "endpoints";
const char kGroupKey[] = "group";
-const char kGroupDefaultValue[] = "default";
+const char kDefaultGroupName[] = "default";
const char kMaxAgeKey[] = "max_age";
const char kPriorityKey[] = "priority";
const char kWeightKey[] = "weight";
@@ -66,17 +61,11 @@
// |value| is the parsed JSON value of the endpoint tuple.
//
// |*endpoint_out| will contain the endpoint URL parsed out of the tuple.
-HeaderEndpointOutcome ProcessEndpoint(ReportingDelegate* delegate,
- ReportingCache* cache,
- base::TimeTicks now,
- const std::string& group,
- int ttl_sec,
- ReportingClient::Subdomains subdomains,
- const url::Origin& origin,
- const base::Value& value,
- GURL* endpoint_url_out) {
- *endpoint_url_out = GURL();
-
+HeaderEndpointOutcome ProcessEndpoint(
+ ReportingDelegate* delegate,
+ const url::Origin& origin,
+ const base::Value& value,
+ ReportingClient::EndpointInfo* endpoint_info_out) {
const base::DictionaryValue* dict = nullptr;
if (!value.GetAsDictionary(&dict))
return HeaderEndpointOutcome::DISCARDED_NOT_DICTIONARY;
@@ -93,30 +82,25 @@
return HeaderEndpointOutcome::DISCARDED_URL_INVALID;
if (!endpoint_url.SchemeIsCryptographic())
return HeaderEndpointOutcome::DISCARDED_URL_INSECURE;
+ endpoint_info_out->url = std::move(endpoint_url);
- int priority = ReportingClient::kDefaultPriority;
+ int priority = ReportingClient::EndpointInfo::kDefaultPriority;
if (dict->HasKey(kPriorityKey) && !dict->GetInteger(kPriorityKey, &priority))
return HeaderEndpointOutcome::DISCARDED_PRIORITY_NOT_INTEGER;
+ if (priority < 0)
+ return HeaderEndpointOutcome::DISCARDED_PRIORITY_NEGATIVE;
+ endpoint_info_out->priority = priority;
- int weight = ReportingClient::kDefaultWeight;
+ int weight = ReportingClient::EndpointInfo::kDefaultWeight;
if (dict->HasKey(kWeightKey) && !dict->GetInteger(kWeightKey, &weight))
return HeaderEndpointOutcome::DISCARDED_WEIGHT_NOT_INTEGER;
- if (weight <= 0)
- return HeaderEndpointOutcome::DISCARDED_WEIGHT_NOT_POSITIVE;
-
- *endpoint_url_out = endpoint_url;
-
- if (ttl_sec == 0) {
- cache->RemoveClientForOriginAndEndpoint(origin, endpoint_url);
- return HeaderEndpointOutcome::REMOVED;
- }
+ if (weight < 0)
+ return HeaderEndpointOutcome::DISCARDED_WEIGHT_NEGATIVE;
+ endpoint_info_out->weight = weight;
if (!delegate->CanSetClient(origin, endpoint_url))
return HeaderEndpointOutcome::SET_REJECTED_BY_DELEGATE;
- cache->SetClient(origin, endpoint_url, subdomains, group,
- now + base::TimeDelta::FromSeconds(ttl_sec), priority,
- weight);
return HeaderEndpointOutcome::SET;
}
@@ -125,20 +109,21 @@
// |origin| is the origin that sent the Report-To header.
//
// |value| is the parsed JSON value of the endpoint group tuple.
-HeaderEndpointGroupOutcome ProcessEndpointGroup(ReportingDelegate* delegate,
- ReportingCache* cache,
- std::set<GURL>* new_endpoints,
- base::TimeTicks now,
- const url::Origin& origin,
- const base::Value& value) {
+HeaderEndpointGroupOutcome ProcessEndpointGroup(
+ ReportingDelegate* delegate,
+ ReportingCache* cache,
+ const url::Origin& origin,
+ const base::Value& value,
+ ReportingEndpointGroup* parsed_endpoint_group_out) {
const base::DictionaryValue* dict = nullptr;
if (!value.GetAsDictionary(&dict))
return HeaderEndpointGroupOutcome::DISCARDED_NOT_DICTIONARY;
DCHECK(dict);
- std::string group = kGroupDefaultValue;
- if (dict->HasKey(kGroupKey) && !dict->GetString(kGroupKey, &group))
+ std::string group_name = kDefaultGroupName;
+ if (dict->HasKey(kGroupKey) && !dict->GetString(kGroupKey, &group_name))
return HeaderEndpointGroupOutcome::DISCARDED_GROUP_NOT_STRING;
+ parsed_endpoint_group_out->name = std::move(group_name);
int ttl_sec = -1;
if (!dict->HasKey(kMaxAgeKey))
@@ -147,13 +132,18 @@
return HeaderEndpointGroupOutcome::DISCARDED_TTL_NOT_INTEGER;
if (ttl_sec < 0)
return HeaderEndpointGroupOutcome::DISCARDED_TTL_NEGATIVE;
+ // max_age: 0 signifies removal of the endpoint group.
+ if (ttl_sec == 0) {
+ cache->RemoveEndpointGroup(origin, group_name);
+ return HeaderEndpointGroupOutcome::REMOVED_TTL_ZERO;
+ }
+ parsed_endpoint_group_out->ttl = base::TimeDelta::FromSeconds(ttl_sec);
- ReportingClient::Subdomains subdomains = ReportingClient::Subdomains::EXCLUDE;
bool subdomains_bool = false;
if (dict->HasKey(kIncludeSubdomainsKey) &&
dict->GetBoolean(kIncludeSubdomainsKey, &subdomains_bool) &&
subdomains_bool == true) {
- subdomains = ReportingClient::Subdomains::INCLUDE;
+ parsed_endpoint_group_out->include_subdomains = OriginSubdomains::INCLUDE;
}
const base::ListValue* endpoint_list = nullptr;
@@ -162,20 +152,32 @@
if (!dict->GetList(kEndpointsKey, &endpoint_list))
return HeaderEndpointGroupOutcome::DISCARDED_ENDPOINTS_NOT_LIST;
+ std::vector<ReportingClient::EndpointInfo> endpoints;
+
for (size_t i = 0; i < endpoint_list->GetSize(); i++) {
const base::Value* endpoint = nullptr;
bool got_endpoint = endpoint_list->Get(i, &endpoint);
DCHECK(got_endpoint);
- GURL endpoint_url;
+
+ ReportingClient::EndpointInfo parsed_endpoint;
HeaderEndpointOutcome outcome =
- ProcessEndpoint(delegate, cache, now, group, ttl_sec, subdomains,
- origin, *endpoint, &endpoint_url);
- if (EndpointParsedSuccessfully(outcome))
- new_endpoints->insert(endpoint_url);
+ ProcessEndpoint(delegate, origin, *endpoint, &parsed_endpoint);
+
+ if (outcome == HeaderEndpointOutcome::SET)
+ endpoints.push_back(std::move(parsed_endpoint));
+
RecordHeaderEndpointOutcome(outcome);
}
+ // Remove the group if it is empty.
+ if (endpoints.empty()) {
+ cache->RemoveEndpointGroup(origin, group_name);
+ return HeaderEndpointGroupOutcome::REMOVED_EMPTY;
+ }
+
+ parsed_endpoint_group_out->endpoints = std::move(endpoints);
+
return HeaderEndpointGroupOutcome::PARSED;
}
@@ -233,27 +235,28 @@
url::Origin origin = url::Origin::Create(url);
- std::vector<GURL> old_endpoints;
- cache->GetEndpointsForOrigin(origin, &old_endpoints);
+ std::vector<ReportingEndpointGroup> parsed_header;
- std::set<GURL> new_endpoints;
-
- base::TimeTicks now = context->tick_clock()->NowTicks();
for (size_t i = 0; i < group_list->GetSize(); i++) {
- const base::Value* group = nullptr;
- bool got_group = group_list->Get(i, &group);
+ const base::Value* group_value = nullptr;
+ bool got_group = group_list->Get(i, &group_value);
DCHECK(got_group);
+ ReportingEndpointGroup parsed_endpoint_group;
HeaderEndpointGroupOutcome outcome = ProcessEndpointGroup(
- delegate, cache, &new_endpoints, now, origin, *group);
+ delegate, cache, origin, *group_value, &parsed_endpoint_group);
RecordHeaderEndpointGroupOutcome(outcome);
+ if (outcome == HeaderEndpointGroupOutcome::PARSED)
+ parsed_header.push_back(std::move(parsed_endpoint_group));
}
- // Remove any endpoints that weren't specified in the current header(s).
- for (const GURL& old_endpoint : old_endpoints) {
- if (new_endpoints.count(old_endpoint) == 0u)
- cache->RemoveClientForOriginAndEndpoint(origin, old_endpoint);
+ // Remove the client if it has no valid endpoint groups.
+ if (parsed_header.empty()) {
+ cache->RemoveClient(origin);
+ RecordHeaderOutcome(HeaderOutcome::REMOVED_EMPTY);
+ return;
}
+ cache->OnParsedHeader(origin, std::move(parsed_header));
RecordHeaderOutcome(HeaderOutcome::PARSED);
}
