[Autofill Assistant] Allow lenient placeholder replacement.

Bug: b/145043394 Change-Id: Ieb85689efec602ea4392c51d289f5bbdefd43d25 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2270213 Commit-Queue: Clemens Arbesser <[email protected]> Reviewed-by: Sandro Maggi <[email protected]> Cr-Commit-Position: refs/heads/master@{#785377}
blueboxd · Jul 6, 2020 · 5358d4d · 5358d4d
1 parent 257d56b
commit 5358d4d
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 18 deletions.
diff --git a/components/autofill_assistant/browser/field_formatter.cc b/components/autofill_assistant/browser/field_formatter.cc
@@ -16,8 +16,9 @@
 
 namespace {
 // Regex to find placeholders of the form ${key}, where key is an arbitrary
-// string that does not contain curly braces.
-const char kPlaceholderExtractor[] = R"re(\$\{([^{}]+)\})re";
+// string that does not contain curly braces. The first capture group is for
+// the prefix before the key, the second for the key itself.
+const char kPlaceholderExtractor[] = R"re(([^$]*)\$\{([^{}]+)\})re";
 
 base::Optional<std::string> GetFieldValue(
     const std::map<std::string, std::string>& mappings,
@@ -50,24 +51,32 @@ namespace field_formatter {
 
 base::Optional<std::string> FormatString(
     const std::string& pattern,
-    const std::map<std::string, std::string>& mappings) {
+    const std::map<std::string, std::string>& mappings,
+    bool strict) {
   if (pattern.empty()) {
     return std::string();
   }
 
-  std::string key;
-  std::string out = pattern;
+  std::string out;
   re2::StringPiece input(pattern);
-  while (re2::RE2::FindAndConsume(&input, kPlaceholderExtractor, &key)) {
+  std::string prefix;
+  std::string key;
+  while (
+      re2::RE2::FindAndConsume(&input, kPlaceholderExtractor, &prefix, &key)) {
     auto rewrite_value = GetFieldValue(mappings, key);
     if (!rewrite_value.has_value()) {
-      VLOG(2) << "No value for " << key << " in " << pattern;
-      return base::nullopt;
+      if (strict) {
+        VLOG(2) << "No value for " << key << " in " << pattern;
+        return base::nullopt;
+      }
+      // Leave placeholder unchanged.
+      rewrite_value = "${" + key + "}";
     }
 
-    re2::RE2::Replace(&out, kPlaceholderExtractor,
-                      re2::StringPiece(rewrite_value.value()));
+    out = out + prefix + *rewrite_value;
   }
+  // Append remaining unmatched suffix (if any).
+  out = out + input.ToString();
 
   return out;
 }

diff --git a/components/autofill_assistant/browser/field_formatter.h b/components/autofill_assistant/browser/field_formatter.h
@@ -16,11 +16,13 @@ namespace field_formatter {
 
 // Replaces all placeholder occurrences of the form ${key} in |input| with the
 // corresponding value in |mappings|, where |key| is an arbitrary string that
-// does not contain curly braces. Fails if any of the found placeholders is not
-// in |mappings|.
+// does not contain curly braces. If |strict| is true, this will fail if any of
+// the found placeholders is not in |mappings|. Otherwise, placeholders other
+// than those from |mappings| will be left unchanged.
 base::Optional<std::string> FormatString(
     const std::string& input,
-    const std::map<std::string, std::string>& mappings);
+    const std::map<std::string, std::string>& mappings,
+    bool strict = true);
 
 // Creates a lookup map for all non-empty autofill and custom
 // AutofillFormatProto::AutofillAssistantCustomField field types in

diff --git a/components/autofill_assistant/browser/field_formatter_unittest.cc b/components/autofill_assistant/browser/field_formatter_unittest.cc
@@ -25,14 +25,20 @@ TEST(FieldFormatterTest, FormatString) {
   std::map<std::string, std::string> mappings = {
       {"keyA", "valueA"}, {"keyB", "valueB"}, {"keyC", "valueC"}};
 
-  EXPECT_EQ(*FormatString("", mappings), "");
-  EXPECT_EQ(*FormatString("input", mappings), "input");
-  EXPECT_EQ(*FormatString("prefix ${keyA}", mappings), "prefix valueA");
-  EXPECT_EQ(*FormatString("prefix ${keyA}${keyB}${keyC} suffix", mappings),
+  EXPECT_EQ(FormatString("", mappings), "");
+  EXPECT_EQ(FormatString("input", mappings), "input");
+  EXPECT_EQ(FormatString("prefix ${keyA}", mappings), "prefix valueA");
+  EXPECT_EQ(FormatString("prefix ${keyA}${keyB}${keyC} suffix", mappings),
             "prefix valueAvalueBvalueC suffix");
-  EXPECT_EQ(*FormatString("keyA = ${keyA}", mappings), "keyA = valueA");
+  EXPECT_EQ(FormatString("keyA = ${keyA}", mappings), "keyA = valueA");
   EXPECT_EQ(FormatString("${keyD}", mappings), base::nullopt);
   EXPECT_EQ(FormatString("${keyA}${keyD}", mappings), base::nullopt);
+
+  EXPECT_EQ(FormatString("${keyD}", mappings, /*strict = */ false), "${keyD}");
+  EXPECT_EQ(FormatString("${keyA}${keyD}", mappings, /*strict = */ false),
+            "valueA${keyD}");
+  EXPECT_EQ(FormatString("${keyD}${keyA}", mappings, /*strict = */ false),
+            "${keyD}valueA");
 }
 
 TEST(FieldFormatterTest, AutofillProfile) {