net/http/httptest: clarify that Server.Client is only good for Server.URL

[bradfitz]

The docs don't make it clear that the http.Client returned by httptest.Server.Client is only good for hitting httptest.Server.URL.

That is, the http.Client's Transport doesn't redirect all dials of any hostname to the test server. (Maybe it should? Would that break existing users?)

/cc @Capstan

[Capstan]

If a test is testing that a client configures the right host and port, it'd be nice if there were a recipe for how to use httptest.Server with that, ideally without much or heavyweight (e.g., cert generation on the fly) boilerplate.

[odeke-em]

If a test is testing that a client configures the right host and port, it'd be nice if there were a recipe for how to use httptest.Server with that, ideally without much or heavyweight (e.g., cert generation on the fly) boilerplate.

I think one can already use a roundtripper e.g. https://play.golang.org/p/UBfwFeTxK2n or inlined below

package main

import (
    "fmt"
    "io/ioutil"
    "log"
    "net/http"
    "net/http/httputil"
    "strings"
)

type hostnameChecker string

func (hnc hostnameChecker) RoundTrip(req *http.Request) (*http.Response, error) {
    if g, w := req.URL.Hostname(), string(hnc); g != w {
        return nil, fmt.Errorf("Hostname mismatch: %q, wanted %q", g, w)
    }
    res := &http.Response{
        Status:        "OK",
        StatusCode:    200,
        Body:          ioutil.NopCloser(strings.NewReader("Hello")),
        ContentLength: 5,
    }
    return res, nil
}

func main() {
    hnc := hostnameChecker("golang.org")
    req, _ := http.NewRequest("GETT", "http://example.org", nil)
    client := &http.Client{Transport: hnc}
    res, err := client.Do(req)
    if err != nil {
        log.Fatal(err)
    }
    blob, _ := httputil.DumpResponse(res, true)
    println(string(blob))
}

so that use case can already be handled, unless am mistaken.

The docs don't make it clear that the http.Client returned by httptest.Server.Client is only good for hitting httptest.Server.URL.

As you obviously already know, the client created here is the bare minimum http client

Start

https://github.com/golang/go/blob/46be01f4e06a2ef1d2450a81dd855671eac5b855/src/net/http/httptest/server.go#L122-L124

and then just configured with the same certs as the server

StartTLS

https://github.com/golang/go/blob/46be01f4e06a2ef1d2450a81dd855671eac5b855/src/net/http/httptest/server.go#L139-L169

and the docs https://golang.org/pkg/net/http/httptest/#Server.Client

loosely talk about that client being for making calls to server but don't emphasize exclusive allegiance to the server.

I think that changing dials of any hostname to the test server might break users whose tests might be comparing HTTP responses served by a handler to those from an external server during say integration tests.

Perhaps to help us compare and find the usage of (*httptest.Server).Client() with other URLs, I'll kindly ping the Sourcegraph people who've indexed source code publicly available and examine some usages @sqs @vanesa or even Github's BigQuery corpus as well.

Ultimately, thought I think it is reasonable to tighten all redirects regardless of hostname to the test server, and if anyone is using the client from net/http/httptest, they should instead switch to using net/http.

[gopherbot]

Change https://go.dev/cl/581777 mentions this issue: net/http/httptest: add comment to Server.Client() about Server.URL