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

skip test with libpcre2 10.38 #7588

Closed
wants to merge 1 commit into from

Conversation

remicollet
Copy link
Member

With libpcre2 10.38

TEST FAILURE: ../ext/pcre/tests/bug70345.diff --
001+ Warning: preg_split(): Compilation failed: \K is not allowed in lookarounds (but see PCRE2_EXTRA_ALLOW_LOOKAROUND_BSK) at offset 9 in /builddir/build/BUILD/php-8.0.12RC1/ext/pcre/tests/bug70345.php on line 5
     bool(false)
     
004+ Warning: preg_match(): Compilation failed: \K is not allowed in lookarounds (but see PCRE2_EXTRA_ALLOW_LOOKAROUND_BSK) at offset 12 in /builddir/build/BUILD/php-8.0.12RC1/ext/pcre/tests/bug70345.php on line 9
005+ NULL
003- Warning: preg_match(): Get subpatterns list failed in %s on line %d
004- array(0) {
005- }

This is for 7.4
Probably a better fix is possible in 8.1+ defined some new constant to manage this \K modifier using PCRE2_EXTRA_ALLOW_LOOKAROUND_BSK

@remicollet remicollet requested a review from cmb69 October 18, 2021 08:28
@nikic
Copy link
Member

nikic commented Oct 18, 2021

Setting the flag seems like the right option, to preserve BC.

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Maybe set the flag for 8.1 and earlier, but not for master? According to the PCRE2 docs it is "somewhat dangerous".

@remicollet
Copy link
Member Author

Maybe set the flag for 8.1 and earlier, but not for master? According to the PCRE2 docs it is "somewhat dangerous".

I agree, but we perhaps also have to expose this flag to users ?

@nikic
Copy link
Member

nikic commented Oct 19, 2021

Does anyone know what the "somewhat dangerous" there actually means?

@cmb69
Copy link
Member

cmb69 commented Oct 19, 2021

Does anyone know what the "somewhat dangerous" there actually means?

PCRE2Project/pcre2#4 (comment) explains it.

@nikic
Copy link
Member

nikic commented Oct 20, 2021

Okay, so the problem is that some uses of pcre2_match() in conjunction with this feature may result in an infinite loop -- the question would then be whether this affects our particular usage.

@remicollet
Copy link
Member Author

See pr #7602 (7.4) and pr #7603 (8.0+)

@remicollet remicollet closed this Oct 21, 2021
@remicollet remicollet deleted the issue-pcre1038 branch October 21, 2021 08:41
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.

4 participants