8000 Bug in simple YAML references · Issue #10888 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Bug in simple YAML references #10888

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
tylercollier opened this issue May 12, 2014 · 9 comments
Closed

Bug in simple YAML references #10888

tylercollier opened this issue May 12, 2014 · 9 comments
Labels
Yaml

Comments

@tylercollier
Copy link

I think I've found a bug with YAML reference parsing. Referencing multiple times results in grabbing data below the referenced section. I'm using the Symfony YAML component from version 2.4.4 with PHP 5.4.23 on Ubuntu 12.04.

Given myfile.yaml:

section: &section1
    x: y
different:
    a: b
section: &section2
    << : *section1
section:
    << : *section2

Code:

print_r(Yaml::parse('myfile.yaml'));

Expected output:

[section] => Array
    (
        [x] => y
    )

[different] => Array
    (
        [a] => b
    )

Actual output:

[section] => Array
    (
        [a] => b
    )

[different] => Array
    (
        [a] => b
    )

Note how [x] => y has been replaced with [a] => b.

Note that you get the same actual output even if you remove the line x: y.
@jakzal jakzal added the Yaml label May 12, 2014
@tylercollier
Copy link
Author

I'm trying to debug this myself, but can anyone else confirm the bug?

@tylercollier
Copy link
Author

I've figured out the problem. However, in the 2.5 branch (note that I discovered t 8000 his problem in 2.4.4), I see this ParserTest.php has a test named testMappingDuplicateKeyBlock. The docblock states:

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.

I'm surprised. My interpretation of this makes me think that keys should be unique. Following the link provided by the docblock, the page states:

YAML insists they “MUST” be.

Thus, perhaps I didn't find a bug. Because I'm redefining keys. I think it would be better though if the parser issued a warning/error, rather than output the wrong thing.

I have a use case for this in my project. I effectively concatenate YAML files. I start with a base YAML file and override keys in subsequent files as needed, which allows for a default configuration and overrides at a more localized level. Thus, key overrides should be allowed, and I'm asking for a spec change to YAML, not a bug fix.

I will address this in my own fork of this repo. I'll provide a link here after I've done that.

@tylercollier
Copy link
Author

It's 3 lines of code, not including the version/tag changes.

See tylercollier@31d3ca7.

@tylercollier
Copy link
Author

Please ignore this post, as it's incorrect. See following post.

The above link is for my fork of symfony/symfony. If you want the fork of just the yaml component, see https://github.com/tylercollier/Yaml/tree/2.4.4-override-keys.

You can use my fork via composer with the following:

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/tylercollier/yaml"
        }
    ],
    "require": {
        "tylercollier/yaml": "dev-2.4.4-override-keys"
    }
}

@tylercollier
Copy link
Author

Please ignore my composer.json above. It's not correct. If I get it corrected, I'll update it here. At this moment, I've changed the require section from tylercollier/yaml to symfony/yaml, but then I get dependency issues with phpunit:

$ sudo php composer.phar update symfony/yaml
Loading composer repositories with package information
Updating dependencies (including require-dev)              
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - phpunit/phpunit 3.7.35 requires symfony/yaml ~2.0 -> satisfiable by symfony/yaml[2.0.x-dev, 2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev].
    - phpunit/phpunit 3.7.35 requires symfony/yaml ~2.0 -> satisfiable by symfony/yaml[2.0.x-dev, 2.1.x-dev, 2.2.x-dev, 2.3.x-dev, 2.4.x-dev, 2.5.x-dev, 2.6.x-dev].
    - Can only install one of: symfony/yaml[2.1.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.2.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.3.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.4.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.5.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.6.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[dev-2.4.4-override-keys, 2.1.x-dev].
    - Can only install one of: symfony/yaml[dev-2.4.4-override-keys, 2.2.x-dev].
    - Can only install one of: symfony/yaml[dev-2.4.4-override-keys, 2.3.x-dev].
    - Can only install one of: symfony/yaml[dev-2.4.4-override-keys, 2.4.x-dev].
    - Can only install one of: symfony/yaml[2.5.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.6.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[2.0.x-dev, dev-2.4.4-override-keys].
    - Can only install one of: symfony/yaml[dev-2.4.4-override-keys, 2.0.x-dev].
    - Installation request for symfony/yaml dev-2.4.4-override-keys -> satisfiable by symfony/yaml[dev-2.4.4-override-keys].
    - Installation request for phpunit/phpunit == 3.7.35.0 -> satisfiable by phpunit/phpunit[3.7.35].

It looks like I can't do all this because phpunit itself directly depends on symfony/yaml. If I figure out how to switch it out for mine, I will update here.

@tylercollier
Copy link
Author

Ok, for now you can use this composer.json. According to the composer alias documentation, it's not a good idea to use inline aliases as a permanent fix, but it'll suffice for your project for now.

Note the as in the "version".

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/tylercollier/yaml"
        }
    ],
    "require": {
        "symfony/yaml":"dev-2.4.4-override-keys as 2.4.x-dev"
    }
}

@Tobion
Copy link
Contributor
Tobion commented Jun 18, 2014

@tylercollier duplicate keys should be ignored according to yaml spec. this has been fixed in #10902 since 2.5. your described behavior in < 2.5 is still strange and buggy.
btw, if you want to overwrite keys, you should use the merge syntax (<<) instead. but it does not currently work, see #11142

@tylercollier
Copy link
Author

To respond, I'll reiterate what I wrote above.

I realize what I'm really asking for is a spec change to YAML. I think my use case of building YAML documents dynamically by concatenating them at run-time is a good example of why this would be helpful. Using the merge syntax is not possible if you're not sure if anchors have previously been defined.

The spec says that if duplicate keys exist, they may be ignored but an appropriate warning should be issued. That doesn't currently happen, and I think it should.

I'm glad it's been fixed in 2.5+, but for anyone who has < 2.5, now you have a way to deal with it with my pull request, if you want my behavior of allowing it. If you wanted to disallow it, you can take a look at the pull request to find out where you'd add code to deal with it.

@Tobion
6BFB Copy link
Contributor
Tobion commented Jun 18, 2014

Closing as it's not relevant in 2.5+ and for 2.3/2.4 it does not matter that much as it's invalid YAML to use the same key (and behavior can be interpreted as undefined).

@Tobion Tobion closed this as completed Jun 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Yaml
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jakzal @tylercollier @Tobion
0