Reporting API: Remove old endpoints when processing header.

Origins shouldn't need to remember and explicitly remove every
deprecated endpoint they've used in the past.

This change alters the header parsing behavior so any endpoint that was
configured for an origin but is not included in a new header from that
origin will be removed.

Change-Id: I87590e00340684ad3deaefe5ff2a2939d904a983
Reviewed-on: https://chromium-review.googlesource.com/797737
Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521355}

net/reporting/reporting_cache.cc

@@ -167,38 +167,6 @@ class ReportingCacheImpl : public ReportingCache {
     context_->NotifyCacheUpdated();
   }
 
-  void GetClients(
-      std::vector<const ReportingClient*>* clients_out) const override {
-    clients_out->clear();
-    for (const auto& it : clients_)
-      for (const auto& endpoint_and_client : it.second)
-        clients_out->push_back(endpoint_and_client.second.get());
-  }
-
-  void GetClientsForOriginAndGroup(
-      const url::Origin& origin,
-      const std::string& group,
-      std::vector<const ReportingClient*>* clients_out) const override {
-    clients_out->clear();
-
-    const auto it = clients_.find(origin);
-    if (it != clients_.end()) {
-      for (const auto& endpoint_and_client : it->second) {
-        if (endpoint_and_client.second->group == group)
-          clients_out->push_back(endpoint_and_client.second.get());
-      }
-    }
-
-    // If no clients were found, try successive superdomain suffixes until a
-    // client with includeSubdomains is found or there are no more domain
-    // components left.
-    std::string domain = origin.host();
-    while (clients_out->empty() && !domain.empty()) {
-      GetWildcardClientsForDomainAndGroup(domain, group, clients_out);
-      domain = GetSuperdomain(domain);
-    }
-  }
-
   void SetClient(const url::Origin& origin,
                  const GURL& endpoint,
                  ReportingClient::Subdomains subdomains,
@@ -242,6 +210,51 @@ class ReportingCacheImpl : public ReportingCache {
     client_last_used_[client] = tick_clock()->NowTicks();
   }
 
+  void GetClients(
+      std::vector<const ReportingClient*>* clients_out) const override {
+    clients_out->clear();
+    for (const auto& it : clients_)
+      for (const auto& endpoint_and_client : it.second)
+        clients_out->push_back(endpoint_and_client.second.get());
+  }
+
+  void GetClientsForOriginAndGroup(
+      const url::Origin& origin,
+      const std::string& group,
+      std::vector<const ReportingClient*>* clients_out) const override {
+    clients_out->clear();
+
+    const auto it = clients_.find(origin);
+    if (it != clients_.end()) {
+      for (const auto& endpoint_and_client : it->second) {
+        if (endpoint_and_client.second->group == group)
+          clients_out->push_back(endpoint_and_client.second.get());
+      }
+    }
+
+    // If no clients were found, try successive superdomain suffixes until a
+    // client with includeSubdomains is found or there are no more domain
+    // components left.
+    std::string domain = origin.host();
+    while (clients_out->empty() && !domain.empty()) {
+      GetWildcardClientsForDomainAndGroup(domain, group, clients_out);
+      domain = GetSuperdomain(domain);
+    }
+  }
+
+  // TODO(juliatuttle): Unittests.
+  void GetEndpointsForOrigin(const url::Origin& origin,
+                             std::vector<GURL>* endpoints_out) const override {
+    endpoints_out->clear();
+
+    const auto it = clients_.find(origin);
+    if (it == clients_.end())
+      return;
+
+    for (const auto& endpoint_and_client : it->second)
+      endpoints_out->push_back(endpoint_and_client.first);
+  }
+
   void RemoveClients(
       const std::vector<const ReportingClient*>& clients_to_remove) override {
     for (const ReportingClient* client : clients_to_remove)
@@ -295,36 +308,6 @@ class ReportingCacheImpl : public ReportingCache {
   }
 
  private:
-  ReportingContext* context_;
-
-  // Owns all reports, keyed by const raw pointer for easier lookup.
-  std::unordered_map<const ReportingReport*, std::unique_ptr<ReportingReport>>
-      reports_;
-
-  // Reports that have been marked pending (in use elsewhere and should not be
-  // deleted until no longer pending).
-  std::unordered_set<const ReportingReport*> pending_reports_;
-
-  // Reports that have been marked doomed (would have been deleted, but were
-  // pending when the deletion was requested).
-  std::unordered_set<const ReportingReport*> doomed_reports_;
-
-  // Owns all clients, keyed by origin, then endpoint URL.
-  // (These would be unordered_map, but neither url::Origin nor GURL has a hash
-  // function implemented.)
-  std::map<url::Origin, std::map<GURL, std::unique_ptr<ReportingClient>>>
-      clients_;
-
-  // References but does not own all clients with includeSubdomains set, keyed
-  // by domain name.
-  std::unordered_map<std::string, std::unordered_set<const ReportingClient*>>
-      wildcard_clients_;
-
-  // The time that each client has last been used.
-  std::unordered_map<const ReportingClient*, base::TimeTicks> client_last_used_;
-
-  base::TickClock* tick_clock() { return context_->tick_clock(); }
-
   void RemoveReportInternal(const ReportingReport* report) {
     reports_[report]->RecordOutcome(tick_clock()->NowTicks());
     size_t erased = reports_.erase(report);
@@ -454,6 +437,36 @@ class ReportingCacheImpl : public ReportingCache {
     else
       return earliest_used;
   }
+
+  base::TickClock* tick_clock() { return context_->tick_clock(); }
+
+  ReportingContext* context_;
+
+  // Owns all reports, keyed by const raw pointer for easier lookup.
+  std::unordered_map<const ReportingReport*, std::unique_ptr<ReportingReport>>
+      reports_;
+
+  // Reports that have been marked pending (in use elsewhere and should not be
+  // deleted until no longer pending).
+  std::unordered_set<const ReportingReport*> pending_reports_;
+
+  // Reports that have been marked doomed (would have been deleted, but were
+  // pending when the deletion was requested).
+  std::unordered_set<const ReportingReport*> doomed_reports_;
+
+  // Owns all clients, keyed by origin, then endpoint URL.
+  // (These would be unordered_map, but neither url::Origin nor GURL has a hash
+  // function implemented.)
+  std::map<url::Origin, std::map<GURL, std::unique_ptr<ReportingClient>>>
+      clients_;
+
+  // References but does not own all clients with includeSubdomains set, keyed
+  // by domain name.
+  std::unordered_map<std::string, std::unordered_set<const ReportingClient*>>
+      wildcard_clients_;
+
+  // The time that each client has last been used.
+  std::unordered_map<const ReportingClient*, base::TimeTicks> client_last_used_;
 };
 
 }  // namespace

net/reporting/reporting_cache.h

@@ -126,6 +126,13 @@ class NET_EXPORT ReportingCache {
       const std::string& group,
       std::vector<const ReportingClient*>* clients_out) const = 0;
 
+  // Gets all of the endpoints in the cache configured for a particular origin.
+  // Does not pay attention to wildcard hosts; only returns endpoints configured
+  // by |origin| itself.
+  virtual void GetEndpointsForOrigin(
+      const url::Origin& origin,
+      std::vector<GURL>* endpoints_out) const = 0;
+
   // Removes a set of clients.
   //
   // May invalidate ReportingClient pointers returned by |GetClients| or

net/reporting/reporting_header_parser.cc

@@ -52,6 +52,12 @@ enum class HeaderEndpointOutcome {
   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("Reporting.HeaderEndpointOutcome", outcome,
                             HeaderEndpointOutcome::MAX);
@@ -63,11 +69,21 @@ const char kGroupKey[] = "group";
 const char kGroupDefaultValue[] = "default";
 const char kMaxAgeKey[] = "max-age";
 
+// Processes a single endpoint tuple received in a Report-To header.
+//
+// |origin| is the origin that sent the Report-To header.
+//
+// |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 GURL& url,
-                                      const base::Value& value) {
+                                      const url::Origin& origin,
+                                      const base::Value& value,
+                                      GURL* endpoint_url_out) {
+  *endpoint_url_out = GURL();
+
   const base::DictionaryValue* dict = nullptr;
   if (!value.GetAsDictionary(&dict))
     return HeaderEndpointOutcome::DISCARDED_NOT_DICTIONARY;
@@ -105,13 +121,13 @@ HeaderEndpointOutcome ProcessEndpoint(ReportingDelegate* delegate,
     subdomains = ReportingClient::Subdomains::INCLUDE;
   }
 
+  *endpoint_url_out = endpoint_url;
+
   if (ttl_sec == 0) {
-    cache->RemoveClientForOriginAndEndpoint(url::Origin::Create(url),
-                                            endpoint_url);
+    cache->RemoveClientForOriginAndEndpoint(origin, endpoint_url);
     return HeaderEndpointOutcome::REMOVED;
   }
 
-  url::Origin origin = url::Origin::Create(url);
   if (!delegate->CanSetClient(origin, endpoint_url))
     return HeaderEndpointOutcome::SET_REJECTED_BY_DELEGATE;
 
@@ -150,19 +166,37 @@ void ReportingHeaderParser::ParseHeader(ReportingContext* context,
     return;
   }
 
-  const base::ListValue* list = nullptr;
-  bool is_list = value->GetAsList(&list);
+  const base::ListValue* endpoint_list = nullptr;
+  bool is_list = value->GetAsList(&endpoint_list);
   DCHECK(is_list);
 
   ReportingDelegate* delegate = context->delegate();
   ReportingCache* cache = context->cache();
+
+  url::Origin origin = url::Origin::Create(url);
+
+  std::vector<GURL> old_endpoints;
+  cache->GetEndpointsForOrigin(origin, &old_endpoints);
+
+  std::set<GURL> new_endpoints;
+
   base::TimeTicks now = context->tick_clock()->NowTicks();
-  for (size_t i = 0; i < list->GetSize(); i++) {
+  for (size_t i = 0; i < endpoint_list->GetSize(); i++) {
     const base::Value* endpoint = nullptr;
-    bool got_endpoint = list->Get(i, &endpoint);
+    bool got_endpoint = endpoint_list->Get(i, &endpoint);
     DCHECK(got_endpoint);
-    RecordHeaderEndpointOutcome(
-        ProcessEndpoint(delegate, cache, now, url, *endpoint));
+    GURL endpoint_url;
+    HeaderEndpointOutcome outcome =
+        ProcessEndpoint(delegate, cache, now, origin, *endpoint, &endpoint_url);
+    if (EndpointParsedSuccessfully(outcome))
+      new_endpoints.insert(endpoint_url);
+    RecordHeaderEndpointOutcome(outcome);
+  }
+
+  // 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);
   }
 }
 

net/reporting/reporting_header_parser_unittest.cc

@@ -104,5 +104,22 @@ TEST_F(ReportingHeaderParserTest, ZeroMaxAge) {
   EXPECT_EQ(nullptr, FindClientInCache(cache(), kOrigin_, kEndpoint_));
 }
 
+TEST_F(ReportingHeaderParserTest, RemoveOld) {
+  static const GURL kDifferentEndpoint_ = GURL("https://endpoint2/");
+
+  ReportingHeaderParser::ParseHeader(
+      context(), kUrl_,
+      "{\"url\":\"" + kEndpoint_.spec() + "\",\"max-age\":86400}");
+
+  EXPECT_TRUE(FindClientInCache(cache(), kOrigin_, kEndpoint_));
+
+  ReportingHeaderParser::ParseHeader(
+      context(), kUrl_,
+      "{\"url\":\"" + kDifferentEndpoint_.spec() + "\",\"max-age\":86400}");
+
+  EXPECT_FALSE(FindClientInCache(cache(), kOrigin_, kEndpoint_));
+  EXPECT_TRUE(FindClientInCache(cache(), kOrigin_, kDifferentEndpoint_));
+}
+
 }  // namespace
 }  // namespace net