-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Replace deprecated filter FILTER_SANITIZE_STRING
#5540
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
Conversation
Replaced with FILTER_SANITIZE_FULL_SPECIAL_CHARS. Equivalent to calling htmlspecialchars() with ENT_QUOTES set.
Replaced with FILTER_SANITIZE_FULL_SPECIAL_CHARS. Equivalent to calling htmlspecialchars() with ENT_QUOTES set.
Good catch. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this PR are not inconsistent.
Please make the same changes in other places.
@kenjis changing to striptags() requires the underlying functions to be changed and removes flexibility for developers. |
Sorry, what do you mean by removes flexibility ? First of all, input filtering like FILTER_SANITIZE_STRING is a incorrect practice. |
Developers now have the option to use a different filter. If we force htmlspecialchars() the dev doesn't have that flexibility. FILTER_SANITIZE_FULL_SPECIAL_CHARS is the equivalent of htmlspecialchars(), but keeps the flexibility. |
@TimZ99 Thank you for the explanation. I don't mean to force How about @paulbalandan What do you think? |
If this PR's solution is way better and deviates from the current incorrect behavior, I'd approve this. So, I don't mind if the previously replaced places by |
I searched the source code and found only one I would like to deprecate the param. Because it is a bad practice. Input filtering like And when you output the filtered value as HTML, you don't escape ( |
I think there are three types of
2 or 3 is a bad practice. I don't recommend to use. 2 is to specify filter itself. If a user uses FILTER_SANITIZE_STRING, it is not framework responsibility. 1 is a security measure. I'm not sure the way is good practice or not, but some measures must be needed. |
My conclusion on this PR.
|
@TimZ99 Thank you for your contribution. |
Description
As of PHP8.1
FILTER_SANITIZE_STRING
is deprecated. Replaced it withFILTER_SANITIZE_FULL_SPECIAL_CHARS
. This is equivalent to calling htmlspecialchars() with ENT_QUOTES set. (PHP.net)Checklist: