Skip to content

[Yaml] Allow parsing of unindented closing quotation mark#33119

Closed
johnillo wants to merge 2 commits intosymfony:4.3from
johnillo:fix/yaml-unindented-close-quote
Closed

[Yaml] Allow parsing of unindented closing quotation mark#33119
johnillo wants to merge 2 commits intosymfony:4.3from
johnillo:fix/yaml-unindented-close-quote

Conversation

@johnillo
Copy link
Copy Markdown
Contributor

@johnillo johnillo commented Aug 12, 2019 •
edited by javiereguiluz
Loading

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #33082
License MIT
Doc PR symfony/symfony-docs#...

Unindented quotation marks are not parsed correctly by the parser. Online parsers accepts values/strings with unindented quotation marks as valid yaml.

$yaml = <<<YAML
---
foo:
  bar: 'baz

'
YAML;
dump($this->parser->parse($yaml));

Before fix

Symfony\Component\Yaml\Exception\ParseException: Malformed inline YAML string: 'baz at line 4.

After fix

array:1 [
  "foo" => array:1 [
    "bar" => "baz\n"
  ]
]

@johnillo johnillo requested a review from xabbuh as a code owner August 12, 2019 03:52
@johnillo johnillo changed the title fix(yml): allow parsing of unindented closing quotation mark. [Yaml] Allow parsing of unindented closing quotation mark Aug 12, 2019
@xabbuh xabbuh added the Yaml label Aug 12, 2019
@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Aug 12, 2019
@javiereguiluz
Copy link
Copy Markdown
Member

javiereguiluz commented Aug 12, 2019

@johnillo thanks for contributing this fix!

I have a question about the number of \n in the parsed result. Shouldn't we get two \n instead of one?

image

@nicolas-grekas
Copy link
Copy Markdown
Member

nicolas-grekas commented Aug 12, 2019

@javiereguiluz:

image

@xabbuh
Copy link
Copy Markdown
Member

xabbuh commented Aug 12, 2019

If I do not miss anything, the following YAML string would still not be parsed:

foo:
  bar: 'baz

foobar'

This is due to the fact that the parser does not properly handle the inline notation when split across multiple lines.

From my point of view I would rather invest some time to try to add support for the multi-line notation spanning multiple lines entirely instead of solving it only partially.

@javiereguiluz
Copy link
Copy Markdown
Member

javiereguiluz commented Aug 12, 2019

About the missing \n, I guess it's related to the "line folding" feature of the YAML spec: "When encountering a sequence of (possibly indented) line breaks without any additional intermediate characters, the parser ignores the first one and preserves the rest."

@GuilhemN
Copy link
Copy Markdown
Contributor

GuilhemN commented Aug 14, 2019

I don't think this is valid Yaml, the spec explicitly refers to the indentation of each line when defining quoted scalars ( https://yaml.org/spec/1.1/current.html#l-single-inner(n) ):

s-ignored-prefix(n,single)
( ns-single-char nb-single-char* )?

And s-ignored-prefix imposes parsing n white spaces before any content.

@fabpot
Copy link
Copy Markdown
Member

fabpot commented Aug 21, 2019

For the record, we don't thrive to support all the YAML spec. And I agree with @xabbuh that if we were going to add support for this new feature, it should be complete. I don't think this is worth it. @johnillo Do you want to give it a try?

@johnillo
Copy link
Copy Markdown
Contributor Author

johnillo commented Aug 24, 2019

@fabpot I think I have to agree with you that it's not worth it. I've tried different parsers online and they behave differently on this kind of input.

I'll close this PR. Thanks everyone.

@johnillo johnillo closed this Aug 24, 2019
@maxbeckers maxbeckers mentioned this pull request Oct 28, 2022
Merged
fabpot added a commit that referenced this pull request Feb 3, 2024
@fabpot
feature #48022 [Yaml] Fix Yaml Parser with quote end in a new line (m…
40a2cfb 
…axbeckers)

This PR was submitted for the 5.4 branch but it was merged into the 7.1 branch instead.

Discussion
----------

[Yaml] Fix Yaml Parser with quote end in a new line

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #33082
| License       | MIT
| Doc PR        | N/A

This is a fix for issue #33082.

The bug described in the ticket breaks on a ending quote in a new line:
```
foo:
  bar: 'baz

'
  baz: 'Lorem'
```
Before the fix:
`Symfony\Component\Yaml\Exception\ParseException: Malformed inline YAML string: 'baz at line 4.`

There was already a PR #33119, which was closed because of problems.

Commits
-------

21cec3f [Yaml] Fix Yaml Parser with quote end in a new line
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xabbuh xabbuh Awaiting requested review from xabbuh xabbuh is a code owner

Assignees

No one assigned

Labels

Bug Status: Needs Review Yaml

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

7 participants

@johnillo @javiereguiluz @nicolas-grekas @xabbuh @GuilhemN @fabpot @carsonbot