Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Yaml] ParseException: pcre.backtrack_limit reached#22067
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| } | ||
| }elseif ( | ||
| // this regexp is backtracking-heavy and makes the parser fail on long strings | ||
| self::preg_match('#^(?P<key>'.Inline::REGEX_QUOTED_STRING.'|(?:![^\s]+\s+)?[^\'"\[\{!].*?) *\:(\s+(?P<value>.+?))?\s*$#u',$this->currentLine,$values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Most quantifiers in this regex could probably become possessive ones actually, to remove backtracking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Changing the regex toself::preg_match('#^(?P<key>'.Inline::REGEX_QUOTED_STRING.'|[^ \'"\[\{!].*?) *+\:(\s++(?P<value>.+?))?\s*+$#u', $this->currentLine, $values) doesn't seem to be enough.
Do you see another possible improvement?
ec69e9d tof0256f1Comparenicolas-grekas commentedMar 22, 2017
Regexp fixed. PR ready. |
| } | ||
| }elseif ( | ||
| self::preg_match('#^(?P<key>'.Inline::REGEX_QUOTED_STRING.'|(?:![^\s]+\s+)?[^\'"\[\{!].*?) *\:(\s+(?P<value>.+?))?\s*$#u',$this->currentLine,$values) | ||
| self::preg_match('#^(?P<key>'.Inline::REGEX_QUOTED_STRING.'|(?:![^\s]++\s++)?[^\'"\[\{!].*?) *\:(\s++(?P<value>.+))?$#u',$this->currentLine,$values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@GuilhemN the issue was the last part of the regexp, the related to the last\s* and the ungreedy quantifier just before.
stof left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
| $allowOverwrite =true; | ||
| if (isset($values['value']) &&0 ===strpos($values['value'],'*')) { | ||
| $refName =substr($values['value'],1); | ||
| $refName =substr(rtrim($values['value']),1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
rtrim($values['value'], ' ')? tabs should not be trimmed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
weren't they previously already, with this\s+$ tail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Indeed, it's legit then 👍
fabpot commentedMar 22, 2017
Thank you@nicolas-grekas. |
…olas-grekas)This PR was merged into the 3.3-dev branch.Discussion----------[Yaml] ParseException: pcre.backtrack_limit reached| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | no| Fixed tickets | -| License | MIT| Doc PR | -while merging 3.2 into master, I noticed that `testCanParseVeryLongValue` is triggering this error on master, due to this regexp that we added for handling yaml tags. This regexp needs to be fixed so that we can merge the test case.ping@GuilhemNCommits-------f0256f1 [Yaml] Fix pcre.backtrack_limit reached
lcobucci commentedMar 27, 2017
@nicolas-grekas@fabpot@stof this change breaks the parser when you a have a trailing space after the colon. Doctrine ORM test suite has it and it started to break after this PR got merged (we use I've discovered that by adding the following to the publicfunctiontestCanParseContentWithTrailingSpaces() {$yaml =<<<YAMLitems: foo: barYAML;$expected = ['items' => ['foo' =>'bar'], ];$this->assertEquals($expected,$this->parser->parse($yaml)); } I'm not sure if having a trailing space is valid according to the YAML syntax rules, however it was working before. |
nicolas-grekas commentedMar 28, 2017
Valid report, please open an issue. |
while merging 3.2 into master, I noticed that
testCanParseVeryLongValueis triggering this error on master, due to this regexp that we added for handling yaml tags. This regexp needs to be fixed so that we can merge the test case.ping@GuilhemN