Reject requests with invalid streaming upload body

It seems in some cases network::URLLoader gets a request with a
streaming upload body, but the ChunkedDataPipeGetter for the body is
invalid. Reject such requests gracefully.

Bug: 1156550
Change-Id: Ic8f21cc00044efd839802279126095a681da811c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584640Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838598}

services/network/public/cpp/data_element.cc

@@ -81,6 +81,13 @@ mojo::PendingRemote<mojom::DataPipeGetter> DataElement::CloneDataPipeGetter()
   return clone;
 }
 
+const mojo::PendingRemote<mojom::ChunkedDataPipeGetter>&
+DataElement::chunked_data_pipe_getter() const {
+  DCHECK(type_ == mojom::DataElementType::kChunkedDataPipe ||
+         type_ == mojom::DataElementType::kReadOnceStream);
+  return chunked_data_pipe_getter_;
+}
+
 mojo::PendingRemote<mojom::ChunkedDataPipeGetter>
 DataElement::ReleaseChunkedDataPipeGetter() {
   DCHECK(type_ == mojom::DataElementType::kChunkedDataPipe ||

services/network/public/cpp/data_element.h

@@ -121,8 +121,12 @@ class COMPONENT_EXPORT(NETWORK_CPP_BASE) DataElement {
   mojo::PendingRemote<mojom::DataPipeGetter> ReleaseDataPipeGetter();
   mojo::PendingRemote<mojom::DataPipeGetter> CloneDataPipeGetter() const;
 
+  // Can be called only when this is of type kChunkedDataPipe or
+  // kReadOnceStream.
+  const mojo::PendingRemote<mojom::ChunkedDataPipeGetter>&
+  chunked_data_pipe_getter() const;
   // Takes ownership of the DataPipeGetter, if this is of
-  // TYPE_CHUNKED_DATA_PIPE.
+  // kChunkedDataPipe or kReadOnceStream.
   mojo::PendingRemote<mojom::ChunkedDataPipeGetter>
   ReleaseChunkedDataPipeGetter();
 

services/network/url_loader.cc

@@ -648,6 +648,25 @@ URLLoader::URLLoader(
 
   // Resolve elements from request_body and prepare upload data.
   if (request.request_body.get()) {
+    const auto& elements = *request.request_body->elements();
+    // TODO(crbug.com/1156550): Move this logic to the deserialization part.
+    if (elements.size() == 1 &&
+        (elements[0].type() == mojom::DataElementType::kChunkedDataPipe ||
+         elements[0].type() == mojom::DataElementType::kReadOnceStream)) {
+      const bool chunked_data_pipe_getter_is_valid =
+          elements[0].chunked_data_pipe_getter().is_valid();
+      UMA_HISTOGRAM_BOOLEAN(
+          "NetworkService.StreamingUploadDataPipeGetterValidity",
+          chunked_data_pipe_getter_is_valid);
+      if (!chunked_data_pipe_getter_is_valid) {
+        base::SequencedTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE,
+            base::BindOnce(&URLLoader::NotifyCompleted, base::Unretained(this),
+                           net::ERR_INVALID_ARGUMENT));
+        return;
+      }
+    }
+
     OpenFilesForUpload(request);
     return;
   }

tools/metrics/histograms/histograms_xml/network/histograms.xml

@@ -2151,6 +2151,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="NetworkService.StreamingUploadDataPipeGetterValidity"
+    units="BooleanValid" expires_after="2021-03-31">
+  <owner>yhirano@chromium.org</owner>
+  <owner>yoichio@chromium.org</owner>
+  <summary>
+    It seems in some cases the ChunkedDataPipeGetter for a request body is
+    missing. This histogram counts whether the mojo::Remote is valid, for each
+    request with body of type KChunkedDataPipe or kReadOnceStream.
+  </summary>
+</histogram>
+
 <histogram name="NetworkService.TimeToFirstResponse" units="ms"
     expires_after="M82">
   <owner>cduvall@chromium.org</owner>