[beacon] Fix ArrayBuffer and URLSearchParams data

`navigator.sendBeacon()` was not handling DOMArrayBuffer and
URLSearchParams inputs properly, resulting in failing WPTs related to
the sent Content-Type, as well as in the wrong data sent in the case
of DOMArrayBuffers.
This CL fixes that.

Bug: 876671
Change-Id: I17674b3041aa0f0bdbd1a570ab34be48b0dd98b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489986
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820134}

third_party/blink/common/features.cc

@@ -551,12 +551,6 @@ const base::Feature kCSSReducedFontLoadingLayoutInvalidations{
 const base::Feature kDiscardCodeCacheAfterFirstUse{
     "DiscardCodeCacheAfterFirstUse", base::FEATURE_DISABLED_BY_DEFAULT};
 
-// The kill-switch for the fix for https://crbug.com/1051439.
-// TODO(crbug.com/1053369): Remove this around M84.
-const base::Feature kSuppressContentTypeForBeaconMadeWithArrayBufferView{
-    "SuppressContentTypeForBeaconMadeWithArrayBufferView",
-    base::FEATURE_ENABLED_BY_DEFAULT};
-
 const base::Feature kBlockHTMLParserOnStyleSheets{
     "BlockHTMLParserOnStyleSheets", base::FEATURE_DISABLED_BY_DEFAULT};
 

third_party/blink/public/common/features.h

@@ -170,9 +170,6 @@ BLINK_COMMON_EXPORT extern const base::Feature
 
 BLINK_COMMON_EXPORT extern const base::Feature kDiscardCodeCacheAfterFirstUse;
 
-BLINK_COMMON_EXPORT extern const base::Feature
-    kSuppressContentTypeForBeaconMadeWithArrayBufferView;
-
 BLINK_COMMON_EXPORT extern const base::Feature kBlockHTMLParserOnStyleSheets;
 
 BLINK_COMMON_EXPORT extern const base::Feature kLinkDisabledNewSpecBehavior;

third_party/blink/renderer/bindings/modules/v8/generated.gni

@@ -27,8 +27,6 @@ bindings_modules_generated_union_type_files = [
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_string.h",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_usv_string.cc",
   "$bindings_modules_v8_output_dir/array_buffer_or_array_buffer_view_or_usv_string.h",
-  "$bindings_modules_v8_output_dir/array_buffer_view_or_blob_or_string_or_form_data_or_readable_stream.cc",
-  "$bindings_modules_v8_output_dir/array_buffer_view_or_blob_or_string_or_form_data_or_readable_stream.h",
   "$bindings_modules_v8_output_dir/audio_context_latency_category_or_double.cc",
   "$bindings_modules_v8_output_dir/audio_context_latency_category_or_double.h",
   "$bindings_modules_v8_output_dir/boolean_or_constrain_boolean_parameters.cc",
@@ -87,6 +85,8 @@ bindings_modules_generated_union_type_files = [
   "$bindings_modules_v8_output_dir/path_2d_or_string.h",
   "$bindings_modules_v8_output_dir/point_2d_sequence_or_constrain_point_2d_parameters.cc",
   "$bindings_modules_v8_output_dir/point_2d_sequence_or_constrain_point_2d_parameters.h",
+  "$bindings_modules_v8_output_dir/readable_stream_or_xml_http_request_body_init.cc",
+  "$bindings_modules_v8_output_dir/readable_stream_or_xml_http_request_body_init.h",
   "$bindings_modules_v8_output_dir/rendering_context.cc",
   "$bindings_modules_v8_output_dir/rendering_context.h",
   "$bindings_modules_v8_output_dir/request_or_usv_string_or_request_or_usv_string_sequence.cc",

third_party/blink/renderer/bindings/scripts/bind_gen/path_manager.py

@@ -221,4 +221,7 @@ _BACKWARD_COMPATIBLE_UNION_FILEPATHS = {
     # core/xmlhttprequest/xml_http_request.idl
     "DocumentOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString":
     "DocumentOrXMLHttpRequestBodyInit",
+    # modules/beacon/navigator_beacon.idl
+    'ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString':
+    'ReadableStreamOrXMLHttpRequestBodyInit',
 }

third_party/blink/renderer/bindings/scripts/utilities.py

@@ -502,6 +502,9 @@ def shorten_union_name(union_type):
         # core/xmlhttprequest/xml_http_request.idl
         'DocumentOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString':
         'DocumentOrXMLHttpRequestBodyInit',
+        # modules/beacon/navigator_beacon.idl
+        'ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString':
+        'ReadableStreamOrXMLHttpRequestBodyInit',
     }
 
     idl_type = union_type

third_party/blink/renderer/core/loader/ping_loader.cc

@@ -42,6 +42,7 @@
 #include "third_party/blink/renderer/core/frame/local_frame_client.h"
 #include "third_party/blink/renderer/core/html/forms/form_data.h"
 #include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer_view.h"
+#include "third_party/blink/renderer/core/url/url_search_params.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/loader/cors/cors.h"
 #include "third_party/blink/renderer/platform/loader/fetch/fetch_context.h"
@@ -146,13 +147,6 @@ class BeaconDOMArrayBufferView final : public Beacon {
         data_->BaseAddress(),
         base::checked_cast<wtf_size_t>(data_->byteLengthAsSizeT()));
     request.SetHttpBody(std::move(entity_body));
-
-    if (!base::FeatureList::IsEnabled(
-            features::kSuppressContentTypeForBeaconMadeWithArrayBufferView)) {
-      // FIXME: a reasonable choice, but not in the spec; should it give a
-      // default?
-      request.SetHTTPContentType(AtomicString("application/octet-stream"));
-    }
   }
 
   const AtomicString GetContentType() const override { return g_null_atom; }
@@ -161,6 +155,53 @@ class BeaconDOMArrayBufferView final : public Beacon {
   DOMArrayBufferView* const data_;
 };
 
+class BeaconDOMArrayBuffer final : public Beacon {
+ public:
+  explicit BeaconDOMArrayBuffer(DOMArrayBuffer* data) : data_(data) {
+    CHECK(base::CheckedNumeric<wtf_size_t>(data->ByteLengthAsSizeT()).IsValid())
+        << "EncodedFormData::Create cannot deal with huge ArrayBuffers.";
+  }
+
+  uint64_t size() const override { return data_->ByteLengthAsSizeT(); }
+
+  void Serialize(ResourceRequest& request) const override {
+    DCHECK(data_);
+
+    scoped_refptr<EncodedFormData> entity_body = EncodedFormData::Create(
+        data_->Data(),
+        base::checked_cast<wtf_size_t>(data_->ByteLengthAsSizeT()));
+    request.SetHttpBody(std::move(entity_body));
+  }
+
+  const AtomicString GetContentType() const override { return g_null_atom; }
+
+ private:
+  DOMArrayBuffer* const data_;
+};
+
+class BeaconURLSearchParams final : public Beacon {
+ public:
+  explicit BeaconURLSearchParams(URLSearchParams* data) : data_(data) {}
+
+  uint64_t size() const override {
+    return data_->toString().CharactersSizeInBytes();
+  }
+
+  void Serialize(ResourceRequest& request) const override {
+    DCHECK(data_);
+
+    request.SetHttpBody(data_->ToEncodedFormData());
+    request.SetHTTPContentType(GetContentType());
+  }
+
+  const AtomicString GetContentType() const override {
+    return AtomicString("application/x-www-form-urlencoded;charset=UTF-8");
+  }
+
+ private:
+  URLSearchParams* const data_;
+};
+
 class BeaconFormData final : public Beacon {
  public:
   explicit BeaconFormData(FormData* data)
@@ -292,6 +333,22 @@ bool PingLoader::SendBeacon(const ScriptState& state,
   return SendBeaconCommon(state, frame, beacon_url, beacon);
 }
 
+bool PingLoader::SendBeacon(const ScriptState& state,
+                            LocalFrame* frame,
+                            const KURL& beacon_url,
+                            DOMArrayBuffer* data) {
+  BeaconDOMArrayBuffer beacon(data);
+  return SendBeaconCommon(state, frame, beacon_url, beacon);
+}
+
+bool PingLoader::SendBeacon(const ScriptState& state,
+                            LocalFrame* frame,
+                            const KURL& beacon_url,
+                            URLSearchParams* data) {
+  BeaconURLSearchParams beacon(data);
+  return SendBeaconCommon(state, frame, beacon_url, beacon);
+}
+
 bool PingLoader::SendBeacon(const ScriptState& state,
                             LocalFrame* frame,
                             const KURL& beacon_url,

third_party/blink/renderer/core/loader/ping_loader.h

@@ -45,11 +45,13 @@ namespace blink {
 
 class Blob;
 class DOMArrayBufferView;
+class DOMArrayBuffer;
 class EncodedFormData;
 class FormData;
 class LocalFrame;
 class KURL;
 class ScriptState;
+class URLSearchParams;
 
 // Issue an asynchronous, one-directional request at some resources, ignoring
 // any response. The request is made independent of any LocalFrame staying
@@ -84,6 +86,14 @@ class CORE_EXPORT PingLoader {
                          LocalFrame*,
                          const KURL&,
                          DOMArrayBufferView*);
+  static bool SendBeacon(const ScriptState&,
+                         LocalFrame*,
+                         const KURL&,
+                         DOMArrayBuffer*);
+  static bool SendBeacon(const ScriptState&,
+                         LocalFrame*,
+                         const KURL&,
+                         URLSearchParams*);
   static bool SendBeacon(const ScriptState&, LocalFrame*, const KURL&, Blob*);
   static bool SendBeacon(const ScriptState&,
                          LocalFrame*,

third_party/blink/renderer/modules/beacon/navigator_beacon.cc

@@ -4,7 +4,7 @@
 
 #include "third_party/blink/renderer/modules/beacon/navigator_beacon.h"
 
-#include "third_party/blink/renderer/bindings/modules/v8/array_buffer_view_or_blob_or_string_or_form_data_or_readable_stream.h"
+#include "third_party/blink/renderer/bindings/modules/v8/readable_stream_or_xml_http_request_body_init.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/fileapi/blob.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
@@ -67,7 +67,8 @@ bool NavigatorBeacon::sendBeacon(
     ScriptState* script_state,
     Navigator& navigator,
     const String& urlstring,
-    const ArrayBufferViewOrBlobOrStringOrFormDataOrReadableStream& data,
+    const ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString&
+        data,
     ExceptionState& exception_state) {
   return NavigatorBeacon::From(navigator).SendBeaconImpl(
       script_state, urlstring, data, exception_state);
@@ -76,7 +77,8 @@ bool NavigatorBeacon::sendBeacon(
 bool NavigatorBeacon::SendBeaconImpl(
     ScriptState* script_state,
     const String& urlstring,
-    const ArrayBufferViewOrBlobOrStringOrFormDataOrReadableStream& data,
+    const ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString&
+        data,
     ExceptionState& exception_state) {
   ExecutionContext* context = ExecutionContext::From(script_state);
   KURL url = context->CompleteURL(urlstring);
@@ -85,7 +87,20 @@ bool NavigatorBeacon::SendBeaconImpl(
 
   bool allowed;
 
-  if (data.IsArrayBufferView()) {
+  if (data.IsArrayBuffer()) {
+    auto* data_buffer = data.GetAsArrayBuffer();
+    if (!base::CheckedNumeric<wtf_size_t>(data_buffer->ByteLengthAsSizeT())
+             .IsValid()) {
+      // At the moment the PingLoader::SendBeacon implementation cannot deal
+      // with huge ArrayBuffers.
+      exception_state.ThrowRangeError(
+          "The data provided to sendBeacon() exceeds the maximally possible "
+          "length, which is 4294967295.");
+      return false;
+    }
+    allowed = PingLoader::SendBeacon(
+        *script_state, GetSupplementable()->GetFrame(), url, data_buffer);
+  } else if (data.IsArrayBufferView()) {
     auto* data_view = data.GetAsArrayBufferView().View();
     if (!base::CheckedNumeric<wtf_size_t>(data_view->byteLengthAsSizeT())
              .IsValid()) {
@@ -102,14 +117,18 @@ bool NavigatorBeacon::SendBeaconImpl(
     Blob* blob = data.GetAsBlob();
     allowed = PingLoader::SendBeacon(
         *script_state, GetSupplementable()->GetFrame(), url, blob);
-  } else if (data.IsString()) {
+  } else if (data.IsUSVString()) {
     allowed =
         PingLoader::SendBeacon(*script_state, GetSupplementable()->GetFrame(),
-                               url, data.GetAsString());
+                               url, data.GetAsUSVString());
   } else if (data.IsFormData()) {
     allowed =
         PingLoader::SendBeacon(*script_state, GetSupplementable()->GetFrame(),
                                url, data.GetAsFormData());
+  } else if (data.IsURLSearchParams()) {
+    allowed =
+        PingLoader::SendBeacon(*script_state, GetSupplementable()->GetFrame(),
+                               url, data.GetAsURLSearchParams());
   } else if (data.IsReadableStream()) {
     exception_state.ThrowTypeError(
         "sendBeacon cannot have a ReadableStream body.");

third_party/blink/renderer/modules/beacon/navigator_beacon.h

@@ -14,7 +14,8 @@ namespace blink {
 class ScriptState;
 class ExceptionState;
 class KURL;
-class ArrayBufferViewOrBlobOrStringOrFormDataOrReadableStream;
+class
+    ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString;
 
 class NavigatorBeacon final : public GarbageCollected<NavigatorBeacon>,
                               public Supplement<Navigator> {
@@ -30,7 +31,7 @@ class NavigatorBeacon final : public GarbageCollected<NavigatorBeacon>,
       ScriptState*,
       Navigator&,
       const String&,
-      const ArrayBufferViewOrBlobOrStringOrFormDataOrReadableStream&,
+      const ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString&,
       ExceptionState&);
 
   void Trace(Visitor*) const override;
@@ -39,7 +40,7 @@ class NavigatorBeacon final : public GarbageCollected<NavigatorBeacon>,
   bool SendBeaconImpl(
       ScriptState*,
       const String&,
-      const ArrayBufferViewOrBlobOrStringOrFormDataOrReadableStream&,
+      const ReadableStreamOrBlobOrArrayBufferOrArrayBufferViewOrFormDataOrURLSearchParamsOrUSVString&,
       ExceptionState&);
   bool CanSendBeacon(ExecutionContext*, const KURL&, ExceptionState&);
 };

third_party/blink/renderer/modules/beacon/navigator_beacon.idl

@@ -9,5 +9,5 @@
 ] partial interface Navigator {
     // TODO(ricea): |data| should be BodyInit? when the IDL compiler supports
     // it.
-    [CallWith=ScriptState, MeasureAs=SendBeacon, RaisesException] boolean sendBeacon(USVString url, optional (ArrayBufferView or Blob or DOMString or FormData or ReadableStream)? data = null);
+    [CallWith=ScriptState, MeasureAs=SendBeacon, RaisesException] boolean sendBeacon(USVString url, optional (ReadableStream or XMLHttpRequestBodyInit)? data = null);
 };

third_party/blink/web_tests/android/ChromiumWPTExpectations

@@ -147,7 +147,6 @@ crbug.com/1050754 external/wpt/battery-status/battery-disabled-by-feature-policy
 crbug.com/1050754 external/wpt/battery-status/battery-disallowed-in-cross-origin-iframe.https.sub.html [ Failure ]
 crbug.com/1050754 external/wpt/battery-status/battery-insecure-context.html [ Failure ]
 crbug.com/1050754 external/wpt/beacon/beacon-redirect.sub.window.html [ Failure ]
-crbug.com/1050754 external/wpt/beacon/headers/header-content-type.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-absent-getAvailability.https.window.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-added-getAvailability.https.window.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-powered-off-getAvailability.https.window.html [ Failure ]

third_party/blink/web_tests/android/WeblayerWPTExpectations

@@ -155,7 +155,6 @@ crbug.com/1050754 external/wpt/battery-status/battery-disabled-by-feature-policy
 crbug.com/1050754 external/wpt/battery-status/battery-disallowed-in-cross-origin-iframe.https.sub.html [ Failure ]
 crbug.com/1050754 external/wpt/battery-status/battery-insecure-context.html [ Failure ]
 crbug.com/1050754 external/wpt/beacon/beacon-redirect.sub.window.html [ Failure ]
-crbug.com/1050754 external/wpt/beacon/headers/header-content-type.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-absent-getAvailability.https.window.html [ Failure Timeout ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-added-getAvailability.https.window.html [ Failure Timeout ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-powered-off-getAvailability.https.window.html [ Failure Timeout ]

third_party/blink/web_tests/android/WebviewWPTExpectations

@@ -158,7 +158,6 @@ crbug.com/1050754 external/wpt/battery-status/battery-disabled-by-feature-policy
 crbug.com/1050754 external/wpt/battery-status/battery-disallowed-in-cross-origin-iframe.https.sub.html [ Failure ]
 crbug.com/1050754 external/wpt/battery-status/battery-insecure-context.html [ Failure ]
 crbug.com/1050754 external/wpt/beacon/beacon-redirect.sub.window.html [ Failure ]
-crbug.com/1050754 external/wpt/beacon/headers/header-content-type.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-absent-getAvailability.https.window.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-added-getAvailability.https.window.html [ Failure ]
 crbug.com/1050754 external/wpt/bluetooth/adapter/adapter-powered-off-getAvailability.https.window.html [ Failure ]

third_party/blink/web_tests/external/wpt/beacon/headers/header-content-type.html → third_party/blink/web_tests/external/wpt/beacon/headers/header-content-type-and-body.html

@@ -12,13 +12,13 @@
     <script>
 const RESOURCES_DIR = "/beacon/resources/";
 
-function testContentTypeHeader(what, contentType, title) {
+function testContentTypeAndBody(what, expected, title) {
   function wait(ms) {
     return new Promise(resolve => step_timeout(resolve, ms));
   }
   promise_test(async t => {
     const id = self.token();
-    const testUrl = new Request(RESOURCES_DIR + "content-type.py?cmd=put&id=" + id).url;
+    const testUrl = new Request(RESOURCES_DIR + "content-type-and-body.py?cmd=put&id=" + id).url;
     assert_equals(performance.getEntriesByName(testUrl).length, 0);
     assert_true(navigator.sendBeacon(testUrl, what), "SendBeacon Succeeded");
 
@@ -26,13 +26,17 @@ function testContentTypeHeader(what, contentType, title) {
       await wait(50);
     } while (performance.getEntriesByName(testUrl).length === 0);
     assert_equals(performance.getEntriesByName(testUrl).length, 1);
-    const checkUrl = RESOURCES_DIR + "content-type.py?cmd=get&id=" + id;
+    const checkUrl = RESOURCES_DIR + "content-type-and-body.py?cmd=get&id=" + id;
     const response = await fetch(checkUrl);
     const text = await response.text();
-    if (contentType === "multipart/form-data") {
+    if (expected.startsWith("multipart/form-data")) {
+      const split = expected.split(":");
+      const contentType = split[0];
+      const contentDisposition = "Content-Disposition: form-data; name=\"" + split[1] + "\"; filename=\"blob\"";
       assert_true(text.startsWith(contentType), "Correct Content-Type header result");
+      assert_true(text.includes(contentDisposition), "Body included value");
     } else {
-      assert_equals(text, contentType, "Correct Content-Type header result");
+      assert_equals(text, expected, "Correct Content-Type header result");
     }
   }, "Test content-type header for a body " + title);
 }
@@ -74,12 +78,12 @@ function stringToURLSearchParams(input)
   return new URLSearchParams(input);
 }
 
-testContentTypeHeader("hi!", "text/plain;charset=UTF-8", "string");
-testContentTypeHeader(stringToArrayBufferView("123"), "", "ArrayBufferView");
-testContentTypeHeader(stringToArrayBuffer("123"), "", "ArrayBuffer");
-testContentTypeHeader(stringToBlob("123"), "text/plain", "Blob");
-testContentTypeHeader(stringToFormData("qwerty"), "multipart/form-data", "FormData");
-testContentTypeHeader(stringToURLSearchParams("key1=value1&key2=value2"), "application/x-www-form-urlencoded;charset=UTF-8", "URLSearchParams");
+testContentTypeAndBody("hi!", "text/plain;charset=UTF-8: hi!", "string");
+testContentTypeAndBody(stringToArrayBufferView("123"), ": 1\0" + "2\0" + "3\0", "ArrayBufferView");
+testContentTypeAndBody(stringToArrayBuffer("123"), ": 1\0" + "2\0" + "3\0", "ArrayBuffer");
+testContentTypeAndBody(stringToBlob("123"), "text/plain: 123", "Blob");
+testContentTypeAndBody(stringToFormData("qwerty"), "multipart/form-data:qwerty", "FormData");
+testContentTypeAndBody(stringToURLSearchParams("key1=value1&key2=value2"), "application/x-www-form-urlencoded;charset=UTF-8: key1=value1&key2=value2", "URLSearchParams");
     </script>
   </body>
 </html>

third_party/blink/web_tests/external/wpt/beacon/headers/header-content-type-expected.txt

-This is a testharness.js-based test.
-PASS Test content-type header for a body string
-PASS Test content-type header for a body ArrayBufferView
-FAIL Test content-type header for a body ArrayBuffer assert_equals: Correct Content-Type header result expected "" but got "text/plain;charset=UTF-8"
-PASS Test content-type header for a body Blob
-PASS Test content-type header for a body FormData
-FAIL Test content-type header for a body URLSearchParams assert_equals: Correct Content-Type header result expected "application/x-www-form-urlencoded;charset=UTF-8" but got "text/plain;charset=UTF-8"
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/beacon/resources/content-type.py → third_party/blink/web_tests/external/wpt/beacon/resources/content-type-and-body.py

@@ -2,7 +2,7 @@ def main(request, response):
     command = request.GET.first(b"cmd").lower()
     test_id = request.GET.first(b"id")
     if command == b"put":
-        request.server.stash.put(test_id, request.headers.get(b"Content-Type", b""))
+        request.server.stash.put(test_id, request.headers.get(b"Content-Type", b"") + ": " + request.body)
         return [(b"Content-Type", b"text/plain")], u""
 
     if command == b"get":