Detect duplicate mapping keys introduced through aliases

src/core/yaml/parser.h

@@ -462,21 +462,28 @@ class Parser {
     return stream.str();
   }
 
+  // The node a scalar key denotes, which is what an anchor on that key has to
+  // name so that aliasing it yields the very same key
+  auto resolve_scalar_node(const Token &token,
+                           const std::optional<std::string> &tag = std::nullopt)
+      -> JSON {
+    // Round-trip mode preserves the original key text, so it is not resolved
+    if (this->roundtrip_) {
+      return JSON{std::string{token.value}};
+    }
+
+    // Resolve a scalar key to its typed value, so that keys such as 0x1 and 1
+    // collapse to the same member name, keeping key handling consistent with
+    // values and alias keys. An explicit tag is honored the same way it would
+    // be for a scalar value, so a string-tagged key keeps its literal text
+    return this->interpret_scalar(token.value, token.scalar_style, tag);
+  }
+
   auto resolve_scalar_key(const Token &token,
                           const std::optional<std::string> &tag = std::nullopt)
       -> std::string {
-    // Round-trip mode preserves the original key text, so it is not resolved
-    if (this->roundtrip_) {
-      return std::string{token.value};
-    }
-    // Resolve a scalar key to its typed value and stringify it, so that keys
-    // such as 0x1 and 1 collapse to the same member name, keeping key handling
-    // consistent with values and alias keys. An explicit tag is honored the
-    // same way it would be for a scalar value, so a string-tagged key keeps its
-    // literal text
-    const auto value{
-        this->interpret_scalar(token.value, token.scalar_style, tag)};
-    return this->json_to_key_string(value, token.line, token.column);
+    return this->json_to_key_string(this->resolve_scalar_node(token, tag),
+                                    token.line, token.column);
   }
 
   auto parse_value(const Token &token, const JSON::ParseContext context,
@@ -664,7 +671,9 @@ class Parser {
                 "document start line"};
           }
           if (anchor_name.has_value() && anchor_line == current_token.line) {
-            JSON key_value{current_token.value};
+            // The anchor names the resolved key node, so that aliasing it
+            // yields the very same member name rather than the raw text
+            JSON key_value{this->resolve_scalar_node(current_token, tag)};
             this->recording_anchor_ = false;
             this->anchors_.insert_or_assign(
                 std::string{anchor_name.value()},
@@ -1423,17 +1432,21 @@ class Parser {
       }
 
       std::optional<std::string> key_tag;
+      std::optional<std::string> key_anchor;
       while (token.type == TokenType::Tag || token.type == TokenType::Anchor) {
         if (token.type == TokenType::Tag) {
           key_tag = this->resolve_tag(token.value);
+        } else {
+          key_anchor = std::string{token.value};
         }
         auto next{this->next_token()};
         assert(next.has_value());
         token = next.value();
       }
 
       if (token.type != TokenType::Scalar &&
-          token.type != TokenType::BlockMappingValue) {
+          token.type != TokenType::BlockMappingValue &&
+          token.type != TokenType::Alias) {
         // RFC 8259 Section 4: an object member name is a string, so an explicit
         // mapping key that is itself a collection cannot be represented as
         // JSON. PyYAML raises on the unhashable key and js-yaml rejects the
@@ -1462,6 +1475,48 @@ class Parser {
         current_key_line = token.line;
         current_key_column = token.column;
 
+        // YAML 1.2.2 Section 7.1: an anchor on an explicit key names that key
+        // for later aliases, exactly as it would on any other node
+        if (key_anchor.has_value()) {
+          this->anchors_.insert_or_assign(
+              key_anchor.value(),
+              AnchoredValue{.value = this->resolve_scalar_node(token, key_tag),
+                            .callbacks = {}});
+        }
+
+        if (seen_keys.contains(key)) [[unlikely]] {
+          throw YAMLDuplicateKeyError{key, token.line, token.column};
+        }
+        seen_keys.insert(key);
+
+        auto next{this->next_token()};
+        if (!next.has_value() || next->type != TokenType::BlockMappingValue) {
+          result.assign(key, JSON{nullptr});
+          if (!next.has_value()) {
+            break;
+          }
+          token = next.value();
+          continue;
+        }
+        token = next.value();
+      }
+
+      // YAML 1.2.2 Section 7.1: an alias node in key position stands for the
+      // value of the anchor it names, which is already resolved and so must not
+      // be resolved a second time
+      if (token.type == TokenType::Alias) {
+        const std::string alias_name{token.value};
+        const auto iterator{this->anchors_.find(alias_name)};
+        if (iterator == this->anchors_.end()) [[unlikely]] {
+          throw YAMLUnknownAnchorError{alias_name, token.line, token.column};
+        }
+
+        key = this->json_to_key_string(iterator->second.value, token.line,
+                                       token.column);
+        key_present = true;
+        current_key_line = token.line;
+        current_key_column = token.column;
+
         if (seen_keys.contains(key)) [[unlikely]] {
           throw YAMLDuplicateKeyError{key, token.line, token.column};
         }
@@ -1719,17 +1774,45 @@ class Parser {
           break;
         }
         next = this->next_token();
-        if (!next.has_value() || next->type != TokenType::Scalar) {
+
+        // YAML 1.2.2 Section 7.1: an anchor on an explicit key names that key
+        // for later aliases, exactly as it would on any other node
+        std::optional<std::string> explicit_key_anchor;
+        if (next.has_value() && next->type == TokenType::Anchor) {
+          explicit_key_anchor = std::string{next->value};
+          next = this->next_token();
+        }
+
+        if (next.has_value() && next->type == TokenType::Alias) {
+          // YAML 1.2.2 Section 7.1: an alias node in key position stands for
+          // the value of the anchor it names, which is already resolved and so
+          // must not be resolved a second time
+          const std::string alias_name{next->value};
+          const auto iterator{this->anchors_.find(alias_name)};
+          if (iterator == this->anchors_.end()) [[unlikely]] {
+            throw YAMLUnknownAnchorError{alias_name, next->line, next->column};
+          }
+
+          key = this->json_to_key_string(iterator->second.value, next->line,
+                                         next->column);
+        } else if (!next.has_value() || next->type != TokenType::Scalar) {
           result.assign("", JSON{nullptr});
           next = this->next_token();
           continue;
+        } else {
+          key = this->resolve_scalar_key(next.value());
+          this->record_key_scalar_style(key, next->scalar_style,
+                                        next->quoted_original);
+          if (explicit_key_anchor.has_value()) {
+            this->anchors_.insert_or_assign(
+                explicit_key_anchor.value(),
+                AnchoredValue{.value = this->resolve_scalar_node(next.value()),
+                              .callbacks = {}});
+          }
         }
 
-        key = this->resolve_scalar_key(next.value());
         key_line = next->line;
         key_column = next->column;
-        this->record_key_scalar_style(key, next->scalar_style,
-                                      next->quoted_original);
 
         if (seen_keys.contains(key)) [[unlikely]] {
           throw YAMLDuplicateKeyError{key, next->line, next->column};

test/numeric/numeric_parse_test.cc

@@ -383,6 +383,38 @@ TEST(to_int64_t_base8_invalid_digit) {
   EXPECT_FALSE(result.has_value());
 }
 
+// A digit that is valid for the base followed by one that is not must be
+// rejected outright, rather than resolving to the prefix that did parse
+TEST(to_int64_t_base8_trailing_invalid_digit) {
+  const std::string input{"18"};
+  const auto result{sourcemeta::core::to_int64_t(input, 8)};
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(to_int64_t_base16_trailing_invalid_digit) {
+  const std::string input{"1g"};
+  const auto result{sourcemeta::core::to_int64_t(input, 16)};
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(to_int64_t_base10_trailing_letters) {
+  const std::string input{"123abc"};
+  const auto result{sourcemeta::core::to_int64_t(input, 10)};
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(to_int64_t_trailing_letters) {
+  const std::string input{"123abc"};
+  const auto result{sourcemeta::core::to_int64_t(input)};
+  EXPECT_FALSE(result.has_value());
+}
+
+TEST(to_int64_t_trailing_whitespace) {
+  const std::string input{"123 "};
+  const auto result{sourcemeta::core::to_int64_t(input)};
+  EXPECT_FALSE(result.has_value());
+}
+
 TEST(to_int64_t_base8_out_of_range) {
   const std::string input{"7777777777777777777777777"};
   const auto result{sourcemeta::core::to_int64_t(input, 8)};

test/yaml/yaml_parse_test.cc

@@ -1358,6 +1358,26 @@ TEST(lowercase_hexadecimal_integer_is_parsed) {
   EXPECT_FALSE(result.at("key").is_real());
 }
 
+// A digit outside the indicated base makes the whole scalar fail to resolve as
+// an integer, so it stays a string rather than becoming the prefix that parsed
+TEST(octal_integer_with_invalid_digit_stays_a_string) {
+  const std::string input{"key: 0o18"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "key": "0o18" })JSON")};
+  EXPECT_EQ(result, expected);
+  EXPECT_TRUE(result.at("key").is_string());
+}
+
+TEST(hexadecimal_integer_with_invalid_digit_stays_a_string) {
+  const std::string input{"key: 0x1g"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "key": "0x1g" })JSON")};
+  EXPECT_EQ(result, expected);
+  EXPECT_TRUE(result.at("key").is_string());
+}
+
 // YAML 1.2.2 Section 5.1: the printable character set excludes the control
 // block below the space, so a raw control character is rejected.
 TEST(raw_control_character_is_rejected) {
@@ -2220,3 +2240,155 @@ TEST(tab_after_escaped_line_break) {
   EXPECT_TRUE(result.is_string());
   EXPECT_EQ(result.to_string(), "ab");
 }
+
+// YAML 1.2.2 Section 7.1: an alias in key position stands for the value of its
+// anchor, so it collides with an identical key that is already present
+TEST(explicit_key_alias_duplicate_is_rejected) {
+  const std::string input{"? &anchor foo\n: 1\n? *anchor\n: 2"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "foo");
+    EXPECT_STREQ(error.what(), "Duplicate key in YAML mapping");
+    EXPECT_EQ(error.line(), 3);
+    EXPECT_EQ(error.column(), 3);
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(explicit_key_alias_duplicate_after_plain_key_is_rejected) {
+  const std::string input{"x: 1\n? &anchor foo\n: 2\n? *anchor\n: 3"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "foo");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(explicit_key_alias_resolves_to_its_anchor_value) {
+  const std::string input{"x: &a zzz\n? *a\n: 2"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "x": "zzz", "zzz": 2 })JSON")};
+  EXPECT_EQ(result, expected);
+}
+
+TEST(explicit_key_alias_resolves_when_mapping_starts_with_it) {
+  const std::string input{"? &a foo\n: 1\n? *a\n"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "foo");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(explicit_key_alias_to_unknown_anchor_is_rejected) {
+  const std::string input{"x: 1\n? *missing\n: 2"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLUnknownAnchorError &error) {
+    EXPECT_EQ(error.anchor(), "missing");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+// An anchor names the resolved node, so aliasing a key whose text differs from
+// the member name it resolves to still collides with that member name
+TEST(explicit_key_alias_duplicate_through_resolved_key_is_rejected) {
+  const std::string input{"? &a 0x1\n: 1\n? *a\n: 2"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "1");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(explicit_key_alias_duplicate_through_null_key_is_rejected) {
+  const std::string input{"? &a ~\n: 1\n? *a\n: 2"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+// The same anchor used as a value yields the typed node, not the member name
+TEST(explicit_key_anchor_aliased_as_a_value_keeps_its_type) {
+  const std::string input{"? &a 0x1\n: 1\nb: *a"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "1": 1, "b": 1 })JSON")};
+  EXPECT_EQ(result, expected);
+  EXPECT_TRUE(result.at("b").is_integer());
+}
+
+TEST(explicit_key_alias_duplicate_through_resolved_key_after_plain_key) {
+  const std::string input{"x: 1\n? &a 0x1\n: 2\n? *a\n: 3"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "1");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+// An anchor on an implicit key names the resolved key node too, so aliasing it
+// collides with the member name that key produced rather than with its raw text
+TEST(implicit_key_alias_duplicate_through_resolved_key_is_rejected) {
+  const std::string input{"&a 0x1: first\n*a : second"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "1");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(implicit_key_alias_duplicate_is_rejected) {
+  const std::string input{"&a foo: 1\nbar: 2\n*a : 3"};
+  try {
+    sourcemeta::core::parse_yaml(input);
+    FAIL();
+  } catch (const sourcemeta::core::YAMLDuplicateKeyError &error) {
+    EXPECT_EQ(error.key(), "foo");
+  } catch (...) {
+    FAIL();
+  }
+}
+
+TEST(implicit_key_anchor_aliased_as_a_value_keeps_its_type) {
+  const std::string input{"&a 0x1: first\nb: *a"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "1": "first", "b": 1 })JSON")};
+  EXPECT_EQ(result, expected);
+  EXPECT_TRUE(result.at("b").is_integer());
+}
+
+TEST(implicit_key_anchor_on_a_plain_scalar_stays_a_string) {
+  const std::string input{"&a foo: 1\nb: *a"};
+  const auto result{sourcemeta::core::parse_yaml(input)};
+  const sourcemeta::core::JSON expected{
+      sourcemeta::core::parse_json(R"JSON({ "foo": 1, "b": "foo" })JSON")};
+  EXPECT_EQ(result, expected);
+}