Disallow streaming upload on HTTP/1.1

We decided not to allow the streaming upload feature on HTTP/1.1. Remove
the feature, and move tests accordingly.

See also: whatwg/fetch#966

Bug: 688906
Change-Id: I4e616469aad2378495ad81ba9034ca034f8ab1b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3734308
Commit-Queue: Yutaka Hirano <[email protected]>
Reviewed-by: Sam McNally <[email protected]>
Reviewed-by: Yoichi Osato <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1020706}

Author: yutakahirano

services/network/chunked_data_pipe_upload_data_stream.cc

@@ -203,6 +203,7 @@ void ChunkedDataPipeUploadDataStream::OnSizeReceived(int32_t status,
     // Clear |buf_| as well, so it's only non-null while there's a pending read.
     buf_ = nullptr;
     buf_len_ = 0;
+    chunked_data_pipe_getter_.reset();
 
     OnReadCompleted(status_);
 

third_party/blink/renderer/core/fetch/fetch_manager.cc

@@ -790,8 +790,6 @@ void FetchManager::Loader::PerformHTTPFetch() {
             resolver_->GetScriptState(),
             pending_remote.InitWithNewPipeAndPassReceiver());
         request.MutableBody().SetStreamBody(std::move(pending_remote));
-        request.SetAllowHTTP1ForStreamingUpload(
-            fetch_request_data_->AllowHTTP1ForStreamingUpload());
       }
     }
   }

third_party/blink/renderer/core/fetch/fetch_request_data.cc

@@ -227,8 +227,6 @@ FetchRequestData* FetchRequestData::CloneExceptBody() {
   request->is_history_navigation_ = is_history_navigation_;
   request->window_id_ = window_id_;
   request->trust_token_params_ = trust_token_params_;
-  request->allow_http1_for_streaming_upload_ =
-      allow_http1_for_streaming_upload_;
   return request;
 }
 

third_party/blink/renderer/core/fetch/fetch_request_data.h

@@ -154,13 +154,6 @@ class CORE_EXPORT FetchRequestData final
     trust_token_params_ = std::move(trust_token_params);
   }
 
-  void SetAllowHTTP1ForStreamingUpload(bool allow) {
-    allow_http1_for_streaming_upload_ = allow;
-  }
-  bool AllowHTTP1ForStreamingUpload() const {
-    return allow_http1_for_streaming_upload_;
-  }
-
   void Trace(Visitor*) const;
 
  private:
@@ -212,7 +205,6 @@ class CORE_EXPORT FetchRequestData final
   HeapMojoRemote<network::mojom::blink::URLLoaderFactory> url_loader_factory_;
   base::UnguessableToken window_id_;
   Member<ExecutionContext> execution_context_;
-  bool allow_http1_for_streaming_upload_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/fetch/request.cc

@@ -578,10 +578,6 @@ Request* Request::CreateRequestWithRequestOrString(
 
     request->SetTrustTokenParams(std::move(params));
   }
-  if (init->hasAllowHTTP1ForStreamingUpload()) {
-    request->SetAllowHTTP1ForStreamingUpload(
-        init->allowHTTP1ForStreamingUpload());
-  }
 
   // "Let |r| be a new Request object associated with |request| and a new
   // Headers object whose guard is "request"."

third_party/blink/renderer/core/fetch/request_init.idl

@@ -27,7 +27,6 @@ dictionary RequestInit {
     // contexts, this has to be enforced after the fact because the
     // SecureContext IDL attribute doesn't affect dictionary members.
     [RuntimeEnabled=TrustTokens] TrustToken trustToken;
-    [RuntimeEnabled=FetchUploadStreaming] boolean allowHTTP1ForStreamingUpload;
     // TODO(domfarolino): add support for RequestInit window member.
     //any window; // can only be set to null
 };

third_party/blink/renderer/platform/loader/fetch/fetch_api_request_body_mojom_traits.cc

@@ -27,14 +27,12 @@ StructTraits<blink::mojom::FetchAPIRequestBodyDataView,
   if (auto form_body = mutable_body.FormBody()) {
     // Here we need to keep the original body, because other members such as
     // `identifier` are on the form body.
-    network_body = NetworkResourceRequestBodyFor(
-        blink::ResourceRequestBody(form_body),
-        /*allow_http1_for_streaming_upload=*/false);
+    network_body =
+        NetworkResourceRequestBodyFor(blink::ResourceRequestBody(form_body));
   } else if (mutable_body.StreamBody()) {
     // Here we don't need to keep the original body (and it's impossible to do
     // so, because the streaming body is not copyable).
-    network_body = NetworkResourceRequestBodyFor(
-        std::move(mutable_body), /*allow_http1_for_streaming_upload=*/false);
+    network_body = NetworkResourceRequestBodyFor(std::move(mutable_body));
   }
   if (!network_body) {
     return WTF::Vector<network::DataElement>();

third_party/blink/renderer/platform/loader/fetch/resource_request.h

@@ -512,13 +512,6 @@ class PLATFORM_EXPORT ResourceRequestHead {
   // |url|,
   bool CanDisplay(const KURL&) const;
 
-  void SetAllowHTTP1ForStreamingUpload(bool allow) {
-    allowHTTP1ForStreamingUpload_ = allow;
-  }
-  bool AllowHTTP1ForStreamingUpload() const {
-    return allowHTTP1ForStreamingUpload_;
-  }
-
   // The original destination of a request passed through by a service worker.
   network::mojom::RequestDestination GetOriginalDestination() const {
     return original_destination_;
@@ -636,8 +629,6 @@ class PLATFORM_EXPORT ResourceRequestHead {
   // the prefetch cache will be restricted to top-level-navigations.
   bool prefetch_maybe_for_top_level_navigation_ = false;
 
-  bool allowHTTP1ForStreamingUpload_ = false;
-
   // This is used when fetching preload header requests from cross-origin
   // prefetch responses. The browser process uses this token to ensure the
   // request is cached correctly.

third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc

@@ -233,8 +233,7 @@ void PopulateResourceRequestBody(const EncodedFormData& src,
 }  // namespace
 
 scoped_refptr<network::ResourceRequestBody> NetworkResourceRequestBodyFor(
-    ResourceRequestBody src_body,
-    bool allow_http1_for_streaming_upload) {
+    ResourceRequestBody src_body) {
   scoped_refptr<network::ResourceRequestBody> dest_body;
   if (const EncodedFormData* form_body = src_body.FormBody().get()) {
     dest_body = base::MakeRefCounted<network::ResourceRequestBody>();
@@ -247,8 +246,9 @@ scoped_refptr<network::ResourceRequestBody> NetworkResourceRequestBodyFor(
     dest_body->SetToChunkedDataPipe(
         ToCrossVariantMojoType(std::move(stream_body)),
         network::ResourceRequestBody::ReadOnlyOnce(true));
-    dest_body->SetAllowHTTP1ForStreamingUpload(
-        allow_http1_for_streaming_upload);
+  }
+  if (dest_body) {
+    dest_body->SetAllowHTTP1ForStreamingUpload(false);
   }
   return dest_body;
 }
@@ -375,8 +375,7 @@ void PopulateResourceRequest(const ResourceRequestHead& src,
 
   dest->is_favicon = src.IsFavicon();
 
-  dest->request_body = NetworkResourceRequestBodyFor(
-      std::move(src_body), src.AllowHTTP1ForStreamingUpload());
+  dest->request_body = NetworkResourceRequestBodyFor(std::move(src_body));
   if (dest->request_body) {
     DCHECK_NE(dest->method, net::HttpRequestHeaders::kGetMethod);
     DCHECK_NE(dest->method, net::HttpRequestHeaders::kHeadMethod);

third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.h

@@ -20,8 +20,7 @@ class ResourceRequestHead;
 class ResourceRequestBody;
 
 scoped_refptr<network::ResourceRequestBody> NetworkResourceRequestBodyFor(
-    const ResourceRequestBody src_body,
-    bool allow_http1_for_streaming_upload);
+    const ResourceRequestBody src_body);
 
 void PopulateResourceRequest(const ResourceRequestHead& src,
                              ResourceRequestBody src_body,

third_party/blink/web_tests/external/wpt/fetch/api/basic/request-upload.h2.any.js

@@ -3,6 +3,8 @@
 // META: script=/common/utils.js
 // META: script=/common/get-host-info.sub.js
 
+const duplex = "half";
+
 function testUpload(desc, url, method, createBody, expectedBody) {
   const requestInit = {method};
   promise_test(async () => {
@@ -17,6 +19,17 @@ function testUpload(desc, url, method, createBody, expectedBody) {
   }, desc);
 }
 
+function createStream(chunks) {
+  return new ReadableStream({
+    start: (controller) => {
+      for (const chunk of chunks) {
+        controller.enqueue(chunk);
+      }
+      controller.close();
+    }
+  });
+}
+
 const url = RESOURCES_DIR + "echo-content.h2.py"
 
 testUpload("Fetch with POST with empty ReadableStream", url,
@@ -49,7 +62,7 @@ promise_test(async (test) => {
     "/fetch/connection-pool/resources/network-partition-key.py?"
     + `status=421&uuid=${token()}&partition_id=${self.origin}`
     + `&dispatch=check_partition&addcounter=true`,
-    {method: "POST", body: body});
+    {method: "POST", body: body, duplex});
   assert_equals(resp.status, 421);
   const text = await resp.text();
   assert_equals(text, "ok. Request was sent 1 times. 1 connections were created.");
@@ -82,3 +95,44 @@ promise_test(async (test) => {
   assert_equals(await response.text(), 'test', `Response has correct body`);
 }, "Feature detect for POST with ReadableStream, using request object");
 
+promise_test(async (t) => {
+  const body = createStream(["hello"]);
+  const method = "POST";
+  await promise_rejects_js(t, TypeError, fetch(url, { method, body, duplex }));
+}, "Streaming upload with body containing a String");
+
+promise_test(async (t) => {
+  const body = createStream([null]);
+  const method = "POST";
+  await promise_rejects_js(t, TypeError, fetch(url, { method, body, duplex }));
+}, "Streaming upload with body containing null");
+
+promise_test(async (t) => {
+  const body = createStream([33]);
+  const method = "POST";
+  await promise_rejects_js(t, TypeError, fetch(url, { method, body, duplex }));
+}, "Streaming upload with body containing a number");
+
+promise_test(async (t) => {
+  const url = "/fetch/api/resources/redirect.h2.py?location=/common/blank.html";
+  const body = createStream([]);
+  const method = "POST";
+  await promise_rejects_js(t, TypeError, fetch(url, { method, body, duplex }));
+}, "Streaming upload should fail on redirect (302)");
+
+promise_test(async (t) => {
+  const url = "/fetch/api/resources/redirect.h2.py?" +
+              "redirect_status=303&location=/common/blank.html";
+  const body = createStream([]);
+  const method = "POST";
+  const resp = await fetch(url, { method, body, duplex });
+  assert_equals(resp.status, 200, 'status');
+}, "Streaming upload should work with 303");
+
+promise_test(async (t) => {
+  const url = "/fetch/api/resources/authentication.py?realm=test";
+  const body = createStream([]);
+  const method = "POST";
+  await promise_rejects_js(t, TypeError, fetch(url, { method, body, duplex }));
+}, "Streaming upload should fail on a 401 response");
+

third_party/blink/web_tests/external/wpt/fetch/api/request/request-init-stream.any.js

@@ -50,7 +50,7 @@ test((t) => {
   new Request(request, { body: "..." });
 }, "Constructing a Request with a Request on which body.getReader().read() is called");
 
-promsie_test((t) => {
+promise_test(async (t) => {
   const request = new Request("...", { method: "POST", body: "..." });
   const reader = request.body.getReader();
   await reader.read();