Set sensible http client timeout

[AidanDelaney]

Set the default HTTP client timeout to 1 minute. This avoids extremely long timeouts.

Summary

Go defaults to having no timeout on http connections. In environments where a proxy setting is incorrectly configured, this can cause an excessive wait before we see a "unable to download" message.

Use Cases

Ensure we fail quickly when downloading assets in misconfigured environments.

Checklist

• [x] I have viewed, signed, and submitted the Contributor License Agreement.
• [ ] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
• [ ] I have added an integration test, if necessary.
• [x] I have reviewed the styleguide for guidance on my code quality.
• [x] I'm happy with the commit history on this PR (I have rebased/squashed as needed).

[dmikusa]

A couple of questions:

1. Should the timeout be configurable? I think something like this typically is configurable. I would suggest looking at an env variable, if it's set use that value. If not set, then use the default.

2. I wonder if client.Timeout is the right timeout to set. This description has me worried because it is saying that it will interrupt the download if it doesn't complete within the timeout. If there is a big file or a slow network, that could cause the timer to interrupt and that would be frustrating.

   Timeout specifies a time limit for requests made by this Client. The timeout includes connection time, any redirects, and reading the response body. The timer remains running after Get, Head, Post, or Do return and will interrupt reading of the Response.Body.

   https://pkg.go.dev/net/http#Client

   I think what we probably want is a connect timeout, so that it fails fast (maybe 20s) if the TCP connection isn't made. If the TCP connection is established, then it should probably take as long as it needs to take. There are a ton of different client timeout settings, I just don't think we should include the response body in the timeout for the reasons above.

[AidanDelaney]

The document at https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/#clienttimeouts gives a great overview of timeouts. This led me to the DefaultTransport in https://pkg.go.dev/net/http, which seems to do what we want without further configuration.

I don't yet have a feeling for whether we should expose this to the library user. Using the default approach, they can always override http.DefaultTransport.

[dmikusa]

I don't think what is being proposed now is any different from the default. The default client is:

var DefaultClient = &Client{}

https://cs.opensource.google/go/go/+/refs/tags/go1.20.4:src/net/http/client.go;l=110

If we want to set default timeouts that way then I think we need something like this:

c := &http.Client{
    Transport: &http.Transport{
        Dial: (&net.Dialer{
                Timeout:   30 * time.Second,
                KeepAlive: 30 * time.Second,
        }).Dial,
        TLSHandshakeTimeout:   10 * time.Second,
        ResponseHeaderTimeout: 10 * time.Second,
        ExpectContinueTimeout: 1 * time.Second,
    }
}

and I would be in favor of taking these values from env variables so they can be easily overridden.

My $0.02 on defaults:

• Dial.Timeout 6s (I believe DNS resolution is included in this, so it needs to be over 5s as that's a common DNS timeout)
• Dial.KeepAlive 60s (Longer seems better, why not reuse connections if possible)
• TLS timeout 5s (probably excessive)
• Response headerTimeout 5s (also probably excessive)
• expect continue 1s

[dmikusa]

@AidanDelaney How about https://github.com/paketo-buildpacks/libpak/pull/250?

[dmikusa]

Thanks for raising up the need for this functionality. Closing this PR as we merged in #250.