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

This appears to fix #453, #498, #499 and possibly more issues. #567

Merged
merged 3 commits into from
Apr 13, 2020

Conversation

wisskid
Copy link
Contributor

@wisskid wisskid commented Jan 27, 2020

Will need further testing and unit tests.

Using the @-operator will supress errors generated in code wrapped in the isset() call. That's not good. Better than what we have now, but not ideal.

…eed further testing and unit tests.
@wisskid
Copy link
Contributor Author

wisskid commented Jan 28, 2020

Another approach would be to improve the regex that tries to determine whether $p is a variable. This might do it:
^\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*((->)[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*|\[[^\]]*\])*$

@wisskid
Copy link
Contributor Author

wisskid commented Jan 28, 2020

Slight improvement to balance out brackets with nested brackets: ^\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*((->)[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*|\[.*]*\])*$

@uwetews
Copy link
Contributor

uwetews commented Jan 28, 2020

Please note that the issues at handling isset() and empty() calls can get much more complex.
If the variable tested is an object its properties can be variable.
Array indexes may contain variables.
You will get runtime errors when some of this subvariables are undefined.
I was working on a solution before I got ill last year.
I should be able to work on this problem in about 2-3 weeks

@wisskid wisskid requested a review from uwetews January 28, 2020 11:40
@wisskid
Copy link
Contributor Author

wisskid commented Jan 28, 2020

@uwetews great to hear you are coming back! I was thinking a truly proper solution would require lexing/parsing the parameter to ensure it's a valid php variable instead of using a regex. But my solution posted above seems pretty effective too.
I'd love to hear your thoughts. Get well soon!

@wisskid wisskid marked this pull request as ready for review January 28, 2020 11:40
@wisskid
Copy link
Contributor Author

wisskid commented Feb 3, 2020

@uwetews a similar fix in libs/plugins/modifiercompiler.default.php would fix #336

@wisskid wisskid merged commit 4396351 into master Apr 13, 2020
@wisskid wisskid deleted the bugfix/498_error_in_replacing_isset_with_null_check branch April 13, 2020 20:33
@glensc
Copy link
Contributor

glensc commented May 6, 2020

please note that you can't really get by pattern match accurate result whether
empty() argument is non-variable.

see discussion and example here:

@wisskid wisskid restored the bugfix/498_error_in_replacing_isset_with_null_check branch August 6, 2023 23:54
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.

3 participants