Allow CBOR maps to use integer keys

Currently, CBOR maps only permit keys to be UTF-8 strings. Add support
for unsigned integer keys. Support for negative integer type keys will
be integrated as CL 777807 is lands.

Bug: 786218
Change-Id: Id28864a9c62d0b0d93e71a663b6e8b2b84b2de95
Reviewed-on: https://chromium-review.googlesource.com/781207
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524223}

content/browser/webauth/attestation_object.cc

@@ -25,9 +25,12 @@ AttestationObject::AttestationObject(
 
 std::vector<uint8_t> AttestationObject::SerializeToCBOREncodedBytes() {
   CBORValue::MapValue map;
-  map[kFormatKey] = CBORValue(attestation_statement_->format_name().c_str());
-  map[kAuthDataKey] = CBORValue(authenticator_data_->SerializeToByteArray());
-  map[kAttestationKey] = CBORValue(attestation_statement_->GetAsCBORMap());
+  map[CBORValue(kFormatKey)] =
+      CBORValue(attestation_statement_->format_name().c_str());
+  map[CBORValue(kAuthDataKey)] =
+      CBORValue(authenticator_data_->SerializeToByteArray());
+  map[CBORValue(kAttestationKey)] =
+      CBORValue(attestation_statement_->GetAsCBORMap());
   auto cbor = CBORWriter::Write(CBORValue(map));
   if (cbor.has_value()) {
     return cbor.value();

content/browser/webauth/cbor/cbor_reader.cc

@@ -182,18 +182,19 @@ base::Optional<CBORValue> CBORReader::ReadCBORMap(uint64_t length,
     base::Optional<CBORValue> value = DecodeCBOR(max_nesting_level - 1);
     if (!key.has_value() || !value.has_value())
       return base::nullopt;
-    if (key.value().type() != CBORValue::Type::STRING) {
+
+    // Only CBOR maps with integer or string type keys are allowed.
+    if (key.value().type() != CBORValue::Type::STRING &&
+        key.value().type() != CBORValue::Type::UNSIGNED) {
       error_code_ = DecoderError::INCORRECT_MAP_KEY_TYPE;
       return base::nullopt;
     }
-
-    if (!CheckDuplicateKey(key.value().GetString(), &cbor_map) ||
-        !CheckOutOfOrderKey(key.value().GetString(), &cbor_map)) {
+    if (!CheckDuplicateKey(key.value(), &cbor_map) ||
+        !CheckOutOfOrderKey(key.value(), &cbor_map)) {
       return base::nullopt;
     }
 
-    cbor_map.insert_or_assign(key.value().GetString(),
-                              std::move(value.value()));
+    cbor_map.insert_or_assign(std::move(key.value()), std::move(value.value()));
   }
   return CBORValue(std::move(cbor_map));
 }
@@ -221,7 +222,7 @@ void CBORReader::CheckExtraneousData() {
     error_code_ = DecoderError::EXTRANEOUS_DATA;
 }
 
-bool CBORReader::CheckDuplicateKey(const std::string& new_key,
+bool CBORReader::CheckDuplicateKey(const CBORValue& new_key,
                                    CBORValue::MapValue* map) {
   if (base::ContainsKey(*map, new_key)) {
     error_code_ = DecoderError::DUPLICATE_KEY;
@@ -238,7 +239,7 @@ bool CBORReader::HasValidUTF8Format(const std::string& string_data) {
   return true;
 }
 
-bool CBORReader::CheckOutOfOrderKey(const std::string& new_key,
+bool CBORReader::CheckOutOfOrderKey(const CBORValue& new_key,
                                     CBORValue::MapValue* map) {
   auto comparator = map->key_comp();
   if (!map->empty() && comparator(new_key, map->rbegin()->first)) {

content/browser/webauth/cbor/cbor_reader.h

@@ -39,6 +39,8 @@
 //  - Callers can decode CBOR values with at most 16 nested depth layer. More
 //    strict restrictions on nesting layer size of CBOR values can be enforced
 //    by setting |max_nesting_level|.
+//  - Only CBOR maps with integer or string type keys are supported due to the
+//    cost of serialization when sorting map keys.
 
 namespace content {
 
@@ -88,9 +90,9 @@ class CONTENT_EXPORT CBORReader {
   base::Optional<CBORValue> ReadCBORMap(uint64_t length, int max_nesting_level);
   bool CanConsume(uint64_t bytes);
   void CheckExtraneousData();
-  bool CheckDuplicateKey(const std::string& new_key, CBORValue::MapValue* map);
+  bool CheckDuplicateKey(const CBORValue& new_key, CBORValue::MapValue* map);
   bool HasValidUTF8Format(const std::string& string_data);
-  bool CheckOutOfOrderKey(const std::string& new_key, CBORValue::MapValue* map);
+  bool CheckOutOfOrderKey(const CBORValue& new_key, CBORValue::MapValue* map);
   bool CheckUintEncodedByteLength(uint8_t additional_bytes, uint64_t uint_data);
 
   DecoderError GetErrorCode();

content/browser/webauth/cbor/cbor_reader_unittest.cc

@@ -43,7 +43,7 @@ TEST(CBORReaderTest, TestReadUint) {
 }
 
 TEST(CBORReaderTest, TestUintEncodedWithNonMinimumByteLength) {
-  static const std::vector<std::vector<uint8_t>> non_minimal_uint_encodings = {
+  static const std::vector<uint8_t> non_minimal_uint_encodings[] = {
       // Uint 23 encoded with 1 byte.
       {0x18, 0x17},
       // Uint 255 encoded with 2 bytes.
@@ -51,11 +51,37 @@ TEST(CBORReaderTest, TestUintEncodedWithNonMinimumByteLength) {
       // Uint 65535 encoded with 4 byte.
       {0x1a, 0x00, 0x00, 0xff, 0xff},
       // Uint 4294967295 encoded with 8 byte.
-      {0x1b, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff}};
+      {0x1b, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff},
+
+      // When decoding byte has more than one syntax error, the first syntax
+      // error encountered during deserialization is returned as the error code.
+      {
+          0xa2,        // map with non-minimally encoded key
+          0x17,        // key 24
+          0x61, 0x42,  // value :"B"
+
+          0x18, 0x17,  // key 23 encoded with extra byte
+          0x61, 0x45   // value "E"
+      },
+      {
+          0xa2,        // map with out of order and non-minimally encoded key
+          0x18, 0x17,  // key 23 encoded with extra byte
+          0x61, 0x45,  // value "E"
+          0x17,        // key 23
+          0x61, 0x42   // value :"B"
+      },
+      {
+          0xa2,        // map with duplicate non-minimally encoded key
+          0x18, 0x17,  // key 23 encoded with extra byte
+          0x61, 0x45,  // value "E"
+          0x18, 0x17,  // key 23 encoded with extra byte
+          0x61, 0x42   // value :"B"
+      },
+  };
 
   int test_case_index = 0;
   CBORReader::DecoderError error_code;
-  for (const auto non_minimal_uint : non_minimal_uint_encodings) {
+  for (const auto& non_minimal_uint : non_minimal_uint_encodings) {
     testing::Message scope_message;
     scope_message << "testing element at index " << test_case_index++;
     SCOPED_TRACE(scope_message);
@@ -177,7 +203,7 @@ TEST(CBORReaderTest, TestReadArray) {
 
   base::Optional<CBORValue> cbor = CBORReader::Read(kArrayTestCaseCbor);
   ASSERT_TRUE(cbor.has_value());
-  CBORValue cbor_array = std::move(cbor.value());
+  const CBORValue cbor_array = std::move(cbor.value());
   ASSERT_EQ(cbor_array.type(), CBORValue::Type::ARRAY);
   ASSERT_THAT(cbor_array.GetArray(), testing::SizeIs(25));
 
@@ -196,7 +222,10 @@ TEST(CBORReaderTest, TestReadArray) {
 TEST(CBORReaderTest, TestReadMapWithMapValue) {
   static const std::vector<uint8_t> kMapTestCaseCbor = {
       // clang-format off
-      0xa3,  // map of 3 pairs:
+      0xa4,  // map with 4 key value pairs:
+        0x18, 0x18,  // 24
+        0x63, 0x61, 0x62, 0x63, // "abc"
+
         0x60,  // ""
         0x61, 0x2e,  // "."
 
@@ -210,23 +239,82 @@ TEST(CBORReaderTest, TestReadMapWithMapValue) {
 
   base::Optional<CBORValue> cbor = CBORReader::Read(kMapTestCaseCbor);
   ASSERT_TRUE(cbor.has_value());
-  CBORValue cbor_val = std::move(cbor.value());
+  const CBORValue cbor_val = std::move(cbor.value());
   ASSERT_EQ(cbor_val.type(), CBORValue::Type::MAP);
-  ASSERT_EQ(cbor_val.GetMap().size(), 3u);
+  ASSERT_EQ(cbor_val.GetMap().size(), 4u);
 
-  ASSERT_EQ(cbor_val.GetMap().count(""), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("")->second.type(), CBORValue::Type::STRING);
-  EXPECT_EQ(cbor_val.GetMap().find("")->second.GetString(), ".");
+  const CBORValue key_uint(24);
+  ASSERT_EQ(cbor_val.GetMap().count(key_uint), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_uint)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_uint)->second.GetString(), "abc");
 
-  ASSERT_EQ(cbor_val.GetMap().count("b"), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("b")->second.type(),
+  const CBORValue key_empty_string("");
+  ASSERT_EQ(cbor_val.GetMap().count(key_empty_string), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_empty_string)->second.type(),
             CBORValue::Type::STRING);
-  EXPECT_EQ(cbor_val.GetMap().find("b")->second.GetString(), "B");
+  EXPECT_EQ(cbor_val.GetMap().find(key_empty_string)->second.GetString(), ".");
 
-  ASSERT_EQ(cbor_val.GetMap().count("aa"), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("aa")->second.type(),
+  const CBORValue key_b("b");
+  ASSERT_EQ(cbor_val.GetMap().count(key_b), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_b)->second.type(),
             CBORValue::Type::STRING);
-  EXPECT_EQ(cbor_val.GetMap().find("aa")->second.GetString(), "AA");
+  EXPECT_EQ(cbor_val.GetMap().find(key_b)->second.GetString(), "B");
+
+  const CBORValue key_aa("aa");
+  ASSERT_EQ(cbor_val.GetMap().count(key_aa), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_aa)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_aa)->second.GetString(), "AA");
+}
+
+TEST(CBORReaderTest, TestReadMapWithIntegerKeys) {
+  static const std::vector<uint8_t> kMapWithIntegerKeyCbor = {
+      // clang-format off
+      0xA4,                 // map with 4 key value pairs
+         0x01,              // key : 1
+         0x61, 0x61,        // value : "a"
+
+         0x09,              // key : 9
+         0x61, 0x62,        // value : "b"
+
+         0x19, 0x03, 0xE7,  // key : 999
+         0x61, 0x63,        // value "c"
+
+         0x19, 0x04, 0x57,  // key : 1111
+         0x61, 0x64,        // value : "d"
+      // clang-format on
+  };
+
+  base::Optional<CBORValue> cbor = CBORReader::Read(kMapWithIntegerKeyCbor);
+  ASSERT_TRUE(cbor.has_value());
+  const CBORValue cbor_val = std::move(cbor.value());
+  ASSERT_EQ(cbor_val.type(), CBORValue::Type::MAP);
+  ASSERT_EQ(cbor_val.GetMap().size(), 4u);
+
+  const CBORValue key_1(1);
+  ASSERT_EQ(cbor_val.GetMap().count(key_1), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_1)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_1)->second.GetString(), "a");
+
+  const CBORValue key_9(9);
+  ASSERT_EQ(cbor_val.GetMap().count(key_9), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_9)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_9)->second.GetString(), "b");
+
+  const CBORValue key_999(999);
+  ASSERT_EQ(cbor_val.GetMap().count(key_999), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_999)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_999)->second.GetString(), "c");
+
+  const CBORValue key_1111(1111);
+  ASSERT_EQ(cbor_val.GetMap().count(key_1111), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_1111)->second.type(),
+            CBORValue::Type::STRING);
+  EXPECT_EQ(cbor_val.GetMap().find(key_1111)->second.GetString(), "d");
 }
 
 TEST(CBORReaderTest, TestReadMapWithArray) {
@@ -245,19 +333,22 @@ TEST(CBORReaderTest, TestReadMapWithArray) {
 
   base::Optional<CBORValue> cbor = CBORReader::Read(kMapArrayTestCaseCbor);
   ASSERT_TRUE(cbor.has_value());
-  CBORValue cbor_val = std::move(cbor.value());
+  const CBORValue cbor_val = std::move(cbor.value());
   ASSERT_EQ(cbor_val.type(), CBORValue::Type::MAP);
   ASSERT_EQ(cbor_val.GetMap().size(), 2u);
 
-  ASSERT_EQ(cbor_val.GetMap().count("a"), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("a")->second.type(),
+  const CBORValue key_a("a");
+  ASSERT_EQ(cbor_val.GetMap().count(key_a), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_a)->second.type(),
             CBORValue::Type::UNSIGNED);
-  EXPECT_EQ(cbor_val.GetMap().find("a")->second.GetUnsigned(), 1u);
+  EXPECT_EQ(cbor_val.GetMap().find(key_a)->second.GetUnsigned(), 1u);
 
-  ASSERT_EQ(cbor_val.GetMap().count("b"), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("b")->second.type(), CBORValue::Type::ARRAY);
+  const CBORValue key_b("b");
+  ASSERT_EQ(cbor_val.GetMap().count(key_b), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_b)->second.type(),
+            CBORValue::Type::ARRAY);
 
-  CBORValue nested_array = cbor_val.GetMap().find("b")->second.Clone();
+  const CBORValue nested_array = cbor_val.GetMap().find(key_b)->second.Clone();
   ASSERT_EQ(nested_array.GetArray().size(), 2u);
   for (int i = 0; i < 2; i++) {
     ASSERT_THAT(nested_array.GetArray()[i].type(), CBORValue::Type::UNSIGNED);
@@ -285,32 +376,37 @@ TEST(CBORReaderTest, TestReadNestedMap) {
 
   base::Optional<CBORValue> cbor = CBORReader::Read(kNestedMapTestCase);
   ASSERT_TRUE(cbor.has_value());
-  CBORValue cbor_val = std::move(cbor.value());
+  const CBORValue cbor_val = std::move(cbor.value());
   ASSERT_EQ(cbor_val.type(), CBORValue::Type::MAP);
   ASSERT_EQ(cbor_val.GetMap().size(), 2u);
-  ASSERT_EQ(cbor_val.GetMap().count("a"), 1u);
-  ASSERT_EQ(cbor_val.GetMap().find("a")->second.type(),
+
+  const CBORValue key_a("a");
+  ASSERT_EQ(cbor_val.GetMap().count(key_a), 1u);
+  ASSERT_EQ(cbor_val.GetMap().find(key_a)->second.type(),
             CBORValue::Type::UNSIGNED);
-  EXPECT_EQ(cbor_val.GetMap().find("a")->second.GetUnsigned(), 1u);
+  EXPECT_EQ(cbor_val.GetMap().find(key_a)->second.GetUnsigned(), 1u);
 
-  ASSERT_EQ(cbor_val.GetMap().count("b"), 1u);
-  CBORValue nested_map = cbor_val.GetMap().find("b")->second.Clone();
+  const CBORValue key_b("b");
+  ASSERT_EQ(cbor_val.GetMap().count(key_b), 1u);
+  const CBORValue nested_map = cbor_val.GetMap().find(key_b)->second.Clone();
   ASSERT_EQ(nested_map.type(), CBORValue::Type::MAP);
   ASSERT_EQ(nested_map.GetMap().size(), 2u);
 
-  ASSERT_EQ(nested_map.GetMap().count("c"), 1u);
-  ASSERT_EQ(nested_map.GetMap().find("c")->second.type(),
+  const CBORValue key_c("c");
+  ASSERT_EQ(nested_map.GetMap().count(key_c), 1u);
+  ASSERT_EQ(nested_map.GetMap().find(key_c)->second.type(),
             CBORValue::Type::UNSIGNED);
-  EXPECT_EQ(nested_map.GetMap().find("c")->second.GetUnsigned(), 2u);
+  EXPECT_EQ(nested_map.GetMap().find(key_c)->second.GetUnsigned(), 2u);
 
-  ASSERT_EQ(nested_map.GetMap().count("d"), 1u);
-  ASSERT_EQ(nested_map.GetMap().find("d")->second.type(),
+  const CBORValue key_d("d");
+  ASSERT_EQ(nested_map.GetMap().count(key_d), 1u);
+  ASSERT_EQ(nested_map.GetMap().find(key_d)->second.type(),
             CBORValue::Type::UNSIGNED);
-  EXPECT_EQ(nested_map.GetMap().find("d")->second.GetUnsigned(), 3u);
+  EXPECT_EQ(nested_map.GetMap().find(key_d)->second.GetUnsigned(), 3u);
 }
 
 TEST(CBORReaderTest, TestIncompleteCBORDataError) {
-  static const std::vector<std::vector<uint8_t>> incomplete_cbor_list = {
+  static const std::vector<uint8_t> incomplete_cbor_list[] = {
       // Additional info byte corresponds to unsigned int that corresponds
       // to 2 additional bytes. But actual data encoded  in one byte.
       {0x19, 0x03},
@@ -327,7 +423,7 @@ TEST(CBORReaderTest, TestIncompleteCBORDataError) {
   };
 
   int test_element_index = 0;
-  for (auto incomplete_data : incomplete_cbor_list) {
+  for (const auto& incomplete_data : incomplete_cbor_list) {
     testing::Message scope_message;
     scope_message << "testing incomplete data at index : "
                   << test_element_index++;
@@ -342,14 +438,14 @@ TEST(CBORReaderTest, TestIncompleteCBORDataError) {
 }
 
 // While RFC 7049 allows CBOR map keys with all types, current decoder only
-// supports string type for CBOR map keys. Checks for error when decoding CBOR
-// map with key other than UTF-8 string.
+// supports unsigned integer and string keys.
 TEST(CBORReaderTest, TestUnsupportedMapKeyFormatError) {
   static const std::vector<uint8_t> kMapWithUintKey = {
       // clang-format off
       0xa2,        // map of 2 pairs
-        0x01,      // key : 1
-        0x02,      // value : 2
+
+        0x82, 0x01, 0x02,  // invalid key : [1, 2]
+        0x02,              // value : 2
 
         0x61, 0x64,  // key : "d"
         0x03,        // value : 3
@@ -364,7 +460,7 @@ TEST(CBORReaderTest, TestUnsupportedMapKeyFormatError) {
 }
 
 TEST(CBORReaderTest, TestUnknownAdditionalInfoError) {
-  static const std::vector<std::vector<uint8_t>> kUnknownAdditionalInfoList = {
+  static const std::vector<uint8_t> kUnknownAdditionalInfoList[] = {
       // "IETF" encoded with major type 3 and additional info of 28.
       {0x7C, 0x49, 0x45, 0x54, 0x46},
       // "\"\\" encoded with major type 3 and additional info of 29.
@@ -375,7 +471,7 @@ TEST(CBORReaderTest, TestUnknownAdditionalInfoError) {
       {0x7F, 0xe6, 0xb0, 0xb4}};
 
   int test_element_index = 0;
-  for (auto incorrect_cbor : kUnknownAdditionalInfoList) {
+  for (const auto& incorrect_cbor : kUnknownAdditionalInfoList) {
     testing::Message scope_message;
     scope_message << "testing data : " << test_element_index++;
     SCOPED_TRACE(scope_message);
@@ -389,7 +485,7 @@ TEST(CBORReaderTest, TestUnknownAdditionalInfoError) {
 }
 
 TEST(CBORReaderTest, TestTooMuchNestingError) {
-  static const std::vector<std::vector<uint8_t>> kZeroDepthCBORList = {
+  static const std::vector<uint8_t> kZeroDepthCBORList[] = {
       // Unsigned int with value 100.
       {0x18, 0x64},
       // CBOR bytestring of length 4.
@@ -403,7 +499,7 @@ TEST(CBORReaderTest, TestTooMuchNestingError) {
   };
 
   int test_element_index = 0;
-  for (auto zero_depth_data : kZeroDepthCBORList) {
+  for (const auto& zero_depth_data : kZeroDepthCBORList) {
     testing::Message scope_message;
     scope_message << "testing zero nested data : " << test_element_index++;
     SCOPED_TRACE(scope_message);
@@ -444,35 +540,44 @@ TEST(CBORReaderTest, TestTooMuchNestingError) {
 }
 
 TEST(CBORReaderTest, TestOutOfOrderKeyError) {
-  static const std::vector<uint8_t> kMapWithUnsortedKey = {
+  static const std::vector<uint8_t> kMapsWithUnsortedKeys[] = {
       // clang-format off
-      0xa6,  // map of 6 pairs:
-        0x60,  // ""
-        0x61, 0x2e,  // "."
-
-        0x61, 0x62,  // "b"
-        0x61, 0x42,  // "B"
-
-        0x61, 0x63,  // "c"
-        0x61, 0x43,  // "C"
-
-        0x61, 0x64,  // "d"
-        0x61, 0x44,  // "D"
-
-        0x61, 0x61,  // "a" (Out of order key)
-        0x61, 0x45,  // "E"
-
-        0x62, 0x61, 0x61,  // "aa"
-        0x62, 0x41, 0x41,  // "AA"
-      // clang-format on
+      {0xa2,  // map with 2 keys with same major type and length
+         0x61, 0x62,  // key "b"
+         0x61, 0x42,  // value :"B"
+
+         0x61, 0x61,  // key "a" (out of order byte-wise lexically)
+         0x61, 0x45   // value "E"
+      },
+      {0xa2,  // map with 2 keys with different major type
+         0x61, 0x62,        // key "b"
+         0x02,              // value 2
+
+         // key 1000 (out of order since lower major type sorts first)
+         0x19, 0x03, 0xe8,
+         0x61, 0x61,        // value a
+      },
+      {0xa2,  // map with 2 keys with same major type
+         0x19, 0x03, 0xe8,  // key 1000  (out of order due to longer length)
+         0x61, 0x61,        //value "a"
+
+         0x0a,              // key 10
+         0x61, 0x62},       // value "b"
+      //clang-format on
   };
 
+  int test_element_index = 0;
   CBORReader::DecoderError error_code;
+  for (const auto& unsorted_map : kMapsWithUnsortedKeys) {
+    testing::Message scope_message;
+    scope_message << "testing unsorted map : " << test_element_index++;
+    SCOPED_TRACE(scope_message);
 
-  base::Optional<CBORValue> cbor =
-      CBORReader::Read(kMapWithUnsortedKey, &error_code);
-  EXPECT_FALSE(cbor.has_value());
-  EXPECT_EQ(error_code, CBORReader::DecoderError::OUT_OF_ORDER_KEY);
+    base::Optional<CBORValue> cbor =
+        CBORReader::Read(unsorted_map, &error_code);
+    EXPECT_FALSE(cbor.has_value());
+    EXPECT_EQ(error_code, CBORReader::DecoderError::OUT_OF_ORDER_KEY);
+  }
 }
 
 TEST(CBORReaderTest, TestDuplicateKeyError) {
@@ -510,7 +615,7 @@ TEST(CBORReaderTest, TestDuplicateKeyError) {
 // Leveraging Markus Kuhn’s UTF-8 decoder stress test. See
 // http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt for details.
 TEST(CBORReaderTest, TestIncorrectStringEncodingError) {
-  static const std::vector<std::vector<uint8_t>> utf8_character_encodings = {
+  static const std::vector<uint8_t> utf8_character_encodings[] = {
       // Corresponds to utf8 encoding of "퟿" (section 2.3.1 of stress test).
       {0x63, 0xED, 0x9F, 0xBF},
       // Corresponds to utf8 encoding of "" (section 2.3.2 of stress test).
@@ -521,7 +626,7 @@ TEST(CBORReaderTest, TestIncorrectStringEncodingError) {
 
   int test_element_index = 0;
   CBORReader::DecoderError error_code;
-  for (auto cbor_byte : utf8_character_encodings) {
+  for (const auto& cbor_byte : utf8_character_encodings) {
     testing::Message scope_message;
     scope_message << "testing cbor data utf8 encoding : "
                   << test_element_index++;
@@ -542,7 +647,7 @@ TEST(CBORReaderTest, TestIncorrectStringEncodingError) {
 }
 
 TEST(CBORReaderTest, TestExtraneousCBORDataError) {
-  static const std::vector<std::vector<uint8_t>> zero_padded_cbor_list = {
+  static const std::vector<uint8_t> zero_padded_cbor_list[] = {
       // 1 extra byte after a 2-byte unsigned int.
       {0x19, 0x03, 0x05, 0x00},
       // 1 extra byte after a 4-byte cbor byte array.
@@ -556,7 +661,7 @@ TEST(CBORReaderTest, TestExtraneousCBORDataError) {
   };
 
   int test_element_index = 0;
-  for (auto extraneous_cbor_data : zero_padded_cbor_list) {
+  for (const auto& extraneous_cbor_data : zero_padded_cbor_list) {
     testing::Message scope_message;
     scope_message << "testing cbor extraneous data : " << test_element_index++;
     SCOPED_TRACE(scope_message);

content/browser/webauth/cbor/cbor_values.cc

@@ -74,7 +74,8 @@ CBORValue::CBORValue(ArrayValue&& in_array) noexcept
 CBORValue::CBORValue(const MapValue& in_map) : type_(Type::MAP), map_value_() {
   map_value_.reserve(in_map.size());
   for (const auto& it : in_map)
-    map_value_.emplace_hint(map_value_.end(), it.first, it.second.Clone());
+    map_value_.emplace_hint(map_value_.end(), it.first.Clone(),
+                            it.second.Clone());
 }
 
 CBORValue::CBORValue(MapValue&& in_map) noexcept
@@ -111,7 +112,7 @@ CBORValue CBORValue::Clone() const {
   return CBORValue();
 }
 
-uint64_t CBORValue::GetUnsigned() const {
+const uint64_t& CBORValue::GetUnsigned() const {
   CHECK(is_unsigned());
   return unsigned_value_;
 }

content/browser/webauth/cbor/cbor_values.h

@@ -29,8 +29,7 @@ class CONTENT_EXPORT CBORValue {
     //
     // The sort order defined in CTAP is:
     //   • If the major types are different, the one with the lower value in
-    //     numerical order sorts earlier. (Moot in this code because all keys
-    //     are strings.)
+    //     numerical order sorts earlier.
     //   • If two keys have different lengths, the shorter one sorts earlier.
     //   • If two keys have the same length, the one with the lower value in
     //     (byte-wise) lexical order sorts earlier.
@@ -38,17 +37,32 @@ class CONTENT_EXPORT CBORValue {
     // See section 6 of https://fidoalliance.org/specs/fido-v2.0-rd-20170927/
     // fido-client-to-authenticator-protocol-v2.0-rd-20170927.html.
     //
-    // The sort order defined in
-    // https://tools.ietf.org/html/rfc7049#section-3.9 is similar to the CTAP
-    // order implemented here, but it sorts purely by serialised key and
-    // doesn't specify that major types are compared first. Thus the shortest
-    // key sorts first by the RFC rules (irrespective of the major type), but
-    // may not by CTAP rules.
-    bool operator()(const base::StringPiece& a,
-                    const base::StringPiece& b) const {
-      const size_t a_size = a.size();
-      const size_t b_size = b.size();
-      return std::tie(a_size, a) < std::tie(b_size, b);
+    // THE CTAP SORT ORDER IMPLEMENTED HERE DIFFERS FROM THE CANONICAL CBOR
+    // ORDER defined in https://tools.ietf.org/html/rfc7049#section-3.9, in that
+    // the latter sorts purely by serialised key and doesn't specify that major
+    // types are compared first. Thus the shortest key sorts first by the RFC
+    // rules (irrespective of the major type), but may not by CTAP rules.
+    bool operator()(const CBORValue& a, const CBORValue& b) const {
+      DCHECK((a.is_unsigned() || a.is_string()) &&
+             (b.is_unsigned() || b.is_string()));
+      if (a.type() != b.type())
+        return a.type() < b.type();
+      switch (a.type()) {
+        case Type::UNSIGNED:
+          return a.GetUnsigned() < b.GetUnsigned();
+        case Type::STRING: {
+          const auto& a_str = a.GetString();
+          const size_t a_length = a_str.size();
+          const auto& b_str = b.GetString();
+          const size_t b_length = b_str.size();
+          return std::tie(a_length, a_str) < std::tie(b_length, b_str);
+        }
+        default:
+          break;
+      }
+
+      NOTREACHED();
+      return false;
     }
 
     using is_transparent = void;
@@ -56,7 +70,7 @@ class CONTENT_EXPORT CBORValue {
 
   using BinaryValue = std::vector<uint8_t>;
   using ArrayValue = std::vector<CBORValue>;
-  using MapValue = base::flat_map<std::string, CBORValue, CTAPLess>;
+  using MapValue = base::flat_map<CBORValue, CBORValue, CTAPLess>;
 
   enum class Type {
     UNSIGNED = 0,
@@ -107,7 +121,7 @@ class CONTENT_EXPORT CBORValue {
   bool is_map() const { return type() == Type::MAP; }
 
   // These will all fatally assert if the type doesn't match.
-  uint64_t GetUnsigned() const;
+  const uint64_t& GetUnsigned() const;
   const BinaryValue& GetBytestring() const;
   // Returned string may contain NUL characters.
   const std::string& GetString() const;

content/browser/webauth/cbor/cbor_values_unittest.cc

@@ -32,34 +32,34 @@ TEST(CBORValuesTest, TestNothrow) {
 // Test constructors
 TEST(CBORValuesTest, ConstructUnsigned) {
   CBORValue value(37);
-  EXPECT_EQ(CBORValue::Type::UNSIGNED, value.type());
+  ASSERT_EQ(CBORValue::Type::UNSIGNED, value.type());
   EXPECT_EQ(37u, value.GetUnsigned());
 }
 
 TEST(CBORValuesTest, ConstructStringFromConstCharPtr) {
   const char* str = "foobar";
   CBORValue value(str);
-  EXPECT_EQ(CBORValue::Type::STRING, value.type());
+  ASSERT_EQ(CBORValue::Type::STRING, value.type());
   EXPECT_EQ("foobar", value.GetString());
 }
 
 TEST(CBORValuesTest, ConstructStringFromStdStringConstRef) {
   std::string str = "foobar";
   CBORValue value(str);
-  EXPECT_EQ(CBORValue::Type::STRING, value.type());
+  ASSERT_EQ(CBORValue::Type::STRING, value.type());
   EXPECT_EQ("foobar", value.GetString());
 }
 
 TEST(CBORValuesTest, ConstructStringFromStdStringRefRef) {
   std::string str = "foobar";
   CBORValue value(std::move(str));
-  EXPECT_EQ(CBORValue::Type::STRING, value.type());
+  ASSERT_EQ(CBORValue::Type::STRING, value.type());
   EXPECT_EQ("foobar", value.GetString());
 }
 
 TEST(CBORValuesTest, ConstructBytestring) {
   CBORValue value(CBORValue::BinaryValue({0xF, 0x0, 0x0, 0xB, 0xA, 0x2}));
-  EXPECT_EQ(CBORValue::Type::BYTE_STRING, value.type());
+  ASSERT_EQ(CBORValue::Type::BYTE_STRING, value.type());
   EXPECT_EQ(CBORValue::BinaryValue({0xF, 0x0, 0x0, 0xB, 0xA, 0x2}),
             value.GetBytestring());
 }
@@ -69,42 +69,43 @@ TEST(CBORValuesTest, ConstructArray) {
   array.emplace_back(CBORValue("foo"));
   {
     CBORValue value(array);
-    EXPECT_EQ(CBORValue::Type::ARRAY, value.type());
-    EXPECT_EQ(1u, value.GetArray().size());
-    EXPECT_EQ(CBORValue::Type::STRING, value.GetArray()[0].type());
+    ASSERT_EQ(CBORValue::Type::ARRAY, value.type());
+    ASSERT_EQ(1u, value.GetArray().size());
+    ASSERT_EQ(CBORValue::Type::STRING, value.GetArray()[0].type());
     EXPECT_EQ("foo", value.GetArray()[0].GetString());
   }
 
   array.back() = CBORValue("bar");
   {
     CBORValue value(std::move(array));
-    EXPECT_EQ(CBORValue::Type::ARRAY, value.type());
-    EXPECT_EQ(1u, value.GetArray().size());
-    EXPECT_EQ(CBORValue::Type::STRING, value.GetArray()[0].type());
+    ASSERT_EQ(CBORValue::Type::ARRAY, value.type());
+    ASSERT_EQ(1u, value.GetArray().size());
+    ASSERT_EQ(CBORValue::Type::STRING, value.GetArray()[0].type());
     EXPECT_EQ("bar", value.GetArray()[0].GetString());
   }
 }
 
 TEST(CBORValuesTest, ConstructMap) {
   CBORValue::MapValue map;
-  map.emplace("foo", CBORValue("bar"));
+  const CBORValue key_foo("foo");
+  map[CBORValue("foo")] = CBORValue("bar");
   {
     CBORValue value(map);
-    EXPECT_EQ(CBORValue::Type::MAP, value.type());
-    ASSERT_EQ(value.GetMap().count("foo"), 1u);
-    EXPECT_EQ(CBORValue::Type::STRING,
-              value.GetMap().find("foo")->second.type());
-    EXPECT_EQ("bar", value.GetMap().find("foo")->second.GetString());
+    ASSERT_EQ(CBORValue::Type::MAP, value.type());
+    ASSERT_EQ(value.GetMap().count(key_foo), 1u);
+    ASSERT_EQ(CBORValue::Type::STRING,
+              value.GetMap().find(key_foo)->second.type());
+    EXPECT_EQ("bar", value.GetMap().find(key_foo)->second.GetString());
   }
 
-  map["foo"] = CBORValue("baz");
+  map[CBORValue("foo")] = CBORValue("baz");
   {
     CBORValue value(std::move(map));
-    EXPECT_EQ(CBORValue::Type::MAP, value.type());
-    ASSERT_EQ(value.GetMap().count("foo"), 1u);
-    EXPECT_EQ(CBORValue::Type::STRING,
-              value.GetMap().find("foo")->second.type());
-    EXPECT_EQ("baz", value.GetMap().find("foo")->second.GetString());
+    ASSERT_EQ(CBORValue::Type::MAP, value.type());
+    ASSERT_EQ(value.GetMap().count(key_foo), 1u);
+    ASSERT_EQ(CBORValue::Type::STRING,
+              value.GetMap().find(key_foo)->second.type());
+    EXPECT_EQ("baz", value.GetMap().find(key_foo)->second.GetString());
   }
 }
 
@@ -112,39 +113,39 @@ TEST(CBORValuesTest, ConstructMap) {
 TEST(CBORValuesTest, CopyUnsigned) {
   CBORValue value(74);
   CBORValue copied_value(value.Clone());
-  EXPECT_EQ(value.type(), copied_value.type());
+  ASSERT_EQ(value.type(), copied_value.type());
   EXPECT_EQ(value.GetUnsigned(), copied_value.GetUnsigned());
 
   CBORValue blank;
 
   blank = value.Clone();
-  EXPECT_EQ(value.type(), blank.type());
+  ASSERT_EQ(value.type(), blank.type());
   EXPECT_EQ(value.GetUnsigned(), blank.GetUnsigned());
 }
 
 TEST(CBORValuesTest, CopyString) {
   CBORValue value("foobar");
   CBORValue copied_value(value.Clone());
-  EXPECT_EQ(value.type(), copied_value.type());
+  ASSERT_EQ(value.type(), copied_value.type());
   EXPECT_EQ(value.GetString(), copied_value.GetString());
 
   CBORValue blank;
 
   blank = value.Clone();
-  EXPECT_EQ(value.type(), blank.type());
+  ASSERT_EQ(value.type(), blank.type());
   EXPECT_EQ(value.GetString(), blank.GetString());
 }
 
 TEST(CBORValuesTest, CopyBytestring) {
   CBORValue value(CBORValue::BinaryValue({0xF, 0x0, 0x0, 0xB, 0xA, 0x2}));
   CBORValue copied_value(value.Clone());
-  EXPECT_EQ(value.type(), copied_value.type());
+  ASSERT_EQ(value.type(), copied_value.type());
   EXPECT_EQ(value.GetBytestring(), copied_value.GetBytestring());
 
   CBORValue blank;
 
   blank = value.Clone();
-  EXPECT_EQ(value.type(), blank.type());
+  ASSERT_EQ(value.type(), blank.type());
   EXPECT_EQ(value.GetBytestring(), blank.GetBytestring());
 }
 
@@ -154,7 +155,7 @@ TEST(CBORValuesTest, CopyArray) {
   CBORValue value(std::move(array));
 
   CBORValue copied_value(value.Clone());
-  EXPECT_EQ(1u, copied_value.GetArray().size());
+  ASSERT_EQ(1u, copied_value.GetArray().size());
   EXPECT_EQ(value.GetArray()[0].GetUnsigned(),
             copied_value.GetArray()[0].GetUnsigned());
 
@@ -165,23 +166,25 @@ TEST(CBORValuesTest, CopyArray) {
 
 TEST(CBORValuesTest, CopyMap) {
   CBORValue::MapValue map;
-  map.emplace("unsigned", CBORValue(123));
+  CBORValue key_a("a");
+  map[CBORValue("a")] = CBORValue(123);
   CBORValue value(std::move(map));
 
   CBORValue copied_value(value.Clone());
   EXPECT_EQ(1u, copied_value.GetMap().size());
-  ASSERT_EQ(value.GetMap().count("unsigned"), 1u);
-  ASSERT_EQ(copied_value.GetMap().count("unsigned"), 1u);
-  EXPECT_EQ(value.GetMap().find("unsigned")->second.GetUnsigned(),
-            copied_value.GetMap().find("unsigned")->second.GetUnsigned());
+  ASSERT_EQ(value.GetMap().count(key_a), 1u);
+  ASSERT_EQ(copied_value.GetMap().count(key_a), 1u);
+  ASSERT_TRUE(copied_value.GetMap().find(key_a)->second.is_unsigned());
+  EXPECT_EQ(value.GetMap().find(key_a)->second.GetUnsigned(),
+            copied_value.GetMap().find(key_a)->second.GetUnsigned());
 
   CBORValue blank;
   blank = value.Clone();
   EXPECT_EQ(1u, blank.GetMap().size());
-  ASSERT_EQ(value.GetMap().count("unsigned"), 1u);
-  ASSERT_EQ(copied_value.GetMap().count("unsigned"), 1u);
-  EXPECT_EQ(value.GetMap().find("unsigned")->second.GetUnsigned(),
-            copied_value.GetMap().find("unsigned")->second.GetUnsigned());
+  ASSERT_EQ(blank.GetMap().count(key_a), 1u);
+  ASSERT_TRUE(blank.GetMap().find(key_a)->second.is_unsigned());
+  EXPECT_EQ(value.GetMap().find(key_a)->second.GetUnsigned(),
+            blank.GetMap().find(key_a)->second.GetUnsigned());
 }
 
 // Test move constructors and move-assignment
@@ -227,24 +230,28 @@ TEST(CBORValuesTest, MoveBytestring) {
 
 TEST(CBORValuesTest, MoveConstructMap) {
   CBORValue::MapValue map;
-  map.emplace("unsigned", CBORValue(123));
+  const CBORValue key_a("a");
+  map[CBORValue("a")] = CBORValue(123);
 
   CBORValue value(std::move(map));
   CBORValue moved_value(std::move(value));
-  EXPECT_EQ(CBORValue::Type::MAP, moved_value.type());
-  ASSERT_EQ(moved_value.GetMap().count("unsigned"), 1u);
-  EXPECT_EQ(123u, moved_value.GetMap().find("unsigned")->second.GetUnsigned());
+  ASSERT_EQ(CBORValue::Type::MAP, moved_value.type());
+  ASSERT_EQ(moved_value.GetMap().count(key_a), 1u);
+  ASSERT_TRUE(moved_value.GetMap().find(key_a)->second.is_unsigned());
+  EXPECT_EQ(123u, moved_value.GetMap().find(key_a)->second.GetUnsigned());
 }
 
 TEST(CBORValuesTest, MoveAssignMap) {
   CBORValue::MapValue map;
-  map.emplace("unsigned", CBORValue(123));
+  const CBORValue key_a("a");
+  map[CBORValue("a")] = CBORValue(123);
 
   CBORValue blank;
   blank = CBORValue(std::move(map));
-  EXPECT_EQ(CBORValue::Type::MAP, blank.type());
-  ASSERT_EQ(blank.GetMap().count("unsigned"), 1u);
-  EXPECT_EQ(123u, blank.GetMap().find("unsigned")->second.GetUnsigned());
+  ASSERT_TRUE(blank.is_map());
+  ASSERT_EQ(blank.GetMap().count(key_a), 1u);
+  ASSERT_TRUE(blank.GetMap().find(key_a)->second.is_unsigned());
+  EXPECT_EQ(123u, blank.GetMap().find(key_a)->second.GetUnsigned());
 }
 
 TEST(CBORValuesTest, MoveArray) {

content/browser/webauth/cbor/cbor_writer.cc

@@ -81,7 +81,7 @@ bool CBORWriter::EncodeCBOR(const CBORValue& node, int max_nesting_level) {
       StartItem(CBORValue::Type::MAP, map.size());
 
       for (const auto& value : map) {
-        if (!EncodeCBOR(CBORValue(value.first), max_nesting_level - 1))
+        if (!EncodeCBOR(value.first, max_nesting_level - 1))
           return false;
         if (!EncodeCBOR(value.second, max_nesting_level - 1))
           return false;

content/browser/webauth/cbor/cbor_writer_unittest.cc

@@ -105,37 +105,78 @@ TEST(CBORWriterTest, TestWriteArray) {
 TEST(CBORWriterTest, TestWriteMapWithMapValue) {
   static const uint8_t kMapTestCaseCbor[] = {
       // clang-format off
-      0xa6,  // map of 6 pairs:
-        0x60,  // ""
-        0x61, 0x2e,  // "."
+      0xAF,  // map of 7 pairs:
+        0x00,          // key 0
+        0x61, 0x61,    // value "a"
 
-        0x61, 0x62,  // "b"
-        0x61, 0x42,  // "B"
+        0x17,          // key 23
+        0x61,  0x62,   // value "b"
+
+        0x18, 0x18,    // key 24
+        0x61,   0x63,  // value "c"
+
+        0x18, 0xFF,        // key 255
+        0x61,  0x64,       // value "d"
+
+        0x19, 0x01, 0x00,  // key 256
+        0x61, 0x65,        // value "e"
+
+        0x19, 0xFF, 0xFF,  // key 65535
+        0x61,  0x66,       // value "f"
+
+        0x1A, 0x00, 0x01, 0x00, 0x00,   // key 65536
+        0x61, 0x67,                     // value "g"
 
-        0x61, 0x63,  // "c"
-        0x61, 0x43,  // "C"
+        0x1A, 0xFF, 0xFF, 0xFF, 0xFF,   // key 4294967295
+        0x61, 0x68,                     // value "h"
 
-        0x61, 0x64,  // "d"
-        0x61, 0x44,  // "D"
+        // key 4294967296
+        0x1B, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+        0x61, 0x69,  //  value "i"
 
-        0x61, 0x65,  // "e"
-        0x61, 0x45,  // "E"
+        0x60,        // key ""
+        0x61, 0x2e,  // value "."
 
-        0x62, 0x61, 0x61,  // "aa"
-        0x62, 0x41, 0x41,  // "AA"
+        0x61, 0x62,  // key "b"
+        0x61, 0x42,  // value "B"
+
+        0x61, 0x63,  // key "c"
+        0x61, 0x43,  // value "C"
+
+        0x61, 0x64,  // key "d"
+        0x61, 0x44,  // value "D"
+
+        0x61, 0x65,  // key "e"
+        0x61, 0x45,  // value "E"
+
+        0x62, 0x61, 0x61,  // key "aa"
+        0x62, 0x41, 0x41,  // value "AA"
       // clang-format on
   };
   CBORValue::MapValue map;
   // Shorter strings sort first in CTAP, thus the “aa” value should be
   // serialised last in the map.
-  map["aa"] = CBORValue("AA");
-  map["d"] = CBORValue("D");
-  map["b"] = CBORValue("B");
-  map["e"] = CBORValue("E");
-  map["c"] = CBORValue("C");
-  // The empty string is shorter than all others, so should appear first in the
-  // serialisation.
-  map[""] = CBORValue(".");
+  map[CBORValue("aa")] = CBORValue("AA");
+  map[CBORValue("d")] = CBORValue("D");
+  map[CBORValue("b")] = CBORValue("B");
+  map[CBORValue("e")] = CBORValue("E");
+  map[CBORValue("c")] = CBORValue("C");
+  // The empty string is shorter than all others, so should appear first among
+  // the strings.
+  map[CBORValue("")] = CBORValue(".");
+  // Map keys are sorted by major type, by byte length, and then by
+  // byte-wise lexical order. So all integer type keys should appear before
+  // key "".
+  map[CBORValue(uint64_t(0))] = CBORValue("a");
+  map[CBORValue(23)] = CBORValue("b");
+  map[CBORValue(24)] = CBORValue("c");
+  map[CBORValue(255)] = CBORValue("d");
+  map[CBORValue(256)] = CBORValue("e");
+  map[CBORValue(65535)] = CBORValue("f");
+  map[CBORValue(65536)] = CBORValue("g");
+  map[CBORValue(4294967295)] = CBORValue("h");
+  map[CBORValue(4294967296)] = CBORValue("i");
+
   auto cbor = CBORWriter::Write(CBORValue(map));
   ASSERT_TRUE(cbor.has_value());
   EXPECT_THAT(cbor.value(), testing::ElementsAreArray(
@@ -156,11 +197,11 @@ TEST(CBORWriterTest, TestWriteMapWithArray) {
       // clang-format on
   };
   CBORValue::MapValue map;
-  map["a"] = CBORValue(1);
+  map[CBORValue("a")] = CBORValue(1);
   CBORValue::ArrayValue array;
   array.push_back(CBORValue(2));
   array.push_back(CBORValue(3));
-  map["b"] = CBORValue(array);
+  map[CBORValue("b")] = CBORValue(array);
   auto cbor = CBORWriter::Write(CBORValue(map));
   ASSERT_TRUE(cbor.has_value());
   EXPECT_THAT(cbor.value(),
@@ -185,11 +226,11 @@ TEST(CBORWriterTest, TestWriteNestedMap) {
       // clang-format on
   };
   CBORValue::MapValue map;
-  map["a"] = CBORValue(1);
+  map[CBORValue("a")] = CBORValue(1);
   CBORValue::MapValue nested_map;
-  nested_map["c"] = CBORValue(2);
-  nested_map["d"] = CBORValue(3);
-  map["b"] = CBORValue(nested_map);
+  nested_map[CBORValue("c")] = CBORValue(2);
+  nested_map[CBORValue("d")] = CBORValue(3);
+  map[CBORValue("b")] = CBORValue(nested_map);
   auto cbor = CBORWriter::Write(CBORValue(map));
   ASSERT_TRUE(cbor.has_value());
   EXPECT_THAT(cbor.value(),
@@ -212,7 +253,7 @@ TEST(CBORWriterTest, TestWriteSingleLayer) {
   CBORValue::ArrayValue simple_array;
   simple_array.push_back(CBORValue(2));
   CBORValue::MapValue simple_map;
-  simple_map["b"] = CBORValue(3);
+  simple_map[CBORValue("b")] = CBORValue(3);
   const CBORValue single_layer_cbor_map = CBORValue(simple_map);
   const CBORValue single_layer_cbor_array = CBORValue(simple_array);
 
@@ -236,11 +277,11 @@ TEST(CBORWriterTest, TestWriteSingleLayer) {
 //            "d": 3}}
 TEST(CBORWriterTest, NestedMaps) {
   CBORValue::MapValue cbor_map;
-  cbor_map["a"] = CBORValue(1);
+  cbor_map[CBORValue("a")] = CBORValue(1);
   CBORValue::MapValue nested_map;
-  nested_map["c"] = CBORValue(2);
-  nested_map["d"] = CBORValue(3);
-  cbor_map["b"] = CBORValue(nested_map);
+  nested_map[CBORValue("c")] = CBORValue(2);
+  nested_map[CBORValue("d")] = CBORValue(3);
+  cbor_map[CBORValue("b")] = CBORValue(nested_map);
   EXPECT_TRUE(CBORWriter::Write(CBORValue(cbor_map), 2).has_value());
   EXPECT_FALSE(CBORWriter::Write(CBORValue(cbor_map), 1).has_value());
 }
@@ -257,10 +298,10 @@ TEST(CBORWriterTest, UnbalancedNestedContainers) {
   CBORValue::MapValue cbor_map;
   CBORValue::MapValue nested_map;
 
-  cbor_map["a"] = CBORValue(1);
-  nested_map["c"] = CBORValue(2);
-  nested_map["d"] = CBORValue(3);
-  cbor_map["b"] = CBORValue(nested_map);
+  cbor_map[CBORValue("a")] = CBORValue(1);
+  nested_map[CBORValue("c")] = CBORValue(2);
+  nested_map[CBORValue("d")] = CBORValue(3);
+  cbor_map[CBORValue("b")] = CBORValue(nested_map);
   cbor_array.push_back(CBORValue(1));
   cbor_array.push_back(CBORValue(2));
   cbor_array.push_back(CBORValue(3));
@@ -286,18 +327,18 @@ TEST(CBORWriterTest, OverlyNestedCBOR) {
   CBORValue::ArrayValue inner_array;
   CBORValue::ArrayValue array;
 
-  map["a"] = CBORValue(1);
-  nested_map["c"] = CBORValue(2);
-  nested_map["d"] = CBORValue(3);
-  inner_nested_map["e"] = CBORValue(4);
-  inner_nested_map["f"] = CBORValue(5);
+  map[CBORValue("a")] = CBORValue(1);
+  nested_map[CBORValue("c")] = CBORValue(2);
+  nested_map[CBORValue("d")] = CBORValue(3);
+  inner_nested_map[CBORValue("e")] = CBORValue(4);
+  inner_nested_map[CBORValue("f")] = CBORValue(5);
   inner_array.push_back(CBORValue(6));
   array.push_back(CBORValue(6));
   array.push_back(CBORValue(7));
   array.push_back(CBORValue(inner_array));
-  inner_nested_map["g"] = CBORValue(array);
-  nested_map["h"] = CBORValue(inner_nested_map);
-  map["b"] = CBORValue(nested_map);
+  inner_nested_map[CBORValue("g")] = CBORValue(array);
+  nested_map[CBORValue("h")] = CBORValue(inner_nested_map);
+  map[CBORValue("b")] = CBORValue(nested_map);
 
   EXPECT_TRUE(CBORWriter::Write(CBORValue(map), 5).has_value());
   EXPECT_FALSE(CBORWriter::Write(CBORValue(map), 4).has_value());

content/browser/webauth/ec_public_key.cc

@@ -41,9 +41,9 @@ ECPublicKey::ECPublicKey(std::string algorithm,
 
 std::vector<uint8_t> ECPublicKey::EncodeAsCBOR() {
   CBORValue::MapValue map;
-  map["alg"] = CBORValue(algorithm_.c_str());
-  map["x"] = CBORValue(x_coordinate_);
-  map["y"] = CBORValue(y_coordinate_);
+  map[CBORValue("alg")] = CBORValue(algorithm_.c_str());
+  map[CBORValue("x")] = CBORValue(x_coordinate_);
+  map[CBORValue("y")] = CBORValue(y_coordinate_);
   auto cbor = CBORWriter::Write(CBORValue(map));
   DCHECK(cbor.has_value());
   return cbor.value();

content/browser/webauth/fido_attestation_statement.cc

@@ -87,13 +87,13 @@ FidoAttestationStatement::FidoAttestationStatement(
 
 CBORValue::MapValue FidoAttestationStatement::GetAsCBORMap() {
   CBORValue::MapValue attstmt_map;
-  attstmt_map[kSignatureKey] = CBORValue(signature_);
+  attstmt_map[CBORValue(kSignatureKey)] = CBORValue(signature_);
 
   std::vector<CBORValue> array;
   for (auto cert : x509_certificates_) {
     array.push_back(CBORValue(cert));
   }
-  attstmt_map[kX509CertKey] = CBORValue(array);
+  attstmt_map[CBORValue(kX509CertKey)] = CBORValue(array);
   return attstmt_map;
 }