Issue #3281209: Test filters and searching with strings that need URI encoding
2 open threads
Merge request reports
Activity
added 4 commits
-
7e026d6b...6588b894 - 2 commits from branch
project:1.0.x
- 89e985a6 - added special character projects
- 948058c8 - added search special char test
-
7e026d6b...6588b894 - 2 commits from branch
added 1 commit
- 9fbc6964 - added condition for & in controller and test passes
added 13 commits
-
9fbc6964...1ef0342a - 7 commits from branch
project:1.0.x
- 9d737df8 - added special character projects
- 6b6a3866 - added search special char test
- af1cdf83 - added $ sign
- ab151617 - updated fixture to have ; and <
- 4a68e70a - added condition for & in controller and test passes
- 73506ca6 - updated tests since fixture was updated and new helper was added
Toggle commit list-
9fbc6964...1ef0342a - 7 commits from branch
117 117 118 118 $title = $request->query->get('search'); 119 119 if ($title) { 120 $query['search'] = Xss::filter($title); 120 // Avoid Xss::filter from replacing & with &. 121 $query['search'] = $title == '&' ? $title : Xss::filter($title); This exception makes no sense I think. No title would ever be just
&
, and if it is, it probably shouldn't. This case only covers when the full title is&
and nothing else.Edited by Fran Garcia-LinaresI guess we could “undo” the change, but still do the main XSS filtering
$query['search'] = Xss::filter($title); CHANGE TO $query['search'] = Xss::filter($title); $query['search'] = str_replace('&', '&', $query['search']);
though I’m not even sure we should undo it and just leave it as it is.
We gotta be careful what we allow for. Not filtering at all might be a security risk, and filtering might just fail in some edge cases which we actually might want to ignore (ie: searching for “&”).
changed this line in version 6 of the diff
284 284 $this->click('.dropdown-content #pb-sort > li:nth-child(2)'); 285 285 // Assert that the projects are listed in ascending order of their titles. 286 286 $this->assertProjectsVisible([ 287 '$?Vitamin&C;', 288 287 '&Un:/written', 288 '$?Vitamin&C;', added 13 commits
-
904a27ea...4f6fc58a - 3 commits from branch
project:1.0.x
- e2c3a265 - added special character projects
- 875d3dd2 - added search special char test
- eaad334b - added $ sign
- cd1c6b44 - updated fixture to have ; and <
- 650b4e1d - added condition for & in controller and test passes
- 0c13a52b - updated tests since fixture was updated and new helper was added
- 62278007 - fixed condition and added test case
- 525c45b2 - switch titles in sort
- d5166520 - removed xss:filter uses
- d85a2971 - removed unused import
Toggle commit list-
904a27ea...4f6fc58a - 3 commits from branch
added 12 commits
-
a878797a - 1 commit from branch
project:1.0.x
- be6707bb - added special character projects
- 084d44b0 - added search special char test
- 013b2c86 - added $ sign
- 7ded1ea1 - updated fixture to have ; and <
- 1264117f - added condition for & in controller and test passes
- c79e2bbb - updated tests since fixture was updated and new helper was added
- 04da245a - fixed condition and added test case
- db61730b - switch titles in sort
- f91c82f5 - removed xss:filter uses
- a5d18469 - removed unused import
- 0ed2ab8e - updated test to new name
Toggle commit list-
a878797a - 1 commit from branch
Please register or sign in to reply