From e3ba569c78d2c496537f4546282a5e04f33c886e Mon Sep 17 00:00:00 2001 From: haruyama480 Date: Fri, 25 Aug 2023 15:14:35 +0900 Subject: [PATCH] [release-branch.go1.21] net/http: revert "support streaming POST content in wasm" CL 458395 added support for streaming POST content in Wasm. Unfortunately, this breaks requests to servers that only support HTTP/1.1. Revert the change until a suitable fallback or opt-in strategy can be decided. For #61889. Fixes #62328. Change-Id: If53a77e1890132063b39abde867d34515d4ac2af Reviewed-on: https://go-review.googlesource.com/c/go/+/522955 Run-TryBot: Johan Brandhorst-Satzkorn Reviewed-by: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills Reviewed-by: Johan Brandhorst-Satzkorn Reviewed-on: https://go-review.googlesource.com/c/go/+/524855 Run-TryBot: Dmitri Shuralyov Commit-Queue: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov Auto-Submit: Dmitri Shuralyov --- src/net/http/roundtrip_js.go | 107 ++++++----------------------------- 1 file changed, 18 insertions(+), 89 deletions(-) diff --git a/src/net/http/roundtrip_js.go b/src/net/http/roundtrip_js.go index 2826383ce1..9f9f0cb67d 100644 --- a/src/net/http/roundtrip_js.go +++ b/src/net/http/roundtrip_js.go @@ -55,38 +55,6 @@ var jsFetchMissing = js.Global().Get("fetch").IsUndefined() var jsFetchDisabled = js.Global().Get("process").Type() == js.TypeObject && strings.HasPrefix(js.Global().Get("process").Get("argv0").String(), "node") -// Determine whether the JS runtime supports streaming request bodies. -// Courtesy: https://developer.chrome.com/articles/fetch-streaming-requests/#feature-detection -func supportsPostRequestStreams() bool { - requestOpt := js.Global().Get("Object").New() - requestBody := js.Global().Get("ReadableStream").New() - - requestOpt.Set("method", "POST") - requestOpt.Set("body", requestBody) - - // There is quite a dance required to define a getter if you do not have the { get property() { ... } } - // syntax available. However, it is possible: - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get#defining_a_getter_on_existing_objects_using_defineproperty - duplexCalled := false - duplexGetterObj := js.Global().Get("Object").New() - duplexGetterFunc := js.FuncOf(func(this js.Value, args []js.Value) any { - duplexCalled = true - return "half" - }) - defer duplexGetterFunc.Release() - duplexGetterObj.Set("get", duplexGetterFunc) - js.Global().Get("Object").Call("defineProperty", requestOpt, "duplex", duplexGetterObj) - - // Slight difference here between the aforementioned example: Non-browser-based runtimes - // do not have a non-empty API Base URL (https://html.spec.whatwg.org/multipage/webappapis.html#api-base-url) - // so we have to supply a valid URL here. - requestObject := js.Global().Get("Request").New("https://www.example.org", requestOpt) - - hasContentTypeHeader := requestObject.Get("headers").Call("has", "Content-Type").Bool() - - return duplexCalled && !hasContentTypeHeader -} - // RoundTrip implements the RoundTripper interface using the WHATWG Fetch API. func (t *Transport) RoundTrip(req *Request) (*Response, error) { // The Transport has a documented contract that states that if the DialContext or @@ -136,63 +104,24 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) { opt.Set("headers", headers) if req.Body != nil { - if !supportsPostRequestStreams() { - body, err := io.ReadAll(req.Body) - if err != nil { - 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) - opt.Set("body", buf) - } - } else { - readableStreamCtorArg := js.Global().Get("Object").New() - readableStreamCtorArg.Set("type", "bytes") - readableStreamCtorArg.Set("autoAllocateChunkSize", t.writeBufferSize()) - - readableStreamPull := js.FuncOf(func(this js.Value, args []js.Value) any { - controller := args[0] - byobRequest := controller.Get("byobRequest") - if byobRequest.IsNull() { - controller.Call("close") - } - - byobRequestView := byobRequest.Get("view") - - bodyBuf := make([]byte, byobRequestView.Get("byteLength").Int()) - readBytes, readErr := io.ReadFull(req.Body, bodyBuf) - if readBytes > 0 { - buf := uint8Array.New(byobRequestView.Get("buffer")) - js.CopyBytesToJS(buf, bodyBuf) - byobRequest.Call("respond", readBytes) - } - - if readErr == io.EOF || readErr == io.ErrUnexpectedEOF { - controller.Call("close") - } else if readErr != nil { - readErrCauseObject := js.Global().Get("Object").New() - readErrCauseObject.Set("cause", readErr.Error()) - readErr := js.Global().Get("Error").New("io.ReadFull failed while streaming POST body", readErrCauseObject) - controller.Call("error", readErr) - } - // 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)) - // There is a requirement from the WHATWG fetch standard that the duplex property of - // the object given as the options argument to the fetch call be set to 'half' - // when the body property of the same options object is a ReadableStream: - // https://fetch.spec.whatwg.org/#dom-requestinit-duplex - opt.Set("duplex", "half") + // TODO(johanbrandhorst): Stream request body when possible. + // See https://bugs.chromium.org/p/chromium/issues/detail?id=688906 for Blink issue. + // See https://bugzilla.mozilla.org/show_bug.cgi?id=1387483 for Firefox issue. + // See https://github.com/web-platform-tests/wpt/issues/7693 for WHATWG tests issue. + // See https://developer.mozilla.org/en-US/docs/Web/API/Streams_API for more details on the Streams API + // and browser support. + // NOTE(haruyama480): Ensure HTTP/1 fallback exists. + // See https://go.dev/issue/61889 for discussion. + body, err := io.ReadAll(req.Body) + if err != nil { + 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) + opt.Set("body", buf) } } -- 2.48.1