h5p / h5p-wordpress-plugin

Adds support for H5P Content in WordPress.
https://wordpress.org/plugins/h5p/
71 stars 74 forks source link

FILTER_SANITIZE_STRING is deprecated #152

Open SteelWagstaff opened 1 year ago

SteelWagstaff commented 1 year ago

We run an automated CI/CD pipeline that runs a series of tests. When running the pipeline on a virtual machine running PHP8.1 we get this deprecation warning related to this plugin:

Deprecated: Constant FILTER_SANITIZE_STRING is deprecated in /app/web/app/plugins/h5p/admin/class-h5p-plugin-admin.php on line 547

Looks like https://github.com/h5p/h5p-wordpress-plugin/blob/661bc44f1faef92b2e6312700a0b286b5cbcbdd3/admin/class-h5p-plugin-admin.php#L547 uses FILTER_SANITIZE_STRING which has been deprecated as of PHP 8.1.0. The PHP manual recommends replacement with htmlspecialchars().

See: https://www.php.net/manual/en/filter.filters.sanitize.php and https://www.php.net/manual/en/function.htmlspecialchars.php

richard015ar commented 1 year ago

I noticed eleven occurrences of FILTER_SANITIZE_STRING, included in https://github.com/h5p/h5p-editor-php-library/blob/65d11bc1f64965285455905e6ef227c927341e64/h5peditor-file.class.php#L14.

According to what I understand of the code, the idea is to strip the HTML tags from specific inputs. For example, the following method could replace the FILTER_SANITIZE_STRING flag. I would open a PR but need help finding a helper o common class that can be used across multiple classes to avoid the repetition of this method everywhere.

function filter_input_value(string $string): string {
    $str = preg_replace('/\x00|<[^>]*>?/', '', $string);
     return str_replace(["'", '"'], ["'", '"'], $str);
}
SteelWagstaff commented 1 year ago

@icc are you willing/able to help us identify a helper method or common class? We'd be willing to open a PR for this, if so.

icc commented 1 year ago

I quickly looked over the instances and it seems to me that it should be safe to just replace it with FILTER_UNSAFE_RAW as the input isn't printed anywhere. This should be simpler as well.