Skip to content

Commit

Permalink
bug symfony#57261 [Yaml] Throw on duplicate key even when value is NU…
Browse files Browse the repository at this point in the history
…LL (olsavmic)

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 <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix symfony#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
  • Loading branch information
fabpot committed Jun 2, 2024
2 parents 02d5fce + f9df19b commit 9f11f46
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
5 changes: 5 additions & 0 deletions src/Symfony/Component/Yaml/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

7.2
---

* duplicate mapping keys throw a `ParseException` even when such key has a NULL value

7.1
---

Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Yaml/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Yaml/Tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 9f11f46

Please sign in to comment.