Ignore unknown query parameters should be ignored in REST calls.

[ls-aron-kyle]

I think it makes sense to either ignore unexpected url parameters or potentially dump them into the UnknownFields of the target message.

The idea being that links to REST calls can be passed through many different proxies, each appending their own query parameters. It's not ideal that any one of these additional fields can cause the request to fail.

[jhump]

We could provide an option to ignore them.

But it is not possible to put them into the messages's unknown fields because those fields are binary-encoded, and it is not possible to translate the query-parameter representation to the binary format without a schema. The binary format needs to know the field number(s) and also needs to know the field's type so it knows how to encode the value. For example, a positive numeric query parameter value that is less than 2^31 could be encoded in one of six ways (varint, zig-zag varint, fixed 32 bits, fixed 64 bits, 32-bit float, or 64-bit float), but only one will be valid and correctly decoded by a reader that knows about the field.

[jhump]

BTW, what proxies blindly append query parameters? That generally seems problematic since other REST handler implementations (other than vanguard-go) could also be strict about how unknown query parameters are handled and whether they are rejected or not.

[ls-aron-kyle]

In my case email services are appending parameters to links we send out.

I'm cool with an option to ignore them. Is this something I have to implement?

[jhump]

I'm cool with an option to ignore them. Is this something I have to implement?

While I think this is a reasonable tbhing to offer an option to ignore them, we don't have bandwidth to implement it right now.

So in the short-term, you could wrap the vanguard handler with your own code, that strips the offending query parameters before the request gets to the vanguard transcoder logic.

[ls-aron-kyle]

It's going to be tough to have to keep doing that for every end point. Do you have an example of a generic way of doing that?

Also, where would you recommend putting the option?

[jhump]

You could use a "middleware" handler -- one that wraps the vanguard handler (so effectively intercepts all endpoints).

// populate this with the query param names that need to be stripped
// (note that they will be case-sensitive)
var paramsToRemove []string

func removeQueryParams(handler http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        queryParams := r.URL.Query()
        for _, k := range paramsToRemove {
            delete(queryParams, k)
        }
       r.URL.RawQuery = queryParams.Encode()
    })
}

// In main or some other function that sets up the HTTP server:
// ...
transcoder := vanguard.NewTranscoder(services)
handler := removeQueryParams(transcoder)
mux.Handle("/", handler)
// ...

[ls-aron-kyle]

That would require knowing in advance what parameters to remove on a per endpoint basis. Even reversing this to remove any unknown parameters is ugly in that you'd have to know which parameters to keep for every endpoint.

I'm curious where you would put options to ignore unknown query parameters?
It looks fairly straight forward as to how implement ignoring them, but I'm unsure about how where to implement the option itself.

[jhump]

That would require knowing in advance what parameters to remove on a per endpoint basis.

Would it? Are you saying that the email services append different parameters based on the target URL/endpoint? That seems a bit odd. Or is your worry that some endpoints may have a legitimate request field with the same name, so you don't want to strip the query parameter from all endpoints because it could break things? (That feels a bit dangerous, for what it's worth, that an endpoint might unintentionally accept a request field from an email-service-added-query-param.)

I'm curious where you would put options to ignore unknown query parameters?

Off the top of my head: maybe a new type, like a struct named RESTUnmarshalOptions. For now, it would have just the single field DiscardUnknownQueryParams bool, but we may want to augment this over time to provide other options relevant to unmarshaling query parameters into request fields.

This could be used via a WithRESTUnmarshalOptions(...) function that returns a ServiceOption. If one wanted to add this to every service, you could use it in conjunction with WithDefaultServiceOptions(...) as a TranscoderOption.

[ls-aron-kyle]

Took a stab at adding the option:

#144