Process headers in chrome_pdf::BlinkUrlLoader

Translates between the UrlRequest/UrlResponse (flat string) and
blink::WebURLRequest/blink::WebURLResponse (key-value map) header
formats, based on the behavior of Pepper's
content::CreateWebURLRequest() and content::DataFromWebURLResponse().

The Pepper methods do not distinguish between absent and empty strings,
so UrlRequest and UrlResponse now use empty strings as the "absent"
value as well, as there's no need to support this state separately.

Bug: 1099022
Change-Id: Ia509bea005f356ccbe2bb79aff73a01fd3fd9eed
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2415374
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808171}

pdf/ppapi_migration/url_loader.cc

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <algorithm>
+#include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ -16,6 +17,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/notreached.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_util.h"
 #include "pdf/ppapi_migration/callback.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/trusted/ppb_url_loader_trusted.h"
@@ -27,6 +29,7 @@
 #include "ppapi/cpp/url_response_info.h"
 #include "ppapi/cpp/var.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-shared.h"
+#include "third_party/blink/public/platform/web_http_header_visitor.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_url.h"
 #include "third_party/blink/public/platform/web_url_error.h"
@@ -38,6 +41,29 @@
 
 namespace chrome_pdf {
 
+namespace {
+
+// Taken from `content/renderer/pepper/url_response_info_util.cc`.
+class HeadersToString final : public blink::WebHTTPHeaderVisitor {
+ public:
+  explicit HeadersToString(std::string& buffer_ref) : buffer_ref_(buffer_ref) {}
+
+  void VisitHeader(const blink::WebString& name,
+                   const blink::WebString& value) override {
+    if (!buffer_ref_.empty())
+      buffer_ref_.append("\n");
+    buffer_ref_.append(name.Utf8());
+    buffer_ref_.append(": ");
+    buffer_ref_.append(value.Utf8());
+  }
+
+ private:
+  // Reference allows writing directly into `UrlResponse::headers`.
+  std::string& buffer_ref_;
+};
+
+}  // namespace
+
 UrlRequest::UrlRequest() = default;
 UrlRequest::UrlRequest(const UrlRequest& other) = default;
 UrlRequest::UrlRequest(UrlRequest&& other) noexcept = default;
@@ -83,6 +109,15 @@ void BlinkUrlLoader::Open(const UrlRequest& request, ResultCallback callback) {
   blink_request.SetUrl(GURL(request.url));
   blink_request.SetHttpMethod(blink::WebString::FromASCII(request.method));
 
+  if (!request.headers.empty()) {
+    net::HttpUtil::HeadersIterator it(request.headers.begin(),
+                                      request.headers.end(), "\n\r");
+    while (it.GetNext()) {
+      blink_request.AddHttpHeaderField(blink::WebString::FromUTF8(it.name()),
+                                       blink::WebString::FromUTF8(it.values()));
+    }
+  }
+
   blink_request.SetRequestContext(blink::mojom::RequestContextType::PLUGIN);
   blink_request.SetRequestDestination(
       network::mojom::RequestDestination::kEmbed);
@@ -150,8 +185,12 @@ void BlinkUrlLoader::DidSendData(uint64_t bytes_sent,
 void BlinkUrlLoader::DidReceiveResponse(const blink::WebURLResponse& response) {
   DCHECK_EQ(state_, LoadingState::kOpening);
 
+  // Modeled on `content::DataFromWebURLResponse()`.
   mutable_response().status_code = response.HttpStatusCode();
 
+  HeadersToString headers_to_string(mutable_response().headers);
+  response.VisitHttpHeaderFields(&headers_to_string);
+
   state_ = LoadingState::kStreamingData;
   std::move(open_callback_).Run(PP_OK);
 }
@@ -278,11 +317,11 @@ void PepperUrlLoader::Open(const UrlRequest& request, ResultCallback callback) {
   if (request.ignore_redirects)
     pp_request.SetFollowRedirects(false);
 
-  if (request.custom_referrer_url.has_value())
-    pp_request.SetCustomReferrerURL(request.custom_referrer_url.value());
+  if (!request.custom_referrer_url.empty())
+    pp_request.SetCustomReferrerURL(request.custom_referrer_url);
 
-  if (request.headers.has_value())
-    pp_request.SetHeaders(request.headers.value());
+  if (!request.headers.empty())
+    pp_request.SetHeaders(request.headers);
 
   if (!request.body.empty())
     pp_request.AppendDataToBody(request.body.data(), request.body.size());
@@ -324,7 +363,7 @@ void PepperUrlLoader::DidOpen(ResultCallback callback, int32_t result) {
   if (headers_var.is_string()) {
     mutable_response().headers = headers_var.AsString();
   } else {
-    mutable_response().headers.reset();
+    mutable_response().headers.clear();
   }
 
   std::move(callback).Run(result);

pdf/ppapi_migration/url_loader.h

@@ -14,7 +14,6 @@
 #include "base/containers/circular_deque.h"
 #include "base/containers/span.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "pdf/ppapi_migration/callback.h"
 #include "ppapi/cpp/instance_handle.h"
 #include "ppapi/cpp/url_loader.h"
@@ -47,10 +46,10 @@ struct UrlRequest final {
   bool ignore_redirects = false;
 
   // Custom referrer URL.
-  base::Optional<std::string> custom_referrer_url;
+  std::string custom_referrer_url;
 
   // HTTP headers as a single string of `\n`-delimited key-value pairs.
-  base::Optional<std::string> headers;
+  std::string headers;
 
   // Request body.
   std::string body;
@@ -69,7 +68,7 @@ struct UrlResponse final {
   int32_t status_code = 0;
 
   // HTTP headers as a single string of `\n`-delimited key-value pairs.
-  base::Optional<std::string> headers;
+  std::string headers;
 };
 
 // Abstraction for a Blink or Pepper URL loader.

pdf/ppapi_migration/url_loader_unittest.cc

@@ -4,14 +4,20 @@
 
 #include "pdf/ppapi_migration/url_loader.h"
 
+#include <stddef.h>
+
 #include <memory>
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/containers/span.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/test/mock_callback.h"
 #include "net/base/net_errors.h"
 #include "pdf/ppapi_migration/callback.h"
@@ -19,6 +25,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-shared.h"
+#include "third_party/blink/public/platform/web_http_header_visitor.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_url.h"
 #include "third_party/blink/public/platform/web_url_error.h"
@@ -39,9 +46,24 @@ using ::testing::Invoke;
 using ::testing::NiceMock;
 using ::testing::ReturnNull;
 using ::testing::SaveArg;
+using ::testing::UnorderedElementsAreArray;
 
 constexpr base::span<const char> kFakeData = "fake data";
 
+size_t GetRequestHeaderCount(const blink::WebURLRequest& request) {
+  struct : public blink::WebHTTPHeaderVisitor {
+    void VisitHeader(const blink::WebString& name,
+                     const blink::WebString& value) override {
+      ++count;
+    }
+
+    size_t count = 0;
+  } counting_header_visitor;
+
+  request.VisitHttpHeaderFields(&counting_header_visitor);
+  return counting_header_visitor.count;
+}
+
 blink::WebURLError MakeWebURLError(int reason) {
   return blink::WebURLError(reason, GURL());
 }
@@ -143,6 +165,7 @@ TEST_F(BlinkUrlLoaderTest, Open) {
   EXPECT_FALSE(saved_options_.grant_universal_access);
   EXPECT_EQ(GURL("http://example.com/fake.pdf"), GURL(saved_request_.Url()));
   EXPECT_EQ("FAKE", saved_request_.HttpMethod().Ascii());
+  EXPECT_EQ(0u, GetRequestHeaderCount(saved_request_));
   EXPECT_EQ(blink::mojom::RequestContextType::PLUGIN,
             saved_request_.GetRequestContext());
   EXPECT_EQ(network::mojom::RequestDestination::kEmbed,
@@ -167,6 +190,24 @@ TEST_F(BlinkUrlLoaderTest, OpenWithFailingCreateAssociatedURLLoader) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
 }
 
+TEST_F(BlinkUrlLoaderTest, OpenWithHeaders) {
+  UrlRequest request;
+  request.headers = base::JoinString(
+      {
+          "Content-Length: 123",
+          "Content-Type: application/pdf",
+          "Non-ASCII-Value: 🙃",
+      },
+      "\n");
+  loader_->Open(request, mock_callback_.Get());
+
+  EXPECT_EQ(3u, GetRequestHeaderCount(saved_request_));
+  EXPECT_EQ("123", saved_request_.HttpHeaderField("Content-Length").Utf8());
+  EXPECT_EQ("application/pdf",
+            saved_request_.HttpHeaderField("Content-Type").Utf8());
+  EXPECT_EQ("🙃", saved_request_.HttpHeaderField("Non-ASCII-Value").Utf8());
+}
+
 TEST_F(BlinkUrlLoaderTest, DidReceiveResponse) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   EXPECT_CALL(mock_callback_, Run(PP_OK));
@@ -176,6 +217,26 @@ TEST_F(BlinkUrlLoaderTest, DidReceiveResponse) {
   loader_->DidReceiveResponse(response);
 
   EXPECT_EQ(204, loader_->response().status_code);
+  EXPECT_EQ("", loader_->response().headers);
+}
+
+TEST_F(BlinkUrlLoaderTest, DidReceiveResponseWithHeaders) {
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+
+  blink::WebURLResponse response;
+  response.AddHttpHeaderField("Content-Length", "123");
+  response.AddHttpHeaderField("Content-Type", "application/pdf");
+  response.AddHttpHeaderField("Non-ASCII-Value", "🙃");
+  loader_->DidReceiveResponse(response);
+
+  std::vector<std::string> split_headers =
+      base::SplitString(loader_->response().headers, "\n",
+                        base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+  EXPECT_THAT(split_headers, UnorderedElementsAreArray({
+                                 "Content-Length: 123",
+                                 "Content-Type: application/pdf",
+                                 "Non-ASCII-Value: 🙃",
+                             }));
 }
 
 TEST_F(BlinkUrlLoaderTest, DidReceiveData) {

pdf/url_loader_wrapper_impl.cc

@@ -294,7 +294,7 @@ void URLLoaderWrapperImpl::DidRead(ResultCallback callback, int32_t result) {
 }
 
 void URLLoaderWrapperImpl::SetHeadersFromLoader() {
-  SetResponseHeaders(url_loader_->response().headers.value_or(""));
+  SetResponseHeaders(url_loader_->response().headers);
 }
 
 }  // namespace chrome_pdf