NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Yaml] fix lexing nested sequences/mappings#33763

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:4.4fromxabbuh:pr-33658

Nov 19, 2020

Merged

[Yaml] fix lexing nested sequences/mappings#33763

fabpot merged 1 commit intosymfony:4.4fromxabbuh:pr-33658

Nov 19, 2020

Conversation

Copy link

Member

xabbuh commentedSep 30, 2019•
edited
Loading

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	#34805,#37788,#37876,#39011,#39013,#39064
License	MIT
Doc PR

xabbuh added this to thenext milestone

Sep 30, 2019

carsonbot added Status: Needs Review Yaml Bug labels

Sep 30, 2019

stof reviewed

Sep 30, 2019

View reviewed changes

src/Symfony/Component/Yaml/Parser.php OutdatedShow resolvedHide resolved

nicolas-grekas modified the milestones:next,4.4

Oct 27, 2019

Copy link

Member

fabpot commentedAug 11, 2020

@xabbuh Is it something you're still willing to work on?

xabbuh mentioned this pull request

Aug 18, 2020

[YAML] Unexpected characters using YAML compact syntax#37876

Closed

simonberger mentioned this pull request

Nov 11, 2020

[Yaml] Fix parsing multidimensional inline collections#39013

Closed

xabbuh force-pushed thepr-33658 branch 6 times, most recently from4279223 to800a703Compare

November 14, 2020 14:51

simonberger reviewed

Nov 14, 2020

View reviewed changes

Copy link

Contributor

simonberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Nicely done.

I just missed a test with strings in mappings/sequences containing brackets which has been fixed in this PR as well.
As a possible test the following part ofinlineNotationSpanningMultipleLinesProvider passed successfully.

            'nested collections containing strings with bracket chars' => [                [['ba[r'], ['[ba]r'], ['bar]'], ['foo' => 'bar{'], ['foo' => 'b{ar}'], ['foo' => 'bar}']],                <<<YAML[    [        "ba[r"    ],    [        '[ba]r'    ],    [        "bar]"    ],    {        foo: "bar{"    },    {        foo: "b{ar}"    },    {        foo: 'bar}'    }]

src/Symfony/Component/Yaml/Parser.php OutdatedShow resolvedHide resolved

xabbuh force-pushed thepr-33658 branch from800a703 toc034cbbCompare

November 15, 2020 17:11

Copy link

MemberAuthor

xabbuh commentedNov 15, 2020•
edited
Loading

Many thanks to@RevZer0,@simonberger and@Matth--. I have added the tests you added in#37876,#39013, and#39064 here.

xabbuh force-pushed thepr-33658 branch 3 times, most recently from728cba9 tof21bd46Compare

November 15, 2020 17:28

Copy link

MemberAuthor

xabbuh commentedNov 15, 2020

This is ready to be reviewed.

xabbuh mentioned this pull request

Nov 15, 2020

[Yaml] Fix sequence with following mapping#39064

Closed

This was linked to issuesNov 15, 2020

YAML parsing error on perfectly valid JSON#39011

Closed

Unexpected characters using YAML compact syntax#37788

Closed

jderusse reviewed

Nov 15, 2020

View reviewed changes

src/Symfony/Component/Yaml/Parser.php OutdatedShow resolvedHide resolved

simonberger approved these changes

Nov 15, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Nov 15, 2020

simonberger suggested changes

Nov 15, 2020

View reviewed changes

Copy link

Contributor

simonberger left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Escaped quotes are not yet skipped inlexInlineQuotedString and are leading to problems for example in this yaml test:

[    ["te\"st"],["test"]]

Another simple error case is:

[    ["te\"st]"]]

carsonbot added Status: Needs Work and removed Status: Reviewed labels

Nov 15, 2020

xabbuh linked an issue

Nov 16, 2020

that may beclosed by this pull request

[YAML] Regression in parsing inline objects since version 4.4#34805

Closed

xabbuh mentioned this pull request

Nov 16, 2020

[YAML] Regression in parsing inline objects since version 4.4#34805

Closed

xabbuh force-pushed thepr-33658 branch 2 times, most recently fromd418d1d to0e38c33Compare

November 17, 2020 12:14

Copy link

Contributor

simonberger commentedNov 17, 2020•
edited
Loading

There is (now) another small issue with escaped backslashes in quotes treated as the escape for the quote.

'escaped backslash in mapping' => [                [['"']],                <<<YAML[    ["\\"]]YAML            ]

I got a segmentation error writing the yam like this:[["\\"]]. l didn't look further in. Should be related to the problem but maybe a boundary check isn't robust enough.

Copy link

MemberAuthor

xabbuh commentedNov 17, 2020•
edited
Loading

Did you mean[["\\\\"]]?[["\\"]] isn't valid YAML and runs into a parsing error for me locally.

Copy link

Contributor

simonberger commentedNov 17, 2020

Depends how it is declared. In HEREDOC[["\\"]]in quotes'[["\\\\"]]'.
There is also for me a parser error when using new lines, but there shouldn't.

Malformed inline YAML string: ""\"] ]]]" at line 3 (near "]")

Copy link

MemberAuthor

xabbuh commentedNov 17, 2020

@simonberger I don't see a difference here (see alsohttps://3v4l.org/iOVjl). Could you please send a PR with a test case that you have in mind to my branch if you still think that something isn't working as expected?