Skip to content
Snippets Groups Projects

Issue #3281209: Test filters and searching with strings that need URI encoding

Issue #3281209: Test filters and searching with strings that need URI encoding

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
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-Linares
  • I 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 “&”).

  • Harumi Jang changed this line in version 6 of the diff

    changed this line in version 6 of the diff

  • Definitely had some tunnel vision when writing that line. Updated to above.

  • Please register or sign in to reply
  • Harumi Jang added 1 commit

    added 1 commit

    • b5e08a33 - fixed condition and added test case

    Compare with previous version

  • Harumi Jang added 1 commit
  • Harumi Jang added 1 commit

    added 1 commit

    Compare with previous version

  • Harumi Jang
    Harumi Jang @hooroomoo started a thread on commit 4064b337
  • 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;',
  • Harumi Jang added 1 commit

    added 1 commit

    Compare with previous version

  • Harumi Jang added 1 commit

    added 1 commit

    Compare with previous version

  • Harumi Jang added 13 commits

    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

    Compare with previous version

  • Harumi Jang added 1 commit

    added 1 commit

    Compare with previous version

  • Tim Plunkett added 12 commits

    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

    Compare with previous version

  • merged

  • Please register or sign in to reply
    Loading