net/http: close req.Body only when it's non-nil on js

The main change here is fixing the panic where it was called even when
req.Body was nil. It might also work better to keep the req.Body.Close
calls closer after req.Body is read, so do that too.

Calling readableStreamPull.Release on a js.Func with a zero value
is currently a no-op, but it seems better to avoid it anyway.

Also remove readableStreamStart, readableStreamCancel while here.
They were used in the initial but not final patch set of CL 458395.

Fixes #60809.

Change-Id: I6ff2e3b6ec2cd4b0c9c67939903e32908312db8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/503676
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

1 files changed, 6 insertions, 12 deletions

diff --git a/src/net/http/roundtrip_js.go b/src/net/http/roundtrip_js.go
index 323372584f..2826383ce1 100644
--- a/src/net/http/roundtrip_js.go
+++ b/src/net/http/roundtrip_js.go
@@ -135,7 +135,6 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 	}
 	opt.Set("headers", headers)
 
-	var readableStreamStart, readableStreamPull, readableStreamCancel js.Func
 	if req.Body != nil {
 		if !supportsPostRequestStreams() {
 			body, err := io.ReadAll(req.Body)
@@ -143,6 +142,7 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 				req.Body.Close() // RoundTrip must always close the body, including on errors.
 				return nil, err
 			}
+			req.Body.Close()
 			if len(body) != 0 {
 				buf := uint8Array.New(len(body))
 				js.CopyBytesToJS(buf, body)
@@ -153,7 +153,7 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 			readableStreamCtorArg.Set("type", "bytes")
 			readableStreamCtorArg.Set("autoAllocateChunkSize", t.writeBufferSize())
 
-			readableStreamPull = js.FuncOf(func(this js.Value, args []js.Value) any {
+			readableStreamPull := js.FuncOf(func(this js.Value, args []js.Value) any {
 				controller := args[0]
 				byobRequest := controller.Get("byobRequest")
 				if byobRequest.IsNull() {
@@ -181,6 +181,10 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 				// Note: This a return from the pull callback of the controller and *not* RoundTrip().
 				return nil
 			})
+			defer func() {
+				readableStreamPull.Release()
+				req.Body.Close()
+			}()
 			readableStreamCtorArg.Set("pull", readableStreamPull)
 
 			opt.Set("body", js.Global().Get("ReadableStream").New(readableStreamCtorArg))
@@ -201,11 +205,6 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 	success = js.FuncOf(func(this js.Value, args []js.Value) any {
 		success.Release()
 		failure.Release()
-		readableStreamCancel.Release()
-		readableStreamPull.Release()
-		readableStreamStart.Release()
-
-		req.Body.Close()
 
 		result := args[0]
 		header := Header{}
@@ -270,11 +269,6 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 	failure = js.FuncOf(func(this js.Value, args []js.Value) any {
 		success.Release()
 		failure.Release()
-		readableStreamCancel.Release()
-		readableStreamPull.Release()
-		readableStreamStart.Release()
-
-		req.Body.Close()
 
 		err := args[0]
 		// The error is a JS Error type