search

nytimes / libvmod-queryfilter

Simple querystring filter/sort module for Varnish Cache v3-v6
http://open.blogs.nytimes.com/
Other
17 stars 2 forks source link

Incorrect separator when arrays_enabled is false #6

Closed swla closed 1 year ago

swla commented 3 years ago

If arrays_enabled is false, the separator is not set to '&' after the first parameter unless the first parameter is a flag. The block at lines 261-263 in vmod_queryfilter.c must be moved below the setting of the separator at line 272.

andrew-canaday commented 3 years ago

Oh! This is a great catch! Thank you, @swla!

swla commented 3 years ago

Happy to help :)

stankolubomir commented 2 years ago

Could we know the state of this issue? We just ran into this problem. We will use enabled arrays, but it could be great to fix this because arrays are not always needed in query parameters.

ghost commented 1 year ago

Hey, @stankolubomir there should be a working version in this branch.

The only thing holding up the merge / release is unit tests for the above branch. Contributions are welcome, if you feel inclined! Else, I'll get the tests done and the release cut by end of next week, latest.

Sincerest apologies for the holdup. Truly.

andrew-canaday commented 1 year ago

(Just wanted to reassure: this didn't fall by the wayside. On it. Will post back and close when the release is cut).

ghost commented 1 year ago

@swla / @stankolubomir, this should be fixed. I'll be cutting a new release today diff here.

I ran into some dev complications and subsequently discovered a related bug.

Some temporary docker-based tests are now in place to get around the dev limitations I ran into (also provides some scaffolding for implementing CI for this module in the near future).

Apologies for the holdup. Thanks for hanging in.

ghost commented 1 year ago

v1.0.1 is released!

Going to close. Please feel free to re-open if the issue persists.