Throw FormatException for recursive collections (#2376)

Closes #2375, closes #2373

The semantics of Maps with recursive keys are defined in YAML but
incompatible with Dart. In YAML a key which is itself a map uses
deep/structural equality for it's identity. In Dart a key used in a Map
must have a temporally consistent `hashCode`, but at the moment a map is
inserted into itself as a key that hashCode would need to change.

Since we cannot cleanly handle the full semantics, and since we do not
need the ability to recursively nest collections for any Dart or Flutter
uses of the package, we prefer to not support them at all than to
support them with subtly inconsistent semantics.

Author: natebosch

pkgs/yaml/CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 3.1.4-wip
 
-* Fix a stack overflow in `deepHashCode` when handling self-referential lists.
+* Throw a `FormatException` when parsing self-referential collections instead of
+  a `StackOverflow`. Yaml definitions with collections nested within themselves
+  are unsupported.
 
 ## 3.1.3
 

pkgs/yaml/lib/src/loader.dart

@@ -29,6 +29,9 @@ class Loader {
   /// Aliases by the alias name.
   final _aliases = <String, YamlNode>{};
 
+  /// Anchors that are currently being resolved.
+  final _activeAnchors = <String>{};
+
   /// The span of the entire stream emitted so far.
   FileSpan get span => _span;
   FileSpan _span;
@@ -103,7 +106,13 @@ class Loader {
   /// Composes a node corresponding to an alias.
   YamlNode _loadAlias(AliasEvent event) {
     var alias = _aliases[event.name];
-    if (alias != null) return alias;
+    if (alias != null) {
+      if (_activeAnchors.contains(event.name)) {
+        throw YamlException(
+            'Self-referential collections are not supported.', event.span);
+      }
+      return alias;
+    }
 
     throw YamlException('Undefined alias.', event.span);
   }
@@ -135,10 +144,16 @@ class Loader {
     var node = YamlList.internal(children, firstEvent.span, firstEvent.style);
     _registerAnchor(firstEvent.anchor, node);
 
-    var event = _parser.parse();
-    while (event.type != EventType.sequenceEnd) {
-      children.add(_loadNode(event));
+    Event event;
+    try {
+      if (firstEvent.anchor case final anchor?) _activeAnchors.add(anchor);
       event = _parser.parse();
+      while (event.type != EventType.sequenceEnd) {
+        children.add(_loadNode(event));
+        event = _parser.parse();
+      }
+    } finally {
+      if (firstEvent.anchor case final anchor?) _activeAnchors.remove(anchor);
     }
 
     setSpan(node, firstEvent.span.expand(event.span));
@@ -157,16 +172,22 @@ class Loader {
     var node = YamlMap.internal(children, firstEvent.span, firstEvent.style);
     _registerAnchor(firstEvent.anchor, node);
 
-    var event = _parser.parse();
-    while (event.type != EventType.mappingEnd) {
-      var key = _loadNode(event);
-      var value = _loadNode(_parser.parse());
-      if (children.containsKey(key)) {
-        throw YamlException('Duplicate mapping key.', key.span);
-      }
-
-      children[key] = value;
+    Event event;
+    try {
+      if (firstEvent.anchor case final anchor?) _activeAnchors.add(anchor);
       event = _parser.parse();
+      while (event.type != EventType.mappingEnd) {
+        var key = _loadNode(event);
+        var value = _loadNode(_parser.parse());
+        if (children.containsKey(key)) {
+          throw YamlException('Duplicate mapping key.', key.span);
+        }
+
+        children[key] = value;
+        event = _parser.parse();
+      }
+    } finally {
+      if (firstEvent.anchor case final anchor?) _activeAnchors.remove(anchor);
     }
 
     setSpan(node, firstEvent.span.expand(event.span));

pkgs/yaml/test/yaml_test.dart

@@ -1122,19 +1122,15 @@ void main() {
       expect(anchorList, same(aliasList));
     });
 
-    test('self-referential list does not cause stack overflow in deepHashCode',
-        () {
-      var doc = loadYaml(cleanUpLiteral('''
+    test('self-referential list throws FormatException', () {
+      expect(() => loadYaml(cleanUpLiteral('''
         ? &anchor [*anchor]
-        : value'''));
-      expect(doc, isNotNull);
-      var key = doc.keys.first;
-      expect(key, isA<YamlList>());
-      expect(key[0], same(key));
-
-      var map = deepEqualsMap();
-      map[key] = 'value';
-      expect(map[key], 'value');
+        : value''')), throwsA(isA<FormatException>()));
+    });
+
+    test('self-referential map throws FormatException', () {
+      expect(() => loadYaml('&map { *map : *map }'),
+          throwsA(isA<FormatException>()));
     });
 
     test('[Example 7.1]', () {