Update Structured Headers byte sequence syntax

Byte sequences, previously delimited by asterisks, have changed in
Structured Headers draft 15 to use ':' as the delimiter. This CL updates
the implementation to be in line with the latest draft, while keeping
backwards compatiblity with draft 9 for existing uses.

Bug: 1048755
Change-Id: I1e8684a016085573ab6815bb078c8f7f3a41870e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2073537
Commit-Queue: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746135}

net/http/structured_headers.cc

@@ -143,14 +143,21 @@ class StructuredHeaderParser {
       case '"':
         return ReadString();
       case '*':
-        return ReadByteSequence();
+        if (version_ == kDraft09)
+          return ReadByteSequence();
+        return ReadToken();
+      case ':':
+        if (version_ == kDraft15)
+          return ReadByteSequence();
+        return base::nullopt;
       case '?':
         return ReadBoolean();
       default:
         if (input_.front() == '-' || base::IsAsciiDigit(input_.front()))
           return ReadNumber();
-        else
+        if (base::IsAsciiAlpha(input_.front()))
           return ReadToken();
+        return base::nullopt;
     }
   }
 
@@ -416,13 +423,14 @@ class StructuredHeaderParser {
 
   // Parses a Byte Sequence ([SH09] 4.2.11, [SH15] 4.2.7).
   base::Optional<Item> ReadByteSequence() {
-    if (!ConsumeChar('*')) {
-      LogParseError("ReadByteSequence", "'*'");
+    char delimiter = (version_ == kDraft09 ? '*' : ':');
+    if (!ConsumeChar(delimiter)) {
+      LogParseError("ReadByteSequence", "delimiter");
       return base::nullopt;
     }
-    size_t len = input_.find('*');
+    size_t len = input_.find(delimiter);
     if (len == base::StringPiece::npos) {
-      DVLOG(1) << "ReadByteSequence: missing closing '*'";
+      DVLOG(1) << "ReadByteSequence: missing closing delimiter";
       return base::nullopt;
     }
     std::string base64(input_.substr(0, len));
@@ -435,7 +443,7 @@ class StructuredHeaderParser {
       return base::nullopt;
     }
     input_.remove_prefix(len);
-    ConsumeChar('*');
+    ConsumeChar(delimiter);
     return Item(std::move(binary), Item::kByteSequenceType);
   }
 
@@ -470,8 +478,8 @@ class StructuredHeaderParser {
 
   void LogParseError(const char* func, const char* expected) {
     DVLOG(1) << func << ": " << expected << " expected, got "
-             << (input_.empty() ? "'" + input_.substr(0, 1).as_string() + "'"
-                                : "EOS");
+             << (input_.empty() ? "EOS"
+                                : "'" + input_.substr(0, 1).as_string() + "'");
   }
 
   base::StringPiece input_;
@@ -540,10 +548,10 @@ class StructuredHeaderSerializer {
     }
     if (value.is_byte_sequence()) {
       // Serializes a Byte Sequence ([SH15] 4.1.8).
-      output_ << "*";
+      output_ << ":";
       output_ << base::Base64Encode(
           base::as_bytes(base::make_span(value.GetString())));
-      output_ << "*";
+      output_ << ":";
       return true;
     }
     if (value.is_integer()) {

net/http/structured_headers_unittest.cc

@@ -362,18 +362,18 @@ const struct ItemTestCase {
     {"spelled-out True boolean", "?True", base::nullopt},
     {"spelled-out False boolean", "?False", base::nullopt},
     // Byte Sequence
-    {"basic binary", "*aGVsbG8=*", Item("hello", Item::kByteSequenceType)},
-    {"empty binary", "**", Item("", Item::kByteSequenceType)},
-    {"bad paddding", "*aGVsbG8*", Item("hello", Item::kByteSequenceType),
-     "*aGVsbG8=*"},
-    {"bad end delimiter", "*aGVsbG8=", base::nullopt},
-    {"extra whitespace", "*aGVsb G8=*", base::nullopt},
-    {"extra chars", "*aGVsbG!8=*", base::nullopt},
-    {"suffix chars", "*aGVsbG8=!*", base::nullopt},
-    {"non-zero pad bits", "*iZ==*", Item("\x89", Item::kByteSequenceType),
-     "*iQ==*"},
-    {"non-ASCII binary", "*/+Ah*", Item("\xFF\xE0!", Item::kByteSequenceType)},
-    {"base64url binary", "*_-Ah*", base::nullopt},
+    {"basic binary", ":aGVsbG8=:", Item("hello", Item::kByteSequenceType)},
+    {"empty binary", "::", Item("", Item::kByteSequenceType)},
+    {"bad paddding", ":aGVsbG8:", Item("hello", Item::kByteSequenceType),
+     ":aGVsbG8=:"},
+    {"bad end delimiter", ":aGVsbG8=", base::nullopt},
+    {"extra whitespace", ":aGVsb G8=:", base::nullopt},
+    {"extra chars", ":aGVsbG!8=:", base::nullopt},
+    {"suffix chars", ":aGVsbG8=!:", base::nullopt},
+    {"non-zero pad bits", ":iZ==:", Item("\x89", Item::kByteSequenceType),
+     ":iQ==:"},
+    {"non-ASCII binary", ":/+Ah:", Item("\xFF\xE0!", Item::kByteSequenceType)},
+    {"base64url binary", ":_-Ah:", base::nullopt},
     // String
     {"basic string", "\"foo\"", Item("foo")},
     {"empty string", "\"\"", Item("")},
@@ -407,6 +407,28 @@ const struct ItemTestCase {
     {"c-style unicode escape in string", "\"\\u0061\"", base::nullopt},
 };
 
+const ItemTestCase sh09_item_test_cases[] = {
+    // Integer
+    {"large integer", "9223372036854775807", Integer(9223372036854775807L)},
+    {"large negative integer", "-9223372036854775807",
+     Integer(-9223372036854775807L)},
+    {"too large integer", "9223372036854775808", base::nullopt},
+    {"too large negative integer", "-9223372036854775808", base::nullopt},
+    // Byte Sequence
+    {"basic binary", "*aGVsbG8=*", Item("hello", Item::kByteSequenceType)},
+    {"empty binary", "**", Item("", Item::kByteSequenceType)},
+    {"bad paddding", "*aGVsbG8*", Item("hello", Item::kByteSequenceType),
+     "*aGVsbG8=*"},
+    {"bad end delimiter", "*aGVsbG8=", base::nullopt},
+    {"extra whitespace", "*aGVsb G8=*", base::nullopt},
+    {"extra chars", "*aGVsbG!8=*", base::nullopt},
+    {"suffix chars", "*aGVsbG8=!*", base::nullopt},
+    {"non-zero pad bits", "*iZ==*", Item("\x89", Item::kByteSequenceType),
+     "*iQ==*"},
+    {"non-ASCII binary", "*/+Ah*", Item("\xFF\xE0!", Item::kByteSequenceType)},
+    {"base64url binary", "*_-Ah*", base::nullopt},
+};
+
 // For Structured Headers Draft 15
 const struct ParameterizedItemTestCase {
   const char* name;
@@ -546,21 +568,23 @@ TEST(StructuredHeaderTest, ParseItem) {
   }
 }
 
-// In Structured Headers Draft 9, integers can be larger than 1e15. This
-// behaviour is exposed in the parser for SH09-specific lists, so test it
-// through that interface.
-TEST(StructuredHeaderTest, SH09LargeInteger) {
-  base::Optional<ListOfLists> result =
-      ParseListOfLists("9223372036854775807;-9223372036854775807");
-  ASSERT_TRUE(result.has_value());
-  EXPECT_EQ(*result, (ListOfLists{{Integer(9223372036854775807L),
-                                   Integer(-9223372036854775807L)}}));
-
-  result = ParseListOfLists("9223372036854775808");
-  EXPECT_FALSE(result.has_value());
-
-  result = ParseListOfLists("-9223372036854775808");
-  EXPECT_FALSE(result.has_value());
+// Structured Headers Draft 9 parsing rules are different than Draft 15, and
+// some strings which are considered invalid in SH15 should parse in SH09.
+// The SH09 Item parser is not directly exposed, but can be used indirectly by
+// calling the parser for SH09-specific lists.
+TEST(StructuredHeaderTest, ParseSH09Item) {
+  for (const auto& c : sh09_item_test_cases) {
+    SCOPED_TRACE(c.name);
+    base::Optional<ListOfLists> result = ParseListOfLists(c.raw);
+    if (c.expected.has_value()) {
+      EXPECT_TRUE(result.has_value());
+      EXPECT_EQ(result->size(), 1UL);
+      EXPECT_EQ((*result)[0].size(), 1UL);
+      EXPECT_EQ((*result)[0][0], c.expected);
+    } else {
+      EXPECT_FALSE(result.has_value());
+    }
+  }
 }
 
 // In Structured Headers Draft 9, floats can have more than three fractional