I would suggest we increase the minimum MaxIdleConnsPerHost to something like 5-10 range.
I suggest this because a network connection isn't going to Pin a CPU and resource restriction should probably be done externally via a sync/semaphore?

Here is a potential solution I arbitrarily made up

// Conns calculates a good number of idle connections assuming a GOPROC can handle more than 1 network connection at a time . Algo is Max(GOMAXPROCS*2, 8) 
func Conns() int {
  if  max := runtime.GOMAXPROCS(0) * 2 ; max > 8 {
        return max
  }
  return 8
}

// DefaultPooledTransport returns a new http.Transport with similar default
// values to http.DefaultTransport. Do not use this for transient transports as
// it can leak file descriptors over time. Only use this for transports that
// will be re-used for the same host(s).
func DefaultPooledTransport() *http.Transport {
	transport := &http.Transport{
		Proxy: http.ProxyFromEnvironment,
		DialContext: (&net.Dialer{
			Timeout:   30 * time.Second,
			KeepAlive: 30 * time.Second,
			DualStack: true,
		}).DialContext,
		MaxIdleConns:          100,
		IdleConnTimeout:       90 * time.Second,
		TLSHandshakeTimeout:   10 * time.Second,
		ExpectContinueTimeout: 1 * time.Second,
		MaxIdleConnsPerHost:   runtime.GOMAXPROCS(0) + 1, // <-- Probably could be much larger on the low end
	}
	return transport
}

A few features were added to http.Transport in Go 1.7. The default transport now looks like

var DefaultTransport RoundTripper = &Transport{
    Proxy: ProxyFromEnvironment,
    DialContext: (&net.Dialer{
        Timeout:   30 * time.Second,
        KeepAlive: 30 * time.Second,
    }).DialContext,
    MaxIdleConns:          100,
    IdleConnTimeout:       90 * time.Second,
    TLSHandshakeTimeout:   10 * time.Second,
    ExpectContinueTimeout: 1 * time.Second,
}

Should these new fields be added? Maybe in a separate file with build tags?

Think library should also enforce HTTPClient.Timeout? (which is 0:unlimited by default)

I get the following errors when attempting to go install a project that depends on go-cleanhttp:

# github.com/hashicorp/go-cleanhttp
../gocode/src/github.com/hashicorp/go-cleanhttp/cleanhttp.go:27:4: error: unknown field ‘KeepAlive’ in ‘net.Dialer’
    KeepAlive: 30 * time.Second,
    ^
../gocode/src/github.com/hashicorp/go-cleanhttp/cleanhttp.go:29:3: error: unknown field ‘TLSHandshakeTimeout’ in ‘http.Transport’
   TLSHandshakeTimeout: 10 * time.Second,
   ^

go 1.11.6 and 1.12 contain this change: golang/go#29923

With 1.11.6, the tests for go-cleanhttp fail like this:

--- FAIL: TestPrintablePathCheckHandler (0.00s)
    --- FAIL: TestPrintablePathCheckHandler/invalid_carriage_return (0.00s)
        handlers_test.go:64: parse http://127.0.0.1:42665/invalid
: net/url: invalid control character in URL
    --- FAIL: TestPrintablePathCheckHandler/invalid_null (0.00s)
        handlers_test.go:64: parse http://127.0.0.1:40705/invalid: net/url: invalid control character in URL
    --- PASS: TestPrintablePathCheckHandler/valid_empty_error_status (0.00s)
    --- FAIL: TestPrintablePathCheckHandler/invalid_alternate_status (0.00s)
        handlers_test.go:64: parse http://127.0.0.1:45233/invalid
            : net/url: invalid control character in URL
    --- PASS: TestPrintablePathCheckHandler/valid_nil_input (0.00s)
    --- FAIL: TestPrintablePathCheckHandler/invalid_newline (0.00s)
        handlers_test.go:64: parse http://127.0.0.1:44825/invalid
            : net/url: invalid control character in URL

Thanks,
Chris

As explained in consulapi#1499 and go-dockerclient#434, it is not safe at all to replace http.DefaultClient with &http.Client{} or cleanhttp.DefaultClient(), and the README should address this fairly obscure and dangerous issue. I think it should suggest library authors to either instantiate a global DefaultClient, or to set DisableKeepAlives to true.

This project doesn't appear to include any copyright information or a NOTICE file. Can you add one? This is desired to comply with the open source license conditions.

Hi,
I'm using cleanhttp through retryablehttp, and one of the site I'm querying is making me upgrade to HTTP2.
However the default implementation of cleanhttp does not let the connection happen, and makes the request return empty data.
I tried to debug this a bit, and it seems that ForceAttemptHTTP2 is missing from the default options, and by manually setting it to true, the connection happens and data is returned correctly.
This flag has been added to the http.DefaultTransport, would it be possible to enable for the default transport options in this package too?
Thanks

Kubeflow kfdef / kfctl uses your libraries to download tarballs from HTTPS-URLs.

https://github.com/kubeflow/kfctl/search?q=%22gogetter.getany%22

In an enterprise environment, very often, https-certificates are issued by an internal PKI whose CAs and intermediate CAs are by definition not publicly-trusted. The concept of private PKIs is explained here: https://www.digicert.com/support/resources/faq/identity-and-access-trust/what-is-private-pki-vs-public-pki

There have been requests regarding non-public locations of files in Kubeflow, but my guess is the science-oriented community there does not want to take the time to iron out the dependencies or ask their dependenc

For downloading with python http.get, the concept and environment is explained here in detail for per-request PEM file verify of SSL chain.

opendatahub-io-contrib/jupyterhub-odh#137

More globally and at a go-Level, can you add support for adding custom CAs from a PEM encoded file to the x509.SystemCertPool, please? This would make your tooling much more enterprise-ready and also help make Kubeflow installs more enterprise-ready, especially with regards to on-prem. I think you already have http proxy support and authentication support. Only thing missing now is adding non-public CAs from enterprise private PKIs.

I suppose the location of the file in question is this one:

https://github.com/hashicorp/go-cleanhttp/blob/master/cleanhttp.go

The approach "Append the self-signed cert or CAs to an in-app copy of the host system trust store" is defined here:

https://forfuncsake.github.io/post/2017/08/trust-extra-ca-cert-in-go-app/

This post is quite old, though, from 2017.

My idea in another context, Open Data Hub with Openshift file-based location for trusted CAs, could work here, too.

That is, if we could define a filesystem location for the ca-bundle file, then do AppendCertsFromPEM.

I am not familiar enough with golang to say if an env-based solution is already there and used implicitely by your functions, like here

golang/go@e83bcd9

What do you think?

e.g. env SSL_CERT_FILE read from a configmap ca-bundle.crt section, if that env were present, would your http go-getter routines use that for chain of trust SSL validation?