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.

Sign up for GitHub

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

Jump to bottom

[104] Fix deprecated filter warning #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[104] Fix deprecated filter warning #108

wants to merge 1 commit into from

Conversation

ankitrox
Copy link
Contributor

closes: #104

@ankitrox ankitrox self-assigned this Jun 20, 2022
@github-actions
Copy link

Code coverage report: https://gh-reports.rt.gw/login-with-google/refs/pull/108/merge/

@ankitrox ankitrox requested a review from pdclark June 20, 2022 14:08
pdclark
pdclark suggested changes Jun 21, 2022
View reviewed changes
src/Utils/Helper.php
@@ -87,7 +87,7 @@ public static function filter_input( $type, $variable_name, $filter = FILTER_DEF
* Use the PHP method and bail out.
*/
switch ( $filter ) {
case FILTER_SANITIZE_STRING:
case FILTER_DEFAULT:
Copy link
Contributor

@pdclark pdclark Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Blank filter_var filter or FILTER_DEFAULT, is equivalent to FILTER_UNSAFE_RAW. This will result in no filtering taking place by default. Filtering / sanitization should take place. Is there an issue with FILTER_SANITIZE_STRING in PHP 8.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pdclark FILTER_SANITIZE_STRING has been deprecated in PHP 8.1 and will result in E_WARNING

I think we may have to modify this function to use sanitize_text_field or other appropriate sanitization function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdclark pdclark pdclark requested changes

At least 1 approving review is required to merge this pull request.

Assignees

@ankitrox ankitrox

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Compatibility issue with PHP 8.1
2 participants
@ankitrox @pdclark