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

fix: FILTER_SANITIZE_STRING deprecation #186

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

arzola
Copy link
Contributor

@arzola arzola commented Nov 19, 2024

Problem

PHP Deprecated: Constant FILTER_SANITIZE_STRING is deprecated

Solution

Replace current deprecated code with a suitable option, keeping current functionality intact

Address #152 #170

Input Sanitization Improvements:

  • Introduced a new H5PUtils trait that includes a sanitize_input method to replace the deprecated FILTER_SANITIZE_STRING filter. (admin/class-h5p-utils.php)
  • Updated multiple methods in H5PContentAdmin to use the new sanitize_input method for sanitizing input variables. (admin/class-h5p-content-admin.php) [1] [2] [3] [4] [5] [6] [7]
  • Updated multiple methods in H5PLibraryAdmin to use the new sanitize_input method for sanitizing input variables. (admin/class-h5p-library-admin.php) [1] [2]
  • Updated the alter_title method in H5PPluginAdmin to use htmlspecialchars for sanitizing the page input variable. (admin/class-h5p-plugin-admin.php)

Code Organization:

  • Added the H5PUtils trait to the autoloader to ensure it is properly loaded. (autoloader.php)
  • Included the H5PUtils trait in the H5PContentAdmin and H5PLibraryAdmin classes to utilize the new sanitization method. (admin/class-h5p-content-admin.php, admin/class-h5p-library-admin.php) [1] [2]

Replaced with htmlspecialchars
@otacke
Copy link
Contributor

otacke commented Dec 7, 2024

Looks like FILTER_SANITIZE_STRING is used in 10 different spots in total throughout the H5P plugin for WordPress.

@arzola
Copy link
Contributor Author

arzola commented Dec 9, 2024

Good catch @otacke I'll amend my PR

@otacke
Copy link
Contributor

otacke commented Dec 9, 2024

@arzola That comment was not necessarily directed towards you. But if you have time to replace the other occurrences, too, even better. Hopefully H5P Group is going to merge your request in soon.

@arzola
Copy link
Contributor Author

arzola commented Dec 10, 2024

@otacke I created a trait that could be used to centralize some utility functions like this sanitization process.

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