[Yaml] Fixed YAML Parser does not ignore duplicate keys, violating YAML spec. #10902
Conversation
|
To my knowledge, this turns Symfony's YAML parser into the first of all PHP implementations that does not violate the spec on the aspect of duplicate keys. The PECL YAML extension also fails on that requirement. FWIW, Python's PyYAML is struggling with this bug, too. |
|
The reported test failures are not related to this PR:
|
| // Parser cannot abort this mapping earlier, since lines | ||
| // are processed sequentially. | ||
| if (!isset($output[$key])) { | ||
| $output[$key] = $value; |
There was a problem hiding this comment.
please use 4 spaces for the indentation
There was a problem hiding this comment.
Sorry for that; fixed now.
The current [YAML 1.2] specification clearly states: > JSON's RFC4627 requires that mappings keys merely “SHOULD” be unique, while YAML insists they “MUST” be. The outdated [YAML 1.1] spec contained a crystal clear note on how the error of duplicate keys is to be handled by parsers, which is (sadly) no longer contained in the latest 1.2 spec (only leaving the requirement): > It is an error for two equal keys to appear in the same mapping node. In such a case the YAML processor may continue, ignoring the second `key: value` pair and issuing an appropriate warning. This strategy preserves a consistent information model for one-pass and random access applications. [YAML 1.2]: http://yaml.org/spec/1.2/spec.html#id2759572 [YAML 1.1]: http://yaml.org/spec/1.1/#id932806
|
Thanks for fixing this bug @sun. |
…iolating YAML spec. (sun) This PR was merged into the 2.5-dev branch. Discussion ---------- [Yaml] Fixed YAML Parser does not ignore duplicate keys, violating YAML spec. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | **mayhaps** | Deprecations? | no | Tests pass? | yes | Needs merge to | 2.4 | Fixed tickets | — | License | MIT | Doc PR | — The current [YAML 1.2] specification clearly states: > JSON's RFC4627 requires that mappings keys merely “SHOULD” be unique, while YAML insists they “MUST” be. The outdated [YAML 1.1] spec contained a crystal clear note on how the error of duplicate keys is to be handled by parsers, which is (sadly) no longer contained in the latest 1.2 spec (only leaving the requirement): > It is an error for two equal keys to appear in the same mapping node. In such a case the YAML processor may continue, ignoring the second `key: value` pair and issuing an appropriate warning. This strategy preserves a consistent information model for one-pass and random access applications. [YAML 1.2]: http://yaml.org/spec/1.2/spec.html#id2759572 [YAML 1.1]: http://yaml.org/spec/1.1/#id932806 Commits ------- 951acca Fixed YAML Parser does not ignore duplicate keys, violating YAML spec.
|
This is actually a huge BC break. It was a crucial feature to be able to overwrite specific settings later. For example we used YAML aliases and with a following overwrite alot for defining the elasticsearch mapping in Elastica. I don't see how we can solve this now without copying most of the config. |
|
OK I found the problem. This PR introduced bug #11142 |
The current YAML 1.2 specification clearly states:
The outdated YAML 1.1 spec contained a crystal clear note on how the error of duplicate keys is to be handled by parsers, which is (sadly) no longer contained in the latest 1.2 spec (only leaving the requirement):