Bug 1608911 - Send reports to endpoint as a collection. r=baku

In order to respect the specification, we need to send the reports as a collection.

This gives us the opportunity to group reports by endpoints and principal. That
way, if we have multiple reports to send to the same endpoint, we can do it
with only one request.

Differential Revision: https://phabricator.services.mozilla.com/D59819

--HG--
extra : moz-landing-system : lando

dom/reporting/ReportDeliver.cpp

@@ -32,8 +32,9 @@ class ReportFetchHandler final : public PromiseNativeHandler {
  public:
   NS_DECL_ISUPPORTS
 
-  explicit ReportFetchHandler(const ReportDeliver::ReportData& aReportData)
+  explicit ReportFetchHandler(
-      : mReportData(aReportData) {}
+      const nsTArray<ReportDeliver::ReportData>& aReportData)
+      : mReports(aReportData) {}
 
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     if (!gReportDeliver) {
@@ -57,86 +58,38 @@ class ReportFetchHandler final : public PromiseNativeHandler {
         mozilla::ipc::PBackgroundChild* actorChild =
             mozilla::ipc::BackgroundChild::GetOrCreateForCurrentThread();
 
+        for (const auto& report : mReports) {
           mozilla::ipc::PrincipalInfo principalInfo;
           nsresult rv =
-            PrincipalToPrincipalInfo(mReportData.mPrincipal, &principalInfo);
+              PrincipalToPrincipalInfo(report.mPrincipal, &principalInfo);
           if (NS_WARN_IF(NS_FAILED(rv))) {
-          return;
+            continue;
           }
 
-        actorChild->SendRemoveEndpoint(mReportData.mGroupName,
+          actorChild->SendRemoveEndpoint(report.mGroupName, report.mEndpointURL,
-                                       mReportData.mEndpointURL, principalInfo);
+                                         principalInfo);
+        }
       }
     }
   }
 
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override {
     if (gReportDeliver) {
-      ++mReportData.mFailures;
+      for (auto& report : mReports) {
-      gReportDeliver->AppendReportData(mReportData);
+        ++report.mFailures;
+        gReportDeliver->AppendReportData(report);
+      }
     }
   }
 
  private:
   ~ReportFetchHandler() = default;
 
-  ReportDeliver::ReportData mReportData;
+  nsTArray<ReportDeliver::ReportData> mReports;
 };
 
 NS_IMPL_ISUPPORTS0(ReportFetchHandler)
 
-// This RAII class keeps a list of sandboxed globals for the delivering of
-// reports. In this way, if we have to deliver more than 1 report to the same
-// origin, we reuse the sandbox.
-class MOZ_RAII SandboxGlobalHolder final {
- public:
-  nsIGlobalObject* GetOrCreateSandboxGlobalObject(nsIPrincipal* aPrincipal) {
-    MOZ_ASSERT(aPrincipal);
-
-    nsAutoCString origin;
-    nsresult rv = aPrincipal->GetOrigin(origin);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return nullptr;
-    }
-
-    nsCOMPtr<nsIGlobalObject> globalObject = mGlobals.Get(origin);
-    if (globalObject) {
-      return globalObject;
-    }
-
-    nsIXPConnect* xpc = nsContentUtils::XPConnect();
-    MOZ_ASSERT(xpc, "This should never be null!");
-
-    AutoJSAPI jsapi;
-    jsapi.Init();
-
-    JSContext* cx = jsapi.cx();
-    JS::Rooted<JSObject*> sandbox(cx);
-    rv = xpc->CreateSandbox(cx, aPrincipal, sandbox.address());
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return nullptr;
-    }
-
-    // The JSContext is not in a realm, so CreateSandbox returned an unwrapped
-    // global.
-    MOZ_ASSERT(JS_IsGlobalObject(sandbox));
-
-    globalObject = xpc::NativeGlobal(sandbox);
-    if (NS_WARN_IF(!globalObject)) {
-      return nullptr;
-    }
-
-    if (NS_WARN_IF(!mGlobals.Put(origin, globalObject, fallible))) {
-      return nullptr;
-    }
-
-    return globalObject;
-  }
-
- private:
-  nsInterfaceHashtable<nsCStringHashKey, nsIGlobalObject> mGlobals;
-};
-
 struct StringWriteFunc final : public JSONWriteFunc {
   nsACString&
       mBuffer;  // The lifetime of the struct must be bound to the buffer
@@ -157,10 +110,34 @@ class ReportJSONWriter final : public JSONWriter {
   }
 };
 
-void SendReport(ReportDeliver::ReportData& aReportData,
+void SendReports(nsTArray<ReportDeliver::ReportData>& aReports,
-                SandboxGlobalHolder& aHolder) {
+                 const nsCString& aEndPointUrl, nsIPrincipal* aPrincipal) {
-  nsCOMPtr<nsIGlobalObject> globalObject =
+  if (NS_WARN_IF(aReports.IsEmpty())) {
-      aHolder.GetOrCreateSandboxGlobalObject(aReportData.mPrincipal);
+    return;
+  }
+
+  nsIXPConnect* xpc = nsContentUtils::XPConnect();
+  MOZ_ASSERT(xpc, "This should never be null!");
+
+  nsCOMPtr<nsIGlobalObject> globalObject;
+  {
+    AutoJSAPI jsapi;
+    jsapi.Init();
+
+    JSContext* cx = jsapi.cx();
+    JS::Rooted<JSObject*> sandbox(cx);
+    nsresult rv = xpc->CreateSandbox(cx, aPrincipal, sandbox.address());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return;
+    }
+
+    // The JSContext is not in a realm, so CreateSandbox returned an unwrapped
+    // global.
+    MOZ_ASSERT(JS_IsGlobalObject(sandbox));
+
+    globalObject = xpc::NativeGlobal(sandbox);
+  }
+
   if (NS_WARN_IF(!globalObject)) {
     return;
   }
@@ -169,16 +146,21 @@ void SendReport(ReportDeliver::ReportData& aReportData,
   nsAutoCString body;
   ReportJSONWriter w(body);
 
-  w.Start();
+  w.StartArrayElement();
-
+  for (const auto& report : aReports) {
-  w.IntProperty(
+    MOZ_ASSERT(report.mPrincipal == aPrincipal);
-      "age", (TimeStamp::Now() - aReportData.mCreationTime).ToMilliseconds());
+    MOZ_ASSERT(report.mEndpointURL == aEndPointUrl);
-  w.StringProperty("type", NS_ConvertUTF16toUTF8(aReportData.mType).get());
+    w.StartObjectElement();
-  w.StringProperty("url", NS_ConvertUTF16toUTF8(aReportData.mURL).get());
+    w.IntProperty("age",
+                  (TimeStamp::Now() - report.mCreationTime).ToMilliseconds());
+    w.StringProperty("type", NS_ConvertUTF16toUTF8(report.mType).get());
+    w.StringProperty("url", NS_ConvertUTF16toUTF8(report.mURL).get());
     w.StringProperty("user_agent",
-                   NS_ConvertUTF16toUTF8(aReportData.mUserAgent).get());
+                     NS_ConvertUTF16toUTF8(report.mUserAgent).get());
-  w.JSONProperty("body", aReportData.mReportBodyJSON.get());
+    w.JSONProperty("body", report.mReportBodyJSON.get());
-  w.End();
+    w.EndObject();
+  }
+  w.EndArray();
 
   // The body as stream
   nsCOMPtr<nsIInputStream> streamBody;
@@ -196,7 +178,7 @@ void SendReport(ReportDeliver::ReportData& aReportData,
 
   // URL and fragments
   nsCOMPtr<nsIURI> uri;
-  rv = NS_NewURI(getter_AddRefs(uri), aReportData.mEndpointURL);
+  rv = NS_NewURI(getter_AddRefs(uri), aEndPointUrl);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
@@ -238,14 +220,16 @@ void SendReport(ReportDeliver::ReportData& aReportData,
   RefPtr<Promise> promise = FetchRequest(
       globalObject, fetchInput, RequestInit(), CallerType::NonSystem, error);
   if (error.Failed()) {
-    ++aReportData.mFailures;
+    for (auto& report : aReports) {
+      ++report.mFailures;
       if (gReportDeliver) {
-      gReportDeliver->AppendReportData(aReportData);
+        gReportDeliver->AppendReportData(report);
+      }
     }
     return;
   }
 
-  RefPtr<ReportFetchHandler> handler = new ReportFetchHandler(aReportData);
+  RefPtr<ReportFetchHandler> handler = new ReportFetchHandler(aReports);
   promise->AppendNativeHandler(handler);
 }
 
@@ -357,10 +341,28 @@ ReportDeliver::Notify(nsITimer* aTimer) {
   nsTArray<ReportData> reports;
   reports.SwapElements(mReportQueue);
 
-  SandboxGlobalHolder holder;
+  // group reports by endpoint and nsIPrincipal
-
+  std::map<std::pair<nsCString, nsCOMPtr<nsIPrincipal>>, nsTArray<ReportData>>
+      reportsByPrincipal;
   for (ReportData& report : reports) {
-    SendReport(report, holder);
+    auto already_seen =
+        reportsByPrincipal.find({report.mEndpointURL, report.mPrincipal});
+    if (already_seen == reportsByPrincipal.end()) {
+      reportsByPrincipal.emplace(
+          std::make_pair(report.mEndpointURL, report.mPrincipal),
+          nsTArray<ReportData>({report}));
+    } else {
+      already_seen->second.AppendElement(report);
+    }
+  }
+
+  for (auto& iter : reportsByPrincipal) {
+    std::pair<nsCString, nsCOMPtr<nsIPrincipal>> key = iter.first;
+    nsTArray<ReportData>& value = iter.second;
+    nsCString url = key.first;
+    nsCOMPtr<nsIPrincipal> principal = key.second;
+    nsAutoCString u(url);
+    SendReports(value, url, principal);
   }
 
   return NS_OK;

dom/reporting/tests/delivering.sjs

@@ -70,14 +70,6 @@ function handleRequest(aRequest, aResponse) {
       Array.prototype.push.apply(bytes, body.readByteArray(avail));
      }
 
-    let data = {
-      contentType: aRequest.getHeader("content-type"),
-      origin: aRequest.getHeader("origin"),
-      body: JSON.parse(String.fromCharCode.apply(null, bytes)),
-      url: aRequest.scheme + "://" + aRequest.host + aRequest.path +
-             (aRequest.queryString ? "&" + aRequest.queryString : ""),
-    }
-
      let reports = getState("report");
      if (!reports) {
        reports = [];
@@ -85,7 +77,18 @@ function handleRequest(aRequest, aResponse) {
        reports = JSON.parse(reports);
      }
 
+     const incoming_reports = JSON.parse(String.fromCharCode.apply(null, bytes));
+     for (let report of incoming_reports) {
+      let data = {
+        contentType: aRequest.getHeader("content-type"),
+        origin: aRequest.getHeader("origin"),
+        body: report,
+        url: aRequest.scheme + "://" + aRequest.host + aRequest.path +
+               (aRequest.queryString ? "&" + aRequest.queryString : ""),
+      }
       reports.push(data);
+     }
+
     setState("report", JSON.stringify(reports));
 
     if (params.has("410")) {