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

Sanitization for multicheck + PHP 7.2 #230

Closed
bryan-farris opened this issue Jun 5, 2018 · 3 comments
Closed

Sanitization for multicheck + PHP 7.2 #230

bryan-farris opened this issue Jun 5, 2018 · 3 comments

Comments

@bryan-farris
Copy link

bryan-farris commented Jun 5, 2018

Looks like there's an issue with the of_sanitize_multicheck function when running PHP 7.2.

Here's what it looks like when options get saved and a multicheck is one of the fields: https://cl.ly/1Y0o3R1h042v

It looks like this is the part of the function that is causing the issue:

foreach( $option['options'] as $key => $value ) {
    $output[$key] = false;
}

Going to continue to troubleshoot.

@bryan-farris
Copy link
Author

Just following up here. Looks like this is related to how PHP 7.1+ handles variables that aren't set or initialized as an array. If I change $output = ''; to $output = array(); this seems to resolve the issue.

This also solves the issue reported in #229 as far as I can tell.

@bryan-farris
Copy link
Author

One last follow-up. This is due to $output not being set or initialized as an array. Previous versions of PHP apparently handled this without issue but 7.1+ requires that the array be defined explicity.

You can read more here: https://tehnoblog.org/php-7-1-warnings-illegal-string-offset-cannot-assign-empty-string-to-string-offset-working-fine-in-php-7-0-and-lower/

That said, I'm going to open a pull request for this change, seems to be a pretty easy one.

@bryan-farris
Copy link
Author

Disregard. The latest version of the plugin here on GitHub already has this change. Should've checked there first: https://github.com/devinsays/options-framework-plugin/blob/master/includes/class-options-sanitization.php#L79

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

No branches or pull requests

1 participant