feature #19529 Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw except…

…ions on duplicates (Alex Pott) This PR was squashed before being merged into the 3.2-dev branch (closes #19529). Discussion ---------- Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #19526 | License | MIT | Doc PR | Commits ------- cb362f2 Add Yaml::PARSE_EXCEPTION_ON_DUPLICATE to throw exceptions on duplicates
symfony · Aug 9, 2016 · 2faf6f3 · 2faf6f3
2 parents 90f7ff5 + cb362f2
commit 2faf6f3
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 5 deletions.
diff --git a/UPGRADE-3.2.md b/UPGRADE-3.2.md
@@ -70,3 +70,9 @@ Validator
        // ...
    }
    ```
+
+Yaml
+----
+
+ * Support for silently ignoring duplicate keys in YAML has been deprecated and
+   will lead to a `ParseException` in Symfony 4.0.
diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -230,6 +230,9 @@ Yaml
  * The `!!php/object` tag to indicate dumped PHP objects was removed in favor of
    the `!php/object` tag.
 
+ * Duplicate keys in YAML leads to a `ParseException`.
+
+
 Validator
 ---------
 

diff --git a/src/Symfony/Component/Yaml/CHANGELOG.md b/src/Symfony/Component/Yaml/CHANGELOG.md
@@ -10,6 +10,9 @@ CHANGELOG
    Yaml::parse('!php/const:PHP_INT_MAX', Yaml::PARSE_CONSTANT);
    ```
 
+ * Support for silently ignoring duplicate keys in YAML has been deprecated and
+   will lead to a `ParseException` in Symfony 4.0.
+
 3.1.0
 -----
 

diff --git a/src/Symfony/Component/Yaml/Inline.php b/src/Symfony/Component/Yaml/Inline.php
@@ -475,6 +475,8 @@ private static function parseMapping($mapping, $flags, &$i = 0, $references = ar
                         // are processed sequentially.
                         if (!isset($output[$key])) {
                             $output[$key] = $value;
+                        } else {
+                            @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                         }
                         $done = true;
                         break;
@@ -486,6 +488,8 @@ private static function parseMapping($mapping, $flags, &$i = 0, $references = ar
                         // are processed sequentially.
                         if (!isset($output[$key])) {
                             $output[$key] = $value;
+                        } else {
+                            @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                         }
                         $done = true;
                         break;
@@ -499,6 +503,8 @@ private static function parseMapping($mapping, $flags, &$i = 0, $references = ar
                         // are processed sequentially.
                         if (!isset($output[$key])) {
                             $output[$key] = $value;
+                        } else {
+                            @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                         }
                         $done = true;
                         --$i;

diff --git a/src/Symfony/Component/Yaml/Parser.php b/src/Symfony/Component/Yaml/Parser.php
@@ -241,13 +241,17 @@ public function parse($value, $flags = 0)
                         // But overwriting is allowed when a merge node is used in current block.
                         if ($allowOverwrite || !isset($data[$key])) {
                             $data[$key] = null;
+                        } else {
+                            @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                         }
                     } else {
                         $value = $this->parseBlock($this->getRealCurrentLineNb() + 1, $this->getNextEmbedBlock(), $flags);
                         // 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])) {
                             $data[$key] = $value;
+                        } else {
+                            @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                         }
                     }
                 } else {
@@ -256,6 +260,8 @@ public function parse($value, $flags = 0)
                     // But overwriting is allowed when a merge node is used in current block.
                     if ($allowOverwrite || !isset($data[$key])) {
                         $data[$key] = $value;
+                    } else {
+                        @trigger_error(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $key), E_USER_DEPRECATED);
                     }
                 }
                 if ($isRef) {

diff --git a/src/Symfony/Component/Yaml/Tests/Fixtures/YtsSpecificationExamples.yml b/src/Symfony/Component/Yaml/Tests/Fixtures/YtsSpecificationExamples.yml
@@ -1574,6 +1574,7 @@ php: |
   array(
     'fixed' => 1230.15,
   )
+---
 test: Float
 yaml: |
    canonical: 1.23015e+3

diff --git a/src/Symfony/Component/Yaml/Tests/Fixtures/sfMergeKey.yml b/src/Symfony/Component/Yaml/Tests/Fixtures/sfMergeKey.yml
@@ -18,11 +18,7 @@ yaml: |
         x: Oren
         c:
             foo: bar
-            foo: ignore
             bar: foo
-    duplicate:
-        foo: bar
-        foo: ignore
     foo2: &foo2
         a: Ballmer
     ding: &dong [ fi, fei, fo, fam]
@@ -48,7 +44,6 @@ php: |
     array(
         'foo' => array('a' => 'Steve', 'b' => 'Clark', 'c' => 'Brian'),
         'bar' => array('a' => 'before', 'd' => 'other', 'b' => 'new', 'c' => array('foo' => 'bar', 'bar' => 'foo'), 'x' => 'Oren'),
-        'duplicate' => array('foo' => 'bar'),
         'foo2' => array('a' => 'Ballmer'),
         'ding' => array('fi', 'fei', 'fo', 'fam'),
         'check' => array('a' => 'Steve', 'b' => 'Clark', 'c' => 'Brian', 'fi', 'fei', 'fo', 'fam', 'isit' => 'tested'),

diff --git a/src/Symfony/Component/Yaml/Tests/ParserTest.php b/src/Symfony/Component/Yaml/Tests/ParserTest.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\Yaml\Tests;
 
+use Symfony\Bridge\PhpUnit\ErrorAssert;
 use Symfony\Component\Yaml\Yaml;
 use Symfony\Component\Yaml\Parser;
 
@@ -770,6 +771,7 @@ public function testScalarInSequence()
      *
      * @see http://yaml.org/spec/1.2/spec.html#id2759572
      * @see http://yaml.org/spec/1.1/#id932806
+     * @group legacy
      */
     public function testMappingDuplicateKeyBlock()
     {
@@ -789,6 +791,9 @@ public function testMappingDuplicateKeyBlock()
         $this->assertSame($expected, Yaml::parse($input));
     }
 
+    /**
+     * @group legacy
+     */
     public function testMappingDuplicateKeyFlow()
     {
         $input = <<<EOD
@@ -803,6 +808,75 @@ public function testMappingDuplicateKeyFlow()
         $this->assertSame($expected, Yaml::parse($input));
     }
 
+    /**
+     * @dataProvider getParseExceptionOnDuplicateData
+     * @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
+     * throws \Symfony\Component\Yaml\Exception\ParseException in 4.0
+     */
+    public function testParseExceptionOnDuplicate($input, $duplicate_key)
+    {
+        ErrorAssert::assertDeprecationsAreTriggered(sprintf('Duplicate key "%s" detected whilst parsing YAML. Silent handling of duplicates in YAML is deprecated since version 3.2 and will throw \Symfony\Component\Yaml\Exception\ParseException in 4.0.', $duplicate_key), function () use ($input) {
+            Yaml::parse($input);
+        });
+    }
+
+    public function getParseExceptionOnDuplicateData()
+    {
+        $tests = array();
+
+        $yaml = <<<EOD
+parent: { child: first, child: duplicate }
+EOD;
+        $tests[] = array($yaml, 'child');
+
+        $yaml = <<<EOD
+parent:
+  child: first,
+  child: duplicate
+EOD;
+        $tests[] = array($yaml, 'child');
+
+        $yaml = <<<EOD
+parent: { child: foo }
+parent: { child: bar }
+EOD;
+        $tests[] = array($yaml, 'parent');
+
+        $yaml = <<<EOD
+parent: { child_mapping: { value: bar},  child_mapping: { value: bar} }
+EOD;
+        $tests[] = array($yaml, 'child_mapping');
+
+        $yaml = <<<EOD
+parent:
+  child_mapping:
+    value: bar
+  child_mapping:
+    value: bar
+EOD;
+        $tests[] = array($yaml, 'child_mapping');
+
+        $yaml = <<<EOD
+parent: { child_sequence: ['key1', 'key2', 'key3'],  child_sequence: ['key1', 'key2', 'key3'] }
+EOD;
+        $tests[] = array($yaml, 'child_sequence');
+
+        $yaml = <<<EOD
+parent:
+  child_sequence:
+    - key1
+    - key2
+    - key3
+  child_sequence:
+    - key1
+    - key2
+    - key3
+EOD;
+        $tests[] = array($yaml, 'child_sequence');
+
+        return $tests;
+    }
+
     public function testEmptyValue()
     {
         $input = <<<'EOF'