Added handling of large requests to Autofill API.

Increased the limit of Query GET requests, added a POST Query fallback, and
added UMA metrics to keep track of URL length and associated events.

This change might be needed when metadata is enabled.

Bug: 948862
Change-Id: If0934a685758ab8999ceea6ce22a27499e4f772f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1549615
Commit-Queue: Vincent Boisselle <vincb@google.com>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656080}

components/autofill/core/browser/BUILD.gn

@@ -629,6 +629,7 @@ source_set("unit_tests") {
     "//third_party/libaddressinput:test_support",
     "//third_party/libaddressinput:util",
     "//third_party/libphonenumber",
+    "//third_party/re2:re2",
     "//ui/base",
     "//url",
   ]

components/autofill/core/browser/autofill_download_manager.cc

@@ -60,7 +60,7 @@ constexpr std::pair<int, int> kAutofillExperimentRanges[] = {
     {3314445, 3314448}, {3314854, 3314883},
 };
 
-const size_t kMaxQueryGetSize = 1400;  // 1.25KB
+const size_t kMaxQueryGetSize = 1400;  // 1.25 KiB
 const size_t kAutofillDownloadManagerMaxFormCacheSize = 16;
 const size_t kMaxFieldsPerQueryRequest = 100;
 
@@ -448,16 +448,22 @@ bool GetUploadPayloadForApi(const AutofillUploadContents& upload,
   return upload_request.SerializeToString(payload);
 }
 
-// Gets an API method URL given its type (query or upload) and an optional
-// resource ID.
+// Gets an API method URL given its type (query or upload), an optional
+// resource ID, and the HTTP method to be used.
 // Example usage:
-//   * GetAPIMethodUrl(REQUEST_QUERY, "1234") will return "/v1/pages/1234".
-//   * GetAPIMethodUrl(REQUEST_UPLOAD, "") will return "/v1/forms:vote".
+// * GetAPIMethodUrl(REQUEST_QUERY, "1234", "GET") will return "/v1/pages/1234".
+// * GetAPIMethodUrl(REQUEST_QUERY, "1234", "POST") will return "/v1/pages:get".
+// * GetAPIMethodUrl(REQUEST_UPLOAD, "", "POST") will return "/v1/forms:vote".
 std::string GetAPIMethodUrl(AutofillDownloadManager::RequestType type,
-                            base::StringPiece resource_id) {
+                            base::StringPiece resource_id,
+                            base::StringPiece method) {
   const char* api_method_url;
   if (type == AutofillDownloadManager::REQUEST_QUERY) {
+    if (method == "POST") {
+      api_method_url = "/v1/pages:get";
+    } else {
       api_method_url = "/v1/pages";
+    }
   } else if (type == AutofillDownloadManager::REQUEST_UPLOAD) {
     api_method_url = "/v1/forms:vote";
   } else {
@@ -471,6 +477,35 @@ std::string GetAPIMethodUrl(AutofillDownloadManager::RequestType type,
   return base::StrCat({api_method_url, "/", resource_id});
 }
 
+// Gets HTTP body payload for API POST request.
+std::string GetAPIBodyPayload(const std::string& payload,
+                              AutofillDownloadManager::RequestType type) {
+  // Don't do anything for payloads not related to Query.
+  if (type != AutofillDownloadManager::REQUEST_QUERY) {
+    return payload;
+  }
+  // Wrap query payload in a request proto to interface with API Query method.
+  AutofillPageResourceQueryRequest request;
+  request.set_serialized_request(payload);
+  std::string new_payload;
+  DCHECK(request.SerializeToString(&new_payload))
+      << "could not serialize AutofillPageResourceQueryRequest payload";
+  return new_payload;
+}
+
+// Gets the data payload for API Query (POST and GET).
+bool GetAPIQueryPayload(const AutofillQueryContents& query,
+                        std::string* payload) {
+  std::string serialized_query;
+  if (!CreateApiRequestFromLegacyRequest(query).SerializeToString(
+          &serialized_query)) {
+    return false;
+  }
+  base::Base64UrlEncode(serialized_query,
+                        base::Base64UrlEncodePolicy::INCLUDE_PADDING, payload);
+  return true;
+}
+
 }  // namespace
 
 struct AutofillDownloadManager::FormRequestData {
@@ -542,9 +577,7 @@ bool AutofillDownloadManager::StartQueryRequest(
 
   // Get the query request payload.
   std::string payload;
-  bool is_payload_serialized =
-      UseApi()
-          ? CreateApiRequestFromLegacyRequest(query).SerializeToString(&payload)
+  bool is_payload_serialized = UseApi() ? GetAPIQueryPayload(query, &payload)
                                         : query.SerializeToString(&payload);
   if (!is_payload_serialized) {
     return false;
@@ -674,31 +707,27 @@ AutofillDownloadManager::GetRequestURLAndMethodForApi(
   // ID of the resource to add to the API request URL. Nothing will be added if
   // |resource_id| is empty.
   std::string resource_id;
+  std::string method = "POST";
 
-  // Get the resource id of corresponding webpage when doing a query request.
   if (request_data.request_type == AutofillDownloadManager::REQUEST_QUERY) {
-    if (request_data.payload.length() <= kMaxQueryGetSize) {
-      base::Base64UrlEncode(request_data.payload,
-                            base::Base64UrlEncodePolicy::INCLUDE_PADDING,
-                            &resource_id);
+    if (request_data.payload.length() <= kMaxAPIQueryGetSize) {
+      resource_id = request_data.payload;
+      method = "GET";
+      UMA_HISTOGRAM_BOOLEAN("Autofill.Query.ApiUrlIsTooLong", false);
+    } else {
+      UMA_HISTOGRAM_BOOLEAN("Autofill.Query.ApiUrlIsTooLong", true);
     }
-    // Query method is always GET (represented by 0) with API.
-    UMA_HISTOGRAM_BOOLEAN("Autofill.Query.Method", 0);
+    UMA_HISTOGRAM_BOOLEAN("Autofill.Query.Method", (method == "GET") ? 0 : 1);
   }
 
   // Make the canonical URL to query the API, e.g.,
   // https://autofill.googleapis.com/v1/forms/1234?alt=proto.
   GURL url = autofill_server_url_.Resolve(
-      GetAPIMethodUrl(request_data.request_type, resource_id));
+      GetAPIMethodUrl(request_data.request_type, resource_id, method));
 
   // Add the query parameter to set the response format to a serialized proto.
   url = net::AppendQueryParameter(url, "alt", "proto");
 
-  // Determine the HTTP method that should be used.
-  std::string method =
-      (request_data.request_type == AutofillDownloadManager::REQUEST_QUERY)
-          ? "GET"
-          : "POST";
   return std::make_tuple(std::move(url), std::move(method));
 }
 
@@ -714,6 +743,14 @@ bool AutofillDownloadManager::StartRequest(FormRequestData request_data) {
       UseApi() ? GetRequestURLAndMethodForApi(request_data)
                : GetRequestURLAndMethod(request_data);
 
+  // Track the URL length for GET queries because the URL length can be in the
+  // thousands when rich metadata is enabled.
+  if (request_data.request_type == AutofillDownloadManager::REQUEST_QUERY &&
+      method == "GET") {
+    UMA_HISTOGRAM_COUNTS_100000("Autofill.Query.GetUrlLength",
+                                request_url.spec().length());
+  }
+
   auto resource_request = std::make_unique<network::ResourceRequest>();
   resource_request->url = request_url;
   resource_request->load_flags =
@@ -750,10 +787,14 @@ bool AutofillDownloadManager::StartRequest(FormRequestData request_data) {
   simple_loader->SetAllowHttpErrorResults(true);
 
   if (method == "POST") {
-    const std::string content_type =
+    const std::string& content_type =
         UseApi() ? "application/x-protobuf" : "text/proto";
+    const std::string& payload =
+        UseApi()
+            ? GetAPIBodyPayload(request_data.payload, request_data.request_type)
+            : request_data.payload;
     // Attach payload data and add data format header.
-    simple_loader->AttachStringForUpload(request_data.payload, content_type);
+    simple_loader->AttachStringForUpload(payload, content_type);
   }
 
   // Transfer ownership of the loader into url_loaders_. Temporarily hang

components/autofill/core/browser/autofill_download_manager.h

@@ -31,6 +31,8 @@ namespace autofill {
 class AutofillDriver;
 class FormStructure;
 
+const size_t kMaxAPIQueryGetSize = 10240;  // 10 KiB
+
 // A helper to make sure that tests which modify the set of active autofill
 // experiments do not interfere with one another.
 struct ScopedActiveAutofillExperiments {

components/autofill/core/browser/autofill_download_manager_unittest.cc

@@ -52,6 +52,7 @@
 #include "services/network/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/re2/src/re2/re2.h"
 #include "url/third_party/mozilla/url_parse.h"
 
 using base::UTF8ToUTF16;
@@ -117,6 +118,35 @@ bool GetUploadRequestProtoFromRequest(
   return true;
 }
 
+bool GetAutofillPageResourceQueryRequestFromRequest(
+    network::TestURLLoaderFactory::PendingRequest* loader_request,
+    AutofillPageResourceQueryRequest* query_request) {
+  if (loader_request->request.request_body == nullptr) {
+    return false;
+  }
+
+  std::string request_body_content = GetStringFromDataElements(
+      loader_request->request.request_body->elements());
+  if (!query_request->ParseFromString(request_body_content)) {
+    return false;
+  }
+  return true;
+}
+
+bool DeserializeAutofillPageQueryRequest(base::StringPiece serialized_content,
+                                         AutofillPageQueryRequest* request) {
+  std::string decoded_content;
+  if (!base::Base64UrlDecode(serialized_content,
+                             base::Base64UrlDecodePolicy::REQUIRE_PADDING,
+                             &decoded_content)) {
+    return false;
+  }
+  if (!request->ParseFromString(decoded_content)) {
+    return false;
+  }
+  return true;
+}
+
 }  // namespace
 
 // This tests AutofillDownloadManager. AutofillDownloadManagerTest implements
@@ -499,22 +529,150 @@ TEST_F(AutofillDownloadManagerTest, QueryAPITest) {
   // Inspect the request that the test URL loader sent.
   network::TestURLLoaderFactory::PendingRequest* request =
       test_url_loader_factory_.GetPendingRequest(0);
+
+  // Verify request URL and the data payload it carries.
+  {
     // This is the URL we expect to query the API. The sub-path right after
-  // "/page" corresponds to the serialized AutofillPageQueryRequest proto (that
-  // we filled forms in) encoded in base64. The Autofill
+    // "/page" corresponds to the serialized AutofillPageQueryRequest proto
+    // (that we filled forms in) encoded in base64. The Autofill
     // https://clients1.google.com/ domain URL corresponds to the default domain
-  // used by the download manager.
+    // used by the download manager, which is invalid, but good for testing.
+    const std::string expected_url =
+        R"(https://clients1.google.com/v1/pages/(.+)\?alt=proto)";
+    std::string encoded_request;
+    ASSERT_TRUE(re2::RE2::FullMatch(request->request.url.spec(), expected_url,
+                                    &encoded_request));
+    AutofillPageQueryRequest request_content;
+    ASSERT_TRUE(
+        DeserializeAutofillPageQueryRequest(encoded_request, &request_content));
+    // Verify form content.
+    ASSERT_EQ(request_content.forms().size(), 1);
+    EXPECT_EQ(request_content.forms(0).signature(),
+              form_structures[0]->form_signature());
+    // Verify field content.
+    ASSERT_EQ(request_content.forms(0).fields().size(), 2);
+    EXPECT_EQ(request_content.forms(0).fields(0).signature(),
+              form_structures[0]->field(0)->GetFieldSignature());
+    EXPECT_EQ(request_content.forms(0).fields(1).signature(),
+              form_structures[0]->field(1)->GetFieldSignature());
+  }
+
+  // Verify API key header.
+  {
+    std::string header_value;
+    EXPECT_TRUE(
+        request->request.headers.GetHeader("X-Goog-Api-Key", &header_value));
+    EXPECT_EQ(header_value, "dummykey");
+  }
+  // Verify binary response header.
+  {
+    std::string header_value;
+    ASSERT_TRUE(request->request.headers.GetHeader(
+        "X-Goog-Encode-Response-If-Executable", &header_value));
+    EXPECT_EQ(header_value, "base64");
+  }
+
+  // Verify response.
+  test_url_loader_factory_.SimulateResponseWithoutRemovingFromPendingList(
+      request, "dummy response");
+  // Upon reception of a suggestions query, we expect OnLoadedServerPredictions
+  // to be called back from the observer and some histograms be incremented.
+  EXPECT_EQ(1U, responses_.size());
+  EXPECT_EQ(responses_.front().type_of_response,
+            AutofillDownloadManagerTest::QUERY_SUCCESSFULL);
+  histogram.ExpectBucketCount("Autofill.Query.WasInCache", CACHE_MISS, 1);
+  histogram.ExpectBucketCount("Autofill.Query.HttpResponseOrErrorCode",
+                              net::HTTP_OK, 1);
+}
+
+TEST_F(AutofillDownloadManagerTest, QueryAPITestWhenTooLongUrl) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitWithFeatures(
+      // Enabled
+      // We want to query the API rather than the legacy server.
+      {features::kAutofillUseApi},
+      // Disabled
+      {});
+
+  // Build the form structures that we want to query.
+  FormData form;
+  FormFieldData field;
+  // Fill a really long field that will bust the request URL size limit of 10
+  // KiB. This is not a lot of memory, hence this should not cause problems for
+  // machines running this test. This will force the fallback to POST.
+  field.name_attribute = base::string16(kMaxAPIQueryGetSize, 'a');
+  field.form_control_type = "text";
+  form.fields.push_back(field);
+
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  {
+    auto form_structure = std::make_unique<FormStructure>(form);
+    form_structure->set_is_rich_query_enabled(true);
+    form_structures.push_back(std::move(form_structure));
+  }
+
+  AutofillDownloadManager download_manager(&driver_, this, "dummykey");
+
+  // Start the query request and look if it is successful. No response was
+  // received yet.
+  base::HistogramTester histogram;
+  EXPECT_TRUE(
+      download_manager.StartQueryRequest(ToRawPointerVector(form_structures)));
+
+  // Verify request.
+  // Verify if histograms are right.
+  histogram.ExpectUniqueSample("Autofill.ServerQueryResponse",
+                               AutofillMetrics::QUERY_SENT, 1);
+  // Verify that the logged method is POST.
+  histogram.ExpectUniqueSample("Autofill.Query.Method", METHOD_POST, 1);
+
+  // Get the latest request that the test URL loader sent.
+  network::TestURLLoaderFactory::PendingRequest* request =
+      test_url_loader_factory_.GetPendingRequest(0);
+  // Verify that the POST URL is used when request data too large.
   const std::string expected_url = {
-      "https://clients1.google.com/v1/pages/"
-      "Chc2LjEuMTcxNS4xNDQyL2VuIChHR0xMKRIlCU9O84MyjH9NEgsNeu"
-      "FP4BIAGgAiABILDZxOStASABoAIgAaAA==?"
-      "alt=proto"};
+      "https://clients1.google.com/v1/pages:get?alt=proto"};
+  // Verify API key header.
   EXPECT_EQ(request->request.url, expected_url);
-  std::string api_key_header_value;
-  EXPECT_TRUE(request->request.headers.GetHeader("X-Goog-Api-Key",
-                                                 &api_key_header_value));
-  EXPECT_EQ(api_key_header_value, "dummykey");
+  {
+    std::string header_value;
+    EXPECT_TRUE(
+        request->request.headers.GetHeader("X-Goog-Api-Key", &header_value));
+    EXPECT_EQ(header_value, "dummykey");
+  }
+  // Verify Content-Type header.
+  {
+    std::string header_value;
+    ASSERT_TRUE(
+        request->request.headers.GetHeader("Content-Type", &header_value));
+    EXPECT_EQ(header_value, "application/x-protobuf");
+  }
+  // Verify binary response header.
+  {
+    std::string header_value;
+    ASSERT_TRUE(request->request.headers.GetHeader(
+        "X-Goog-Encode-Response-If-Executable", &header_value));
+    EXPECT_EQ(header_value, "base64");
+  }
+  // Verify content of the POST body data.
+  {
+    AutofillPageResourceQueryRequest query_request;
+    ASSERT_TRUE(GetAutofillPageResourceQueryRequestFromRequest(request,
+                                                               &query_request));
+    AutofillPageQueryRequest request_content;
+    ASSERT_TRUE(DeserializeAutofillPageQueryRequest(
+        query_request.serialized_request(), &request_content));
+    // Verify form content.
+    ASSERT_EQ(request_content.forms().size(), 1);
+    EXPECT_EQ(request_content.forms(0).signature(),
+              form_structures[0]->form_signature());
+    // Verify field content.
+    ASSERT_EQ(request_content.forms(0).fields().size(), 1);
+    EXPECT_EQ(request_content.forms(0).fields(0).signature(),
+              form_structures[0]->field(0)->GetFieldSignature());
+  }
 
+  // Verify response.
   test_url_loader_factory_.SimulateResponseWithoutRemovingFromPendingList(
       request, "dummy response");
   // Upon reception of a suggestions query, we expect OnLoadedServerPredictions