bug #57261 [Yaml] Throw on duplicate key even when value is NULL (olsavmic)

fabpot · fabpot · commit 9f11f4638632 · 2024-06-02T21:15:39.000+02:00
This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [Yaml] Throw on duplicate key even when value is NULL | Q | A | ------------- | --- | Branch? | 5.4 (since v3.0) | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Issues | Fix #57262 | License | MIT Duplicate keys are not valid by definition in YAML. The current implementation contains a bug that allows a key to be defined multiple times when the value is not set. ```yaml services: Foo: Bar: Foo: ``` Extends symfony/yaml@8094454 to throw when a key is set twice in YAML without a value. It may be technically a breaking change (as it suddenly makes some yaml-like files invalid), even though I'd classify it as a bugfix (as by definition, such files were not valid). If we classify it as a bug, we should probably backport the fix to the oldest maintained version. Commits ------- f9df19b [Yaml] Throw on duplicate key even when value is NULL
diff --git a/src/Symfony/Component/Yaml/CHANGELOG.md b/src/Symfony/Component/Yaml/CHANGELOG.md
@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+7.2
+---
+
+ * duplicate mapping keys throw a `ParseException` even when such key has a NULL value
+
 7.1
 ---
 
diff --git a/src/Symfony/Component/Yaml/Parser.php b/src/Symfony/Component/Yaml/Parser.php
@@ -299,7 +299,7 @@ private function doParse(string $value, int $flags): mixed
                     if (!$this->isNextLineIndented() && !$this->isNextLineUnIndentedCollection()) {
                         // Spec: Keys MUST be unique; first one wins.
                         // But overwriting is allowed when a merge node is used in current block.
-                        if ($allowOverwrite || !isset($data[$key])) {
+                        if ($allowOverwrite || !\array_key_exists($key, $data)) {
                             if (null !== $subTag) {
                                 $data[$key] = new TaggedValue($subTag, '');
                             } else {
@@ -320,7 +320,7 @@ private function doParse(string $value, int $flags): mixed
                             }
 
                             $data += $value;
-                        } elseif ($allowOverwrite || !isset($data[$key])) {
+                        } elseif ($allowOverwrite || !\array_key_exists($key, $data)) {
                             // Spec: Keys MUST be unique; first one wins.
                             // But overwriting is allowed when a merge node is used in current block.
                             if (null !== $subTag) {
@@ -336,7 +336,7 @@ private function doParse(string $value, int $flags): mixed
                     $value = $this->parseValue(rtrim($values['value']), $flags, $context);
                     // Spec: Keys MUST be unique; first one wins.
                     // But overwriting is allowed when a merge node is used in current block.
-                    if ($allowOverwrite || !isset($data[$key])) {
+                    if ($allowOverwrite || !\array_key_exists($key, $data)) {
                         $data[$key] = $value;
                     } else {
                         throw new ParseException(sprintf('Duplicate key "%s" detected.', $key), $this->getRealCurrentLineNb() + 1, $this->currentLine);
diff --git a/src/Symfony/Component/Yaml/Tests/ParserTest.php b/src/Symfony/Component/Yaml/Tests/ParserTest.php
@@ -1026,6 +1026,14 @@ public static function getParseExceptionOnDuplicateData()
 EOD;
         $tests[] = [$yaml, 'child_sequence', 6];
 
+        $yaml = <<<EOD
+parent:
+  child:
+  child2:
+  child:
+EOD;
+        $tests[] = [$yaml, 'child', 4];
+
         return $tests;
     }
 
