Skip to content

Fix RegexIterator::USE_KEY related bug #2133

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

Conversation

marcioAlmada
Copy link
Contributor

@marcioAlmada marcioAlmada commented Sep 19, 2016

This is related to #865. Turns out in case USE_KEY flag is active RegexIterator->accept() should keep it's old behavior which is to accept keys mapping arrays:

https://3v4l.org/EmbIN

This broke after PHP 5.5 but was not noticed due to lack of tests for USE_KEY or maybe it was considered undefined behavior. Anyway, it seems that the new behavior is not as reasonable as what we had on v5.4?

Thoughts?

@marcioAlmada marcioAlmada mentioned this pull request Sep 19, 2016
@marcioAlmada
Copy link
Contributor Author

marcioAlmada commented Sep 19, 2016

This behavior is oscillating between minor versions too:

use_key

@@ -2038,7 +2038,7 @@ SPL_METHOD(RegexIterator, accept)

if (Z_TYPE(intern->current.data) == IS_UNDEF) {
RETURN_FALSE;
} else if (Z_TYPE(intern->current.data) == IS_ARRAY) {
} else if (intern->u.regex.flags ^ REGIT_USE_KEY && Z_TYPE(intern->current.data) == IS_ARRAY) {
Copy link
Member

Choose a reason for hiding this comment

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

This should be (intern->u.regex.flags & REGIT_USE_KEY). Alternatively the IS_ARRAY check can be moved into the branch in L2048.

Copy link
Contributor Author

@marcioAlmada marcioAlmada Sep 19, 2016

Choose a reason for hiding this comment

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

E_RUSH :) ended up moving the array check, it's simpler to read. Thanks.

In case USE_KEY flag is active, RegexIterator->accept() should keep it's
old behavior which is to accept keys mapping arrays.

This broke after PHP 5.5 but was not noticed due to lack of tests for USE_KEY.
@marcioAlmada marcioAlmada force-pushed the bug/regex-iterator-USE_KEY branch from 8147f3b to a050565 Compare September 19, 2016 20:14
@nikic
Copy link
Member

nikic commented Sep 22, 2016

Merged via 4397306, thanks!

@nikic nikic closed this Sep 22, 2016
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