/cc @bradfitz @bcmills

For an isolated and runnable repro, please see https://play.golang.org/p/uD5-UNDhTyN or inlined below

package main

import (
	"io/ioutil"
	"net/http"
	"net/http/httptest"
	"time"
)

func main() {
	cst := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Write([]byte("Hello, issue!"))
	}))
	cst.Config.WriteTimeout = -time.Second
	cst.Config.ReadTimeout = -time.Second
	cst.Start()
	defer cst.Close()

	for i := 0; i < 10; i++ {
		res, err := cst.Client().Get(cst.URL)
		if err != nil {
			panic(err)
		}
		_, _ = ioutil.ReadAll(res.Body)
		res.Body.Close()
	}
}

I'm not sure if setting a negative value for those fields has any practical use. Perhaps testing?

I could see documentation being enough for this but it's also a small code change to not add negative values to the timeouts.

I don't see any use of negative values there either. For testing very small, positive values could achieve the same result.

I believe not accepting negative values (or setting them to zero) would make http.Server safer to use. However I wonder how that could affect existing programs and impact the go compatibility promise.

Change https://golang.org/cl/235437 mentions this issue: net/http: add to deadlines only when positive

I believe the current behavior is consistent with the documentation: A negative timeout gives less than no time in which to perform the operation. This isn't useful, but a 1ns timeout isn't really useful either.

I don't think there's any need to change the package behavior or documentation with regard to negative timeouts.

I do note that there is no documentation of what a zero WriteTimeout means. We should explicitly document that this indicates no timeout.

Sometimes we use negative values to mean "disabled, no timeout" when 0 means "pick some default timeout value for me".

I'm fine with making negative values an error or disabled. I agree we should say what 0 means for ReadTimeout.

Sometimes we use negative values to mean "disabled, no timeout" when 0 means "pick some default timeout value for me".

I'm fine with making negative values an error or disabled. I agree we should say what 0 means for ReadTimeout.

I believe we should say what 0 means for both of them (ReadTimeout and WriteTimeout).

Making negative values an error or disabled are both fine for me. However, I believe that making negative values an error is "safer" in the sense that it's easier to spot errors. What lead me to open this issue was an overflow of time.Duration (int64) that make the timeouts negative without nobody noticing it.

@belimawr given that negative values already have a well-defined behavior, that behavior arguably cannot be changed due to the Go 1 compatibility policy.

We can certainly improve the documentation, though.

What lead me to open this issue was an overflow of time.Duration (int64) that make the timeouts negative without nobody noticing it.

Arguably the defect there lies with time.Duration rather than net/http.Server. The same class of errors is possible for any code that manipulates Duration values, so the fix should be a general fix in the time package or some (static or dynamic) analysis tool. (#20757 is closely related.)