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

[BUG]: Filter::StringVal with <<>> #15978

Closed
niden opened this issue Jun 4, 2022 · 3 comments · Fixed by #15983
Closed

[BUG]: Filter::StringVal with <<>> #15978

niden opened this issue Jun 4, 2022 · 3 comments · Fixed by #15983
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report discussion Request for comments and discussion status: medium Medium

Comments

@niden
Copy link
Sponsor Member

niden commented Jun 4, 2022

FILTER_SANITIZE_STRING is to be removed from PHP 8.1. The StringVal filter relies on filter_var() to sanitize strings.

As per the documentation as well as the relevant RFC that removes the constant, htmlspecialchars() is a better alternative to the functionality.

The current implementation - which no longer uses filter_var is based on preg_match is:

public function __invoke($input)
{
    $input = str_replace(chr(0), "", (string) $input);
    $input = preg_replace("/<[^>]*>?/", "", $input);

    return str_replace(["'", "\""], ["&#39;", "&#34;"], $input);
}

When using the following string:

$input = '<< >> ' ' ' ' " " " " < >  - -  ... '

the expected result is:

' &#39; &#39; &#39; &#39; &#34; &#34; &#34; &#34;   - -  ... '

but instead we get:

'> &#39; &#39; &#39; &#39; &#34; &#34; &#34; &#34;   - -  ... '

Options:

  1. correct the preg_match to pick up the >
  2. Change the StringVal filter to use htmlspecialchars() and also introduce StringValLegacy as a new filter which will only work for PHP 7.4 and 8.0 and will utilize filter_var(). This way we have something that people can use as they have been, and offer a path to upgrade the filter within the next major version.
@niden niden added bug A bug report status: unverified Unverified status: medium Medium 5.0 The issues we want to solve in the 5.0 release discussion Request for comments and discussion and removed status: unverified Unverified labels Jun 4, 2022
@niden niden self-assigned this Jun 4, 2022
@niden niden added this to Working on it in Phalcon Roadmap Jun 4, 2022
@niden
Copy link
Sponsor Member Author

niden commented Jun 4, 2022

I am more inclined to go with no 2.

@Jeckerson
Copy link
Member

@niden OK for option 2.

@niden niden mentioned this issue Jun 8, 2022
5 tasks
@niden niden linked a pull request Jun 8, 2022 that will close this issue
5 tasks
@niden
Copy link
Sponsor Member Author

niden commented Jun 8, 2022

Resolved in #15983

@niden niden closed this as completed Jun 8, 2022
Phalcon Roadmap automation moved this from Working on it to Implemented Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report discussion Request for comments and discussion status: medium Medium
Projects
Archived in project
Phalcon Roadmap
  
Implemented
Development

Successfully merging a pull request may close this issue.

2 participants