support query strings in checks

[dbresson]

resolves #1561

[diptanu]

@dbresson I might be wrong, but since you are using Opaque here instead of Path and RawPath and just setting the check.Path, wouldn't the resultant url be something like http://path?

I think we would want this -

url := url.URL{
    Scheme:    check.Protocol,
    Host:       net.JoinHostPort(host, strconv.Itoa(port)),
    Path:        check.Path,
    RawPath:   check.Path,
}

[dbresson]

I'm pretty sure the hostname and port are not part of the opaque section of a URI. If you're looking at the Go net/url documentation and seeing the opaque example shows the hostname as ignored, that's just because the HTTP request object has Host which overrides the host part of the URL.

[dbresson]

Ugh, I guess I'm wrong, based on this:
https://golang.org/pkg/net/url/#URL.String

[dbresson]

It looks like what we really need is URL.parse on the path, and then use URL.ResolveReference to combine the path and query with the rest of the URL.

[diptanu]

@dbresson What I suggested won't work? From the docs it feels like RawPath which introduced in Go 1.5 is an encoded path hint

[dbresson]

Well, it might work, but I don't think it's right. It wouldn't set the query component of the URL. It would produce an invalid URL object that just happens to produce the correct URL string.

[diptanu]

Yeah, if we use RawPath, we would also have to add the RawQuery to add the query parameters.

[dbresson]

My latest commit is probably closer to something that would work. I don't think it's quite right yet though, I suspect it will break if the path isn't set at all.

[diptanu]

Parse is an exported method.

[dbresson]

Well, my test seems to pass. I also double checked and path has to have a value for http checks, so there's no problem there. I think the test failures are unrelated to my changes.

[dbresson]

@diptanu Do you have any further feedback?

[diptanu]

@dbresson Thanks! Merged.

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.