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

Add some sanitization back in #120

Merged
merged 2 commits into from
May 6, 2022
Merged

Add some sanitization back in #120

merged 2 commits into from
May 6, 2022

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented May 2, 2022

Background

In #119, I removed FILTER_SANITIZE_STRING flags for PHP 8.1 compatibility

Changes

  • Add sanitization in 1 case, and remove the variables in 2 cases

Testing instructions

Not needed, just a sanity check would be great

@kienstra kienstra changed the base branch from develop to add/phpunit-8.1 May 2, 2022 19:48
// Enqueue scripts and styles on the edit screen of the Block post type.
if ( $this->slug === $page ) {
if ( filter_input( INPUT_GET, 'page' ) === $this->slug ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this isn't sanitized, there shouldn't be a variable $page that can be used for anything.

filter_input() should just run inline.

@@ -53,7 +53,7 @@ public function validate( $value, $echo ) {
$content = genesis_custom_blocks()->loader->get_data( 'content' );

return empty( $content )
? urldecode( filter_input( INPUT_GET, 'inner_blocks' ) )
? urldecode( wp_strip_all_tags( filter_input( INPUT_GET, 'inner_blocks' ) ) )
Copy link
Contributor Author

@kienstra kienstra May 2, 2022

Choose a reason for hiding this comment

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

wp_strip_all_tags() is similar to what the FILTER_SANITIZE_STRING argument did before I deleted it

$context = filter_input( INPUT_GET, 'context' );

if ( 'edit' === $context ) {
if ( 'edit' === filter_input( INPUT_GET, 'context' ) ) {
Copy link
Contributor Author

@kienstra kienstra May 2, 2022

Choose a reason for hiding this comment

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

Similar to above, because filter_input() isn't sanitizing, it shouldn't be stored in the variable $context, which can be used anywhere.

filter_input() should simply run inline.

@kienstra
Copy link
Contributor Author

kienstra commented May 2, 2022

With this and #119, GCB free looks to be compatible with PHP 8.1.

Though WP 5.9.3 isn't, so there could still be PHP notices, like when installing the plugin.

And Core might have more changes before the 6.0 release for PHP 8.1 compatibility.

@kienstra kienstra marked this pull request as ready for review May 2, 2022 19:55
Base automatically changed from add/phpunit-8.1 to develop May 6, 2022 20:24
@kienstra
Copy link
Contributor Author

kienstra commented May 6, 2022

Hi @dreamwhisper,
Thanks a lot for reviewing this!

@kienstra kienstra merged commit e706c34 into develop May 6, 2022
@kienstra kienstra deleted the update/deprecated-flag branch May 6, 2022 20:25
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