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

Prevent an error in WP 4.9.12 from field sanitization #39

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Jan 30, 2020

Steps to reproduce

  1. wp core update --version=4.9.12 --force
  2. Switch to PHP 7.3 if you can
  3. Go to /wp-admin/admin.php?page=cld_global_transformation&tab=global_video_transformations
  4. Expected: The settings appear
  5. Actual: there's a PHP error:

Recoverable fatal error: Object of class Closure could not be converted to string in /Users/ryankienstra/Projects/sites/sitio/wp-includes/formatting.php on line 1045

fatal-error-recoverable

Background

This error is from:

One of the values that is passed to sanitize_text_field() is:

closure-here

sanitize_text_field passes that to _sanitize_text_fields(). That's not a problem in WP 5.0+, I think. It now exits for values like closures.

But in WP 4.9, it didn't yet check the type, and it caused the error here.

This PR should back-port the WP 5.0 functionality to 4.9. There's no expected change in behavior in WP 5.0+.

But this could use good regression testing on the 'Global Transformations' screens, like at /wp-admin/admin.php?page=cld_global_transformation&tab=global_video_transformations

…error

There was a PHP error when that function accepted a closure,
and in 4.9.12, it didn't have a check that exits if the
argument is the wrong type.
So move the check into a function that later
calls sanitize_text_field().
@kienstra
Copy link
Contributor Author

Request For Review

Hi @dugajean,
Hope you're doing great today. Could you please review this when you have a chance?

Thanks, Dugi!

Copy link
Contributor

@dugajean dugajean left a comment

Choose a reason for hiding this comment

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

Looks very good! Thanks Ryan 😄

@dugajean dugajean merged commit 4ab7e7e into develop Jan 30, 2020
@dugajean dugajean deleted the fix/error-in-settings branch January 30, 2020 12:59
@kienstra
Copy link
Contributor Author

Thanks, Dugi!

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