Skip to content

Ref #77388 - disallow passing BAD_ESCAPE_IS_LITERAL, esp by default #4429

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

Closed
wants to merge 2 commits into from

Conversation

SjonHortensius
Copy link
Contributor

@SjonHortensius SjonHortensius commented Jul 17, 2019

this option is considered dangerous and unwanted by the author of pcre:

Absolutely! I'm a bit horrified to learn that somebody set it as a default. It is very much a use-with-care dangerous feature. I hope the PHP developers take note in due course.

While this is a B/C change - it is more likely to expose (existing) problems that can quickly be fixed by fixing the regexp. There were no objections against removing it on php-internals

this option is considered dangerous and unwanted
@nikic
Copy link
Member

nikic commented Jul 17, 2019

It probably makes more sense to target this at PHP 8.

@@ -704,7 +695,6 @@ PHPAPI pcre_cache_entry* pcre_get_compiled_regex_cache(zend_string *regex)
case 'D': coptions |= PCRE2_DOLLAR_ENDONLY;break;
case 'S': /* Pass. */ break;
case 'U': coptions |= PCRE2_UNGREEDY; break;
case 'X': extra_coptions &= ~PCRE2_EXTRA_BAD_ESCAPE_IS_LITERAL; break;
Copy link
Member

@nikic nikic Jul 17, 2019

Choose a reason for hiding this comment

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

I wonder if this should still allow the modifier but ignore it? Like for S.

Copy link
Contributor Author

@SjonHortensius SjonHortensius Jul 17, 2019

Choose a reason for hiding this comment

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

Right - that's not a bad idea.

@SjonHortensius SjonHortensius changed the base branch from PHP-7.4 to master July 17, 2019 09:47
@SjonHortensius
Copy link
Contributor Author

while github allows changing the base - this only really works if I had made a feature branch.
I will do that now and create a new PR and close this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants