friendsofgo / killgrave

Simple way to generate mock servers written in Go
https://friendsofgo.github.io/killgrave/
MIT License
523 stars 100 forks source link

Multi-value query string params #164

Open silverskater opened 8 months ago

silverskater commented 8 months ago

Multi-value query string params are not supported ENDPOINT?sections=90&sections=130

Imposter request:

  {
    "request": {
      "method": "GET",
       "endpoint": "/search",
      "params": {
        "sections": "130"
      }
    },

It only finds the first value i.e. 90, but never the second.

Also tried the PHP-way sections[]=90&sections[]=130 - this does nothing at all, the param would probably be sections[].

Suggestion:

      "params": {
        "sections": ["90","130"]
      }

Or maybe some other way to work with the full query string.

joanlopez commented 8 months ago

Hi @silverskater,

Thanks for your feature request, I definitely think that it would be nice to have support for such cases. However, after a quick look, I realized that the source of the problem is in gorilla/mux, the library we use for routing.

So, although we could easily add support for reading a JSON as you suggested (e.g. "sections": ["90","130"]), I noticed that calling twice r.Queries(k, v) with the same key and different values doesn't match, which means that this would need to be fixed in gorilla/mux first.

Would you mind opening an issue there? Otherwise, I could do it, even try to open a pull request with the fix, but it would take longer, as I don't have much capacity right now.

Thanks! 🙇🏻

silverskater commented 8 months ago

Hi @joanlopez I'm not sure it wouldn't work in gorill/mux. At first look it seems possible. The values are not stored in a HashMap, each call simply adds a new check

r.addRegexpMatcher(pairs[i]+"="+pairs[i+1], regexpTypeQuery);

https://github.com/gorilla/mux/blob/976b536446a77de8de2d5559c78f612970fb5e37/route.go#L391

It might even work like

r.Queries("sections", "90", "sections", "130")

But if I got it wrong I'd be happy to create a new issue for it in gorilla/mux.

joanlopez commented 8 months ago

Hey @silverskater,

Yeah, you can call r.Queries more than once, or just pass multiple key values, as in your example. As you pointed out, it keeps adding regexpTypeQuery matchers (r.addRegexpMatcher(pairs[i]+"="+pairs[i+1], regexpTypeQuery);).

However, the problem is on the "match" side, as it is performed by the following code:

func (r *routeRegexp) matchQueryString(req *http.Request) bool {
    return r.regexp.MatchString(r.getURLQuery(req))
}

https://github.com/gorilla/mux/blob/main/regexp.go#L278-L280

where r.getURLQuery(req) looks like:

// getURLQuery returns a single query parameter from a request URL.
// For a URL with foo=bar&baz=ding, we return only the relevant key
// value pair for the routeRegexp.
func (r *routeRegexp) getURLQuery(req *http.Request) string {
    if r.regexpType != regexpTypeQuery {
        return ""
    }
    templateKey := strings.SplitN(r.template, "=", 2)[0]
    val, ok := findFirstQueryKey(req.URL.RawQuery, templateKey)
    if ok {
        return templateKey + "=" + val
    }
    return ""
}

https://github.com/gorilla/mux/blob/main/regexp.go#L225-L238

And as you can see there, it only looks for the first query key (findFirstQueryKey). Their docs states:

// findFirstQueryKey returns the same result as (*url.URL).Query()[key][0].
// If key was not found, empty string and false is returned.

So, it is impossible to get it working with two different values for the same key. In fact, if you try to build a small example, with your params, you'll notice that, depending on the order of the query parameters, it matches or not even for a single key-value.

For instance, if you only use:

r.Queries("sections", "130")

Then,

So, unless I'm missing something, or getting something wrong, that logic must be changed in gorilla/mux first. Right?

Thanks! 🙇🏻

joanlopez commented 8 months ago

Or maybe some other way to work with the full query string.

After a short discussion with @aperezg, he made me recall that, unless I'm wrong, it should work by specifying the entire URL as the endpoint field in your imposter. Something like:

{
    "request": {
      "method": "GET",
       "endpoint": "/search?sections=90&sections=130",
      }
}

We know it's far from ideal, because it's very dependant on order, and doesn't work very well with other types of matches, and for instance lacks proper support for regular expressions (so I think we should still push for that fix) in gorilla/mux, but it might be enough for your use case.

I hope it helps! 🤞🏻

silverskater commented 8 months ago

Thank you @joanlopez , this information helped me to create a well documented feature request: https://github.com/gorilla/mux/issues/754

Ranveer777 commented 6 months ago

@joanlopez For your reference https://github.com/gorilla/mux/issues/754#issuecomment-2136111198

silverskater commented 6 months ago

Thank you @Ranveer777

@joanlopez gorilla/mux seems to support multiple values, even AND instead of OR.

joanlopez commented 5 months ago

Hey @silverskater,

even AND instead of OR.

Could you bring an example of AND? I cannot fully see how to do so from what's suggested there.

Thanks! 🙇🏻