Request wrapper make standard request GetBody property always nil

[rgb-24bit]

In the Go standard library, when creating a new request, the GetBody property of the request is set according to the body parameter.

This attribute will affect the client's behavior after receiving the 307, 308 response code. If it is empty, it is likely that it will not follow redirect.

if ireq.GetBody == nil && ireq.outgoingLength() != 0 {
	// We had a request body, and 307/308 require
	// re-sending it, but GetBody is not defined. So just
	// return this response to the user instead of an
	// error, like we did in Go 1.7 and earlier.
	shouldRedirect = false
}

But now when request wrapper creates a new request, the body parameter is not passed, making GetBody always nil.

func NewRequest(method, url string, rawBody interface{}) (*Request, error) {
	bodyReader, contentLength, err := getBodyReaderAndContentLength(rawBody)
	if err != nil {
		return nil, err
	}

	httpReq, err := http.NewRequest(method, url, nil)  // <=== here, body is nil
	if err != nil {
		return nil, err
	}
	httpReq.ContentLength = contentLength

	return &Request{bodyReader, httpReq}, nil
}

[ryanuber]

Good find! This is probably a relic of this library existing since before GetBody() was a thing. Thanks for the report.

[zeisss]

We ran into this today as well with HTTP2:

http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error

Not sure why the HTTP2 is trying to retry requests on its own when the body was sent but it failed since the body could not be consumed again.

[mitar]

I would need to be able to log all requests (and their payloads) inside RequestLogHook. In RequestLogHook only a http.Request is available, but GetBody on it is not.

[tagur87]

Is there any update on a fix for this bug? We are running into this with an API that redirects with a 308 while PUTing a request body.

Any know workarounds we can use in our client to bypass this issue for now?

[tagur87]

@rgb-24bit - I rebased and tested your changes here: #122 on my own fork, and they seem to work well for fixing my 308 redirect issue. Can you rebase your PR and try to get this fix merged in?

[rgb-24bit]

@tagur87 done rebase

[tagur87]

Is there any movement on getting this change merged into master?