8000 [YAML] Invalid YAML did not throw exception anymore · Issue #22967 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[YAML] Invalid YAML did not throw exception anymore #22967

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
lyrixx opened this issue May 30, 2017 · 9 comments
Closed

[YAML] Invalid YAML did not throw exception anymore #22967

lyrixx opened this issue May 30, 2017 · 9 comments
Labels
Bug Yaml

Comments

@lyrixx
Copy link
Member
lyrixx commented May 30, 2017
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.3.0 
<?php

require __DIR__.'/vendor/autoload.php';


$yaml = <<<EOYAML
hello
    foo: bar
EOYAML;

dump(Symfony\Component\Yaml\Yaml::parse($yaml));

output:

"hello foo: bar"
@magnetik
Copy link
Contributor

I think it's ec593b9 fault

@weaverryan
Copy link
Member

@xabbuh what do you think? Is that actually valid YAML? If so... it seems the multi-line parsing could cause some WTF's (as it's much more likely that this is a mistake)

@xabbuh
Copy link
Member
xabbuh commented May 31, 2017

Yes, this is broken by the multi-line parsing. But I did not find a way to fix it easily yet.

@nicolas-grekas
Copy link
Member

maybe @GuilhemN has an idea here?

@GuilhemN
Copy link
Contributor

That's indeed invalid yaml, keys must be inlined (see http://www.yaml.org/spec/1.2/spec.html#ns-plain(n,c))

@xabbuh
Copy link
Member
xabbuh commented Aug 2, 2017

Oh indeed, @GuilhemN is right here. I didn't intend to change this behaviour when introducing the parsing of multi-line scalars. But as it seems that the YAML we were parsing before was not valid, we can close here now.

@xabbuh xabbuh closed this as completed Aug 2, 2017
@lyrixx
Copy link
Member Author
lyrixx commented Aug 2, 2017

@xabbuh I don't understand you here. It IS invalid. So symfony should throw an exception. It's not the case anymore.

@lyrixx lyrixx reopened this Aug 2, 2017
@xabbuh
Copy link
Member
xabbuh commented Aug 2, 2017

@lyrixx Oh sorry, I read too quickly. Thanks for reopening.

@walterdolce
Copy link
8000

Just stumbled upon this issue as I was about to open the same.

By using the YAML parser 3.3.0 with the following:

try {
    $yaml = Yaml::parse(' &  *  !  |  >  \'  "  %  @  ` #, { asd a;sdasd }-@^qw3');
} catch (\Exception $e) {
   echo $e->getMessage();
}

No exception is thrown. No exception is thrown when the Yaml::PARSE_EXCEPTION_ON_INVALID_TYPE flag is used either.

@antograssiot antograssiot mentioned this issue Mar 3, 2018
Merged
antograssiot added a commit to antograssiot/symfony that referenced this issue Mar 3, 2018
@antograssiot
Fixes symfony#22967
3065764
antograssiot added a commit to antograssiot/symfony that referenced this issue Mar 3, 2018
@antograssiot
Fixes symfony#22967
b6efd4d
fabpot added a commit that referenced this issue Apr 3, 2018
@fabpot
bug #26387 [Yaml] Fix regression when trying to parse multiline (anto…
ea69cc2 
…grassiot)

This PR was squashed before being merged into the 3.4 branch (closes #26387).

Discussion
----------

[Yaml] Fix regression when trying to parse multiline

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |  #22967
| License       | MIT
| Doc PR        |

I think this is the lower impacted branch, it fixes a regression added in the last releases campain some days ago.

As discused on Slack @xabbuh

Commits
-------

e787ecf [Yaml] Fix regression when trying to parse multiline
@fabpot fabpot closed this as completed Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Yaml
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@fabpot @weaverryan @nicolas-grekas @magnetik @lyrixx @xabbuh @walterdolce @GuilhemN @carsonbot
0