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

Fix for PHP 7, more debugging output #1373

Merged
merged 1 commit into from Oct 19, 2015
Merged

Fix for PHP 7, more debugging output #1373

merged 1 commit into from Oct 19, 2015

Conversation

ghost
Copy link

@ghost ghost commented Oct 16, 2015

Hallo, eine Änderung für PHP7-Komptabilität (preg_replace_callback wird nicht mehr unterstützt), zwei Änderungen zur Ausgabe des ldap_error, was sehr hilfreich sein kann. Erfolgreich getestet unter PHP 7.0.0RC5 (LDAP logins funktionieren).

@scrutinizer-notifier
Copy link

The inspection completed: No new issues

@turnermm
Copy link
Contributor

preg_replace_callback wird nicht mehr unterstützt
This can't be correct. See: http://php.net/manual/en/migration70.changed-functions.php and elsewhere.

@ghost
Copy link
Author

ghost commented Oct 19, 2015

@turnermm: you're correct, I meant preg_replace with /e. The relevant part in your php7 changed functions links above is "preg_replace() function no longer supports "\e" (PREG_REPLACE_EVAL). preg_replace_callback() should be used instead." The LDAP filter code used preg_replace with /e, and that needed to be changed to preg_replace_callback.

return preg_replace_callback(
'/([\x00-\x1F\*\(\)\\\\])/',
function ($matches) {
return "\\".join("", unpack("H2", $matches[1]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm the code changes here from 4 to 2 slashes. That's because it's no longer within the preg_replace substitution?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I also wondered - this is quite confusing. The old code actually has 5 backslashs, which has to be an error, only 4 are meaningful: it's one backslash that is doubly escaped, once for the inner double quotes, once for the outer single quotes.

Removing the outer quotes from '"\"' yields "\", as can be verified through print('"\"').
"\" is equivalent to "", which cannot be easily verified, since print("\") or the like will give a parsing error.

I verified the equality of the old and the new string via evaluating the following expression, which gives TRUE:
preg_replace('/./e', '"\"', 'x') === preg_replace_callback('/./', function ($matches) { return ""; }, 'x');

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay makes sense. I guess ultimately we should have some tests for this function.

@splitbrain
Copy link
Collaborator

👍

splitbrain added a commit that referenced this pull request Oct 19, 2015
Fix for PHP 7, more debugging output
@splitbrain splitbrain merged commit 3d0cf09 into dokuwiki:master Oct 19, 2015
@ghost ghost deleted the php7 branch November 10, 2015 13:57
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.

None yet

4 participants