Hi,

The regex for cleaning queries seems flawed, I'll give an example.
My query keys to ignore are:

- s
- key

With this, the following query gets cleaned from:

/nl/zoeken?keys=vet&f%5B0%5D=language%3Anl&advanced-form=1

to:

/nl/zoeken?key&f%5B0%5D=language%3Anl&advanced-form=1

The expected result is that the query should stay the same since both query keys are not present in the query string.

For this I modified the query cleaning part, a patch is provided.

CommentFileSizeAuthor
#6 page_cache_query_ignore-broken_match-3127834-6.patch3.75 KBnterbogt
#5 page_cache_query_ignore-broken_match-3127834-5.patch4.22 KBnterbogt
#3 query_string_cleaning.patch7.2 KBnicasso
#2 query_string_cleaning.patch6.8 KBnicasso
fix_query_string_cleaning.patch6.7 KBnicasso

Comments

Nicasso created an issue. See original summary.

nicasso’s picture

nicasso commented
StatusFileSize
new query_string_cleaning.patch6.8 KB
nicasso’s picture

nicasso commented
StatusFileSize
new query_string_cleaning.patch7.2 KB
nterbogt’s picture

nterbogt commented

Hi Nicasso,

What was the motivation behind pulling this out into a service? I don't see it being something that something outside the module would readily need; and it just adds to the service load of Drupal.

nterbogt’s picture

nterbogt commented
StatusFileSize
new page_cache_query_ignore-broken_match-3127834-5.patch4.22 KB

I would recommend a patch like this. It's a little more robust that using explodes and implodes.
I also wouldn't bother splitting this out into a service.

I've also updated the tests so that this can't become a regression issue.

nterbogt’s picture

nterbogt commented
StatusFileSize
new page_cache_query_ignore-broken_match-3127834-6.patch3.75 KB

Sorry, poor patch.

  • nterbogt authored e186051 on 8.x-1.x 
    Issue #3127834 by Nicasso, nterbogt: Query cleaning regex makes mistakes
nterbogt’s picture

nterbogt commented
Status: Active » Fixed

I went with the second patch, because more testing of ?s[]=a&s[]=b&s[]=c had a different outcome.
Merged to the branch and marked as fixed.

Status: Fixed » Closed (fixed)

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