Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#20411 fix Yaml parsing for very long quoted strings #21279

Merged
merged 1 commit into from
Jan 14, 2017

Conversation

RichardBradley
Copy link
Contributor

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #20411
License MIT
Doc PR no

This fixes #20411, a YAML string with too many backslash escapes can trigger a PREG_BACKTRACK_LIMIT_ERROR error in the Yaml parser.

There should be no behavioural change other than the bug fix

I have included a test which fails before this fix and passes after this fix.

{
$longStringWithQuotes = str_repeat("x\r\n\\\"x\"x", 1000);

$yamlString = Inline::dump(['longStringWithQuotes' => $longStringWithQuotes]);
Copy link
Member

Choose a reason for hiding this comment

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

the short array syntax does not work on PHP 5.3, which is still supported by Symfony 2.7. Please change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, good point.
I've pushed a new version.

@stof
Copy link
Member

stof commented Jan 13, 2017

👍 (once the PHP 5.3 compat is fixed of course)

@xabbuh
Copy link
Member

xabbuh commented Jan 14, 2017

Thank you @RichardBradley.

@xabbuh xabbuh merged commit 51bca66 into symfony:2.7 Jan 14, 2017
xabbuh added a commit that referenced this pull request Jan 14, 2017
…ardBradley)

This PR was merged into the 2.7 branch.

Discussion
----------

#20411 fix Yaml parsing for very long quoted strings

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

This fixes #20411, a YAML string with too many backslash escapes can trigger a `PREG_BACKTRACK_LIMIT_ERROR` error in the Yaml parser.

There should be no behavioural change other than the bug fix

I have included a test which fails before this fix and passes after this fix.

Commits
-------

51bca66 #20411 fix Yaml parsing for very long quoted strings
@fabpot fabpot mentioned this pull request Jan 28, 2017
This was referenced Feb 6, 2017
fabpot added a commit that referenced this pull request Mar 17, 2017
…ardBradley)

This PR was squashed before being merged into the 2.7 branch (closes #21523).

Discussion
----------

#20411 fix Yaml parsing for very long quoted strings

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

This is a second fix for the issue discussed in #20411. My first PR (#21279) didn't fix the bug in all cases, sorry.

If a YAML string has too many spaces in the value, it can trigger a `PREG_BACKTRACK_LIMIT_ERROR` error in the Yaml parser.

There should be no behavioural change other than the bug fix

I have included a test which fails before this fix and passes after this fix.

I have also added checks that detect other PCRE internal errors and throw a more descriptive exception. Before this patch, the YAML engine would often give incorrect results, rather than throwing, on a PCRE `PREG_BACKTRACK_LIMIT_ERROR` error.

Commits
-------

c9a1c09 #20411 fix Yaml parsing for very long quoted strings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants