When the searchterm contains a colon, it will not work

The query string uses following format
?f[0]=key:value
if value (and prolly also key, did not test) contains a colon, it is broken.

This will need tests too

CommentFileSizeAuthor
#9 query_string_url-2640746-9.patch3.18 KBborisson_
#9 interdiff.txt2.51 KBborisson_
#6 query_string_url-2640746-6.patch5.69 KBborisson_
#3 query_string_url-2640746-3.patch4.59 KBborisson_

Comments

StryKaizer created an issue. See original summary.

strykaizer’s picture

strykaizer
Primary language Dutch
Location Belgium
commented
Issue summary: View changes
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Active » Needs review
StatusFileSize
new query_string_url-2640746-3.patch4.59 KB

This breaks in wonderfully different ways. We can add validation for a colon in the key quite easily, but because that's actually very easy to override I'm not sure if that's the best path forward. (http://cgit.drupalcode.org/facets/commit/tests/facets_query_processor/sr...).

We should discuss how to fix this.

Status: Needs review » Needs work

The last submitted patch, 3: query_string_url-2640746-3.patch, failed testing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Issue tags: +Needs committer feedback

We should discuss how to resolve this issue.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs work » Needs review
StatusFileSize
new query_string_url-2640746-6.patch5.69 KB

Fixes validation for key.

The added test for failing values still fails, we still have to figure out how to do that. We could do that in a follow-up though.

Status: Needs review » Needs work

The last submitted patch, 6: query_string_url-2640746-6.patch, failed testing.

The last submitted patch, 6: query_string_url-2640746-6.patch, failed testing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Title: Query string url processor can not handle colons » Query string url processor can't handle a colon in the key.
Status: Needs work » Needs review
Issue tags: -Needs committer feedback
StatusFileSize
new interdiff.txt2.51 KB
new query_string_url-2640746-9.patch3.18 KB

I think we should split up the key/value part of this issue and add a new one for the values, this way we can commit the current patch. This makes the current url at least somewhat less fragile.

I moved the other part of this issue to #2656010: Query string url processor can't handle a colon in the value.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Related issues: +#2656010: Query string url processor can't handle a colon in the value.

Committed,

  • borisson_ committed 82bca25 on 8.x-1.x 
    Issue #2640746 by borisson_, StryKaizer: Query string url processor can'...
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Fixed
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.