net/http: clarify documentation around when or if it is safe to reuse a request body

[twmb]

The net/http library's Request documents that a client's Transport is responsible for calling a request body's Close method.

A response can be returned before a request's body is entirely written. I looked a bit and there appears to be no guarantee about a request body being closed at any time (even after a response body hits io.EOF and is closed).

To ensure that a request body is safe to reuse, we have to wrap it in a type that tracks an outstanding close.

However, if a body passed to NewRequest is one of a few types, bytes.Reader being one of those types, a request automatically has its GetBody field populated. On redirect, if any of the body has been read, the body is "cloned" and the original body is closed. If I want to wrap a bytes.Reader in this close counter, I lose the ability to have GetBody used on redirects. My alternative is to set GetBody myself. That makes this close counter a bit more difficult.

I know that GetBody is not called after we have a response since there will be no more redirects to follow. So, being careful, I can write a close counter that increments the number of expected closes if I set GetBody appropriately and it is called.

I currently have the following code:

type ReReqBody struct {
        buf []byte
        at  int 
        wg  *sync.WaitGroup
        ch  chan struct{}
}

func (r *ReReqBody) Read(b []byte) (int, error) {
        n := copy(b, r.buf[r.at:])
        if n == 0 { 
                return 0, io.EOF
        }
        r.at += n
        return n, nil 
}

func (r *ReReqBody) Close() error {
        r.wg.Done()
        return nil 
}

func (r *ReReqBody) Clone() *ReReqBody {
        r.wg.Add(1)
        return &ReReqBody{
                buf: r.buf,
                wg:  r.wg,
        }
}

func (r *ReReqBody) lazyInit() {
        if r.wg == nil {
                r.wg = new(sync.WaitGroup)
                r.ch = make(chan struct{}, 1)
        }
}

func (r *ReReqBody) Wait() <-chan struct{} {
        r.lazyInit()
        go func() {
                r.wg.Wait()
                r.ch <- struct{}{}
        }() 
        return r.ch
}

func (r *ReReqBody) Reset(buf []byte) {
        r.lazyInit()
        r.buf = buf 
        r.at = 0 
        r.wg.Add(1)
}

and it can be used somehow like the following:

<-rrb.Wait() // wait from a prior request
myBody = ... // modify an old request body
req := http.NewRequest("POST", "https://www.example.com", nil)
rrb.Reset(myBody)
req.Body = rrb
req.GetBody = func() (io.ReadCloser, error) { return rrb.Clone() }

I've gathered most of this by reading a bit of the net/http source, but it would be great to have documentation guidance for when exactly it is safe to reuse a request body and if it is safe at all to reuse a request body with GetBody.

[twmb]

(note that the code fails until #26408 is addressed)

[odeke-em]

Tagging @broady @tombergan @bradfitz who might all be interested too, as per @broady's previously addressed issue #19653 that requested for:
a) Clarification on when a request's body can be reused
which was addressed with the documentation https://golang.org/pkg/net/http/#RoundTripper from CL 75671 aka commit 3039bff

@twmb, @bradfitz talked about perhaps documenting that callers can wrap the Body themselves, as you have done to ensure body reading hits EOF before reuse, as you have done too, and he mentioned this in #19653 (comment) but the docs don't explicitly say this, but we allude to it in the docs "Callers should not modify the Request's body until the Response's Body has been closed". Perhaps we need to do more for GetBody clarification?

[twmb]

A bit of clarification around GetBody usage in combination with the existing "arrange to wait" paragraph would suffice here, I think.

[gopherbot]

Change https://golang.org/cl/124315 mentions this issue: net/http: fix double-close of req.Body