[go] net/http: add js/wasm compatible DefaultTransport
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 33bfd522a2f1b7c2df6ce3fb9d645806753160ab
GitHub-Pull-Request: golang/go#25550
---
M src/net/http/http.go
A src/net/http/proxy.go
M src/net/http/request.go
M src/net/http/transport.go
A src/net/http/transport_js.go
5 files changed, 439 insertions(+), 202 deletions(-)
diff --git a/src/net/http/http.go b/src/net/http/http.go
index ce0eceb..d24d089 100644
--- a/src/net/http/http.go
+++ b/src/net/http/http.go
@@ -33,10 +33,6 @@
func (k *contextKey) String() string { return "net/http context value " + k.name }
-// Given a string of the form "host", "host:port", or "[ipv6::address]:port",
-// return true if the string includes a port.
-func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") }
-
// removeEmptyPort strips the empty port in ":port" to ""
// as mandated by RFC 3986 Section 6.2.3.
func removeEmptyPort(host string) string {
@@ -50,15 +46,6 @@
return !httpguts.IsTokenRune(r)
}
-func isASCII(s string) bool {
- for i := 0; i < len(s); i++ {
- if s[i] >= utf8.RuneSelf {
- return false
- }
- }
- return true
-}
-
func hexEscapeNonASCII(s string) string {
newLen := 0
for i := 0; i < len(s); i++ {
diff --git a/src/net/http/proxy.go b/src/net/http/proxy.go
new file mode 100644
index 0000000..8c925c3
--- /dev/null
+++ b/src/net/http/proxy.go
@@ -0,0 +1,217 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package http
+
+import (
+ "errors"
+ "fmt"
+ "golang_org/x/net/idna"
+ "net"
+ "net/url"
+ "os"
+ "strings"
+ "sync"
+ "unicode/utf8"
+)
+
+// ProxyFromEnvironment returns the URL of the proxy to use for a
+// given request, as indicated by the environment variables
+// HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
+// thereof). HTTPS_PROXY takes precedence over HTTP_PROXY for https
+// requests.
+//
+// The environment values may be either a complete URL or a
+// "host[:port]", in which case the "http" scheme is assumed.
+// An error is returned if the value is a different form.
+//
+// A nil URL and nil error are returned if no proxy is defined in the
+// environment, or a proxy should not be used for the given request,
+// as defined by NO_PROXY.
+//
+// As a special case, if req.URL.Host is "localhost" (with or without
+// a port number), then a nil URL and nil error will be returned.
+func ProxyFromEnvironment(req *Request) (*url.URL, error) {
+ var proxy string
+ if req.URL.Scheme == "https" {
+ proxy = httpsProxyEnv.Get()
+ }
+ if proxy == "" {
+ proxy = httpProxyEnv.Get()
+ if proxy != "" && os.Getenv("REQUEST_METHOD") != "" {
+ return nil, errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
+ }
+ }
+ if proxy == "" {
+ return nil, nil
+ }
+ if !useProxy(canonicalAddr(req.URL)) {
+ return nil, nil
+ }
+ proxyURL, err := url.Parse(proxy)
+ if err != nil ||
+ (proxyURL.Scheme != "http" &&
+ proxyURL.Scheme != "https" &&
+ proxyURL.Scheme != "socks5") {
+ // proxy was bogus. Try prepending "http://" to it and
+ // see if that parses correctly. If not, we fall
+ // through and complain about the original one.
+ if proxyURL, err := url.Parse("http://" + proxy); err == nil {
+ return proxyURL, nil
+ }
+
+ }
+ if err != nil {
+ return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
+ }
+ return proxyURL, nil
+}
+
+// ProxyURL returns a proxy function (for use in a Transport)
+// that always returns the same URL.
+func ProxyURL(fixedURL *url.URL) func(*Request) (*url.URL, error) {
+ return func(*Request) (*url.URL, error) {
+ return fixedURL, nil
+ }
+}
+
+func isASCII(s string) bool {
+ for i := 0; i < len(s); i++ {
+ if s[i] >= utf8.RuneSelf {
+ return false
+ }
+ }
+ return true
+}
+
+func idnaASCII(v string) (string, error) {
+ // TODO: Consider removing this check after verifying performance is okay.
+ // Right now punycode verification, length checks, context checks, and the
+ // permissible character tests are all omitted. It also prevents the ToASCII
+ // call from salvaging an invalid IDN, when possible. As a result it may be
+ // possible to have two IDNs that appear identical to the user where the
+ // ASCII-only version causes an error downstream whereas the non-ASCII
+ // version does not.
+ // Note that for correct ASCII IDNs ToASCII will only do considerably more
+ // work, but it will not cause an allocation.
+ if isASCII(v) {
+ return v, nil
+ }
+ return idna.Lookup.ToASCII(v)
+}
+
+// canonicalAddr returns url.Host but always with a ":port" suffix
+func canonicalAddr(url *url.URL) string {
+ addr := url.Hostname()
+ if v, err := idnaASCII(addr); err == nil {
+ addr = v
+ }
+ port := url.Port()
+ if port == "" {
+ port = portMap[url.Scheme]
+ }
+ return net.JoinHostPort(addr, port)
+}
+
+// envOnce looks up an environment variable (optionally by multiple
+// names) once. It mitigates expensive lookups on some platforms
+// (e.g. Windows).
+type envOnce struct {
+ names []string
+ once sync.Once
+ val string
+}
+
+func (e *envOnce) Get() string {
+ e.once.Do(e.init)
+ return e.val
+}
+
+func (e *envOnce) init() {
+ for _, n := range e.names {
+ e.val = os.Getenv(n)
+ if e.val != "" {
+ return
+ }
+ }
+}
+
+// reset is used by tests
+func (e *envOnce) reset() {
+ e.once = sync.Once{}
+ e.val = ""
+}
+
+var (
+ httpProxyEnv = &envOnce{
+ names: []string{"HTTP_PROXY", "http_proxy"},
+ }
+ httpsProxyEnv = &envOnce{
+ names: []string{"HTTPS_PROXY", "https_proxy"},
+ }
+ noProxyEnv = &envOnce{
+ names: []string{"NO_PROXY", "no_proxy"},
+ }
+)
+
+// Given a string of the form "host", "host:port", or "[ipv6::address]:port",
+// return true if the string includes a port.
+func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") }
+
+// useProxy reports whether requests to addr should use a proxy,
+// according to the NO_PROXY or no_proxy environment variable.
+// addr is always a canonicalAddr with a host and port.
+func useProxy(addr string) bool {
+ if len(addr) == 0 {
+ return true
+ }
+ host, _, err := net.SplitHostPort(addr)
+ if err != nil {
+ return false
+ }
+ if host == "localhost" {
+ return false
+ }
+ if ip := net.ParseIP(host); ip != nil {
+ if ip.IsLoopback() {
+ return false
+ }
+ }
+
+ noProxy := noProxyEnv.Get()
+ if noProxy == "*" {
+ return false
+ }
+
+ addr = strings.ToLower(strings.TrimSpace(addr))
+ if hasPort(addr) {
+ addr = addr[:strings.LastIndex(addr, ":")]
+ }
+
+ for _, p := range strings.Split(noProxy, ",") {
+ p = strings.ToLower(strings.TrimSpace(p))
+ if len(p) == 0 {
+ continue
+ }
+ if hasPort(p) {
+ p = p[:strings.LastIndex(p, ":")]
+ }
+ if addr == p {
+ return false
+ }
+ if len(p) == 0 {
+ // There is no host part, likely the entry is malformed; ignore.
+ continue
+ }
+ if p[0] == '.' && (strings.HasSuffix(addr, p) || addr == p[1:]) {
+ // no_proxy ".foo.com" matches "bar.foo.com" or "foo.com"
+ return false
+ }
+ if p[0] != '.' && strings.HasSuffix(addr, p) && addr[len(addr)-len(p)-1] == '.' {
+ // no_proxy "foo.com" matches "bar.foo.com"
+ return false
+ }
+ }
+ return true
+}
diff --git a/src/net/http/request.go b/src/net/http/request.go
index 119a015..a85f6ae 100644
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -25,8 +25,6 @@
"strconv"
"strings"
"sync"
-
- "golang_org/x/net/idna"
)
const (
@@ -638,22 +636,6 @@
// This error type should not escape the net/http package to users.
type requestBodyReadError struct{ error }
-func idnaASCII(v string) (string, error) {
- // TODO: Consider removing this check after verifying performance is okay.
- // Right now punycode verification, length checks, context checks, and the
- // permissible character tests are all omitted. It also prevents the ToASCII
- // call from salvaging an invalid IDN, when possible. As a result it may be
- // possible to have two IDNs that appear identical to the user where the
- // ASCII-only version causes an error downstream whereas the non-ASCII
- // version does not.
- // Note that for correct ASCII IDNs ToASCII will only do considerably more
- // work, but it will not cause an allocation.
- if isASCII(v) {
- return v, nil
- }
- return idna.Lookup.ToASCII(v)
-}
-
// cleanHost cleans up the host sent in request's Host header.
//
// It both strips anything after '/' or ' ', and puts the value
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index cce88ca..4fe0137 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -7,6 +7,8 @@
// This is the low-level Transport implementation of RoundTripper.
// The high-level interface is in client.go.
+// +build !js
+
package http
import (
@@ -255,66 +257,6 @@
}
}
-// ProxyFromEnvironment returns the URL of the proxy to use for a
-// given request, as indicated by the environment variables
-// HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
-// thereof). HTTPS_PROXY takes precedence over HTTP_PROXY for https
-// requests.
-//
-// The environment values may be either a complete URL or a
-// "host[:port]", in which case the "http" scheme is assumed.
-// An error is returned if the value is a different form.
-//
-// A nil URL and nil error are returned if no proxy is defined in the
-// environment, or a proxy should not be used for the given request,
-// as defined by NO_PROXY.
-//
-// As a special case, if req.URL.Host is "localhost" (with or without
-// a port number), then a nil URL and nil error will be returned.
-func ProxyFromEnvironment(req *Request) (*url.URL, error) {
- var proxy string
- if req.URL.Scheme == "https" {
- proxy = httpsProxyEnv.Get()
- }
- if proxy == "" {
- proxy = httpProxyEnv.Get()
- if proxy != "" && os.Getenv("REQUEST_METHOD") != "" {
- return nil, errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
- }
- }
- if proxy == "" {
- return nil, nil
- }
- if !useProxy(canonicalAddr(req.URL)) {
- return nil, nil
- }
- proxyURL, err := url.Parse(proxy)
- if err != nil ||
- (proxyURL.Scheme != "http" &&
- proxyURL.Scheme != "https" &&
- proxyURL.Scheme != "socks5") {
- // proxy was bogus. Try prepending "http://" to it and
- // see if that parses correctly. If not, we fall
- // through and complain about the original one.
- if proxyURL, err := url.Parse("http://" + proxy); err == nil {
- return proxyURL, nil
- }
-
- }
- if err != nil {
- return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
- }
- return proxyURL, nil
-}
-
-// ProxyURL returns a proxy function (for use in a Transport)
-// that always returns the same URL.
-func ProxyURL(fixedURL *url.URL) func(*Request) (*url.URL, error) {
- return func(*Request) (*url.URL, error) {
- return fixedURL, nil
- }
-}
-
// transportRequest is a wrapper around a *Request that adds
// optional extra headers to write and stores any error to return
// from roundTrip.
@@ -573,47 +515,6 @@
// Private implementation past this point.
//
-var (
- httpProxyEnv = &envOnce{
- names: []string{"HTTP_PROXY", "http_proxy"},
- }
- httpsProxyEnv = &envOnce{
- names: []string{"HTTPS_PROXY", "https_proxy"},
- }
- noProxyEnv = &envOnce{
- names: []string{"NO_PROXY", "no_proxy"},
- }
-)
-
-// envOnce looks up an environment variable (optionally by multiple
-// names) once. It mitigates expensive lookups on some platforms
-// (e.g. Windows).
-type envOnce struct {
- names []string
- once sync.Once
- val string
-}
-
-func (e *envOnce) Get() string {
- e.once.Do(e.init)
- return e.val
-}
-
-func (e *envOnce) init() {
- for _, n := range e.names {
- e.val = os.Getenv(n)
- if e.val != "" {
- return
- }
- }
-}
-
-// reset is used by tests
-func (e *envOnce) reset() {
- e.once = sync.Once{}
- e.val = ""
-}
-
func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) {
if port := treq.URL.Port(); !validPort(port) {
return cm, fmt.Errorf("invalid URL port %q", port)
@@ -1235,63 +1136,6 @@
return
}
-// useProxy reports whether requests to addr should use a proxy,
-// according to the NO_PROXY or no_proxy environment variable.
-// addr is always a canonicalAddr with a host and port.
-func useProxy(addr string) bool {
- if len(addr) == 0 {
- return true
- }
- host, _, err := net.SplitHostPort(addr)
- if err != nil {
- return false
- }
- if host == "localhost" {
- return false
- }
- if ip := net.ParseIP(host); ip != nil {
- if ip.IsLoopback() {
- return false
- }
- }
-
- noProxy := noProxyEnv.Get()
- if noProxy == "*" {
- return false
- }
-
- addr = strings.ToLower(strings.TrimSpace(addr))
- if hasPort(addr) {
- addr = addr[:strings.LastIndex(addr, ":")]
- }
-
- for _, p := range strings.Split(noProxy, ",") {
- p = strings.ToLower(strings.TrimSpace(p))
- if len(p) == 0 {
- continue
- }
- if hasPort(p) {
- p = p[:strings.LastIndex(p, ":")]
- }
- if addr == p {
- return false
- }
- if len(p) == 0 {
- // There is no host part, likely the entry is malformed; ignore.
- continue
- }
- if p[0] == '.' && (strings.HasSuffix(addr, p) || addr == p[1:]) {
- // no_proxy ".foo.com" matches "bar.foo.com" or "foo.com"
- return false
- }
- if p[0] != '.' && strings.HasSuffix(addr, p) && addr[len(addr)-len(p)-1] == '.' {
- // no_proxy "foo.com" matches "bar.foo.com"
- return false
- }
- }
- return true
-}
-
// connectMethod is the map key (in its String form) for keeping persistent
// TCP connections alive for subsequent HTTP requests.
//
@@ -2118,19 +1962,6 @@
"socks5": "1080",
}
-// canonicalAddr returns url.Host but always with a ":port" suffix
-func canonicalAddr(url *url.URL) string {
- addr := url.Hostname()
- if v, err := idnaASCII(addr); err == nil {
- addr = v
- }
- port := url.Port()
- if port == "" {
- port = portMap[url.Scheme]
- }
- return net.JoinHostPort(addr, port)
-}
-
// bodyEOFSignal is used by the HTTP/1 transport when reading response
// bodies to make sure we see the end of a response body before
// proceeding and reading on the connection again.
diff --git a/src/net/http/transport_js.go b/src/net/http/transport_js.go
new file mode 100644
index 0000000..511c78d
--- /dev/null
+++ b/src/net/http/transport_js.go
@@ -0,0 +1,220 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build js,wasm
+
+package http
+
+import (
+ "errors"
+ "fmt"
+ "io"
+ "io/ioutil"
+ "net/http"
+ "strconv"
+ "syscall/js"
+)
+
+// DefaultTransport is the default implementation of Transport and is
+// used by DefaultClient. It uses the WHATWG Fetch Standard to perform
+// requests. See https://fetch.spec.whatwg.org/ for more information
+// on this standard.
+var DefaultTransport RoundTripper = &Transport{}
+
+// streamReader implements an io.ReadCloser wrapper for ReadableStream.
+// See https://fetch.spec.whatwg.org/#readablestream for more information.
+type streamReader struct {
+ pending []byte
+ stream js.Value
+}
+
+func (r *streamReader) Read(p []byte) (n int, err error) {
+ if len(r.pending) == 0 {
+ var (
+ bCh = make(chan []byte)
+ errCh = make(chan error)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ result := args[0]
+ if result.Get("done").Bool() {
+ errCh <- io.EOF
+ return
+ }
+ value := make([]byte, result.Get("value").Get("byteLength").Int())
+ js.ValueOf(value).Call("set", result.Get("value"))
+ bCh <- value
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ // Assumes it's a DOMException.
+ // See https://heycam.github.io/webidl/#idl-DOMException for
+ // more information on this type
+ errCh <- errors.New(args[0].Get("message").String())
+ })
+ defer failure.Close()
+ r.stream.Call("read").Call("then", success, failure)
+ select {
+ case b := <-bCh:
+ r.pending = b
+ case err := <-errCh:
+ return 0, err
+ }
+ }
+ n = copy(p, r.pending)
+ r.pending = r.pending[n:]
+ return n, nil
+}
+
+func (r *streamReader) Close() error {
+ // This ignores any error returned from cancel method. So far, I did not encounter any concrete
+ // situation where reporting the error is meaningful. Most users ignore error from resp.Body.Close().
+ // If there's a need to report error here, it can be implemented and tested when that need comes up.
+ r.stream.Call("cancel")
+ return nil
+}
+
+// arrayReader implements an io.ReadCloser wrapper for ArrayBuffer.
+// https://developer.mozilla.org/en-US/docs/Web/API/Body/arrayBuffer.
+type arrayReader struct {
+ arrayPromise js.Value
+ pending []byte
+ read bool
+}
+
+func (r *arrayReader) Read(p []byte) (n int, err error) {
+ if !r.read {
+ r.read = true
+ var (
+ bCh = make(chan []byte)
+ errCh = make(chan error)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ // Wrap the input ArrayBuffer with a Uint8Array
+ uint8arrayWrapper := js.Global.Get("Uint8Array").New(args[0])
+ value := make([]byte, uint8arrayWrapper.Get("byteLength").Int())
+ js.ValueOf(value).Call("set", uint8arrayWrapper)
+ bCh <- value
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ // Assumes it's a DOMException.
+ errCh <- errors.New(args[0].Get("message").String())
+ })
+ defer failure.Close()
+ r.arrayPromise.Call("then", success, failure)
+ select {
+ case b := <-bCh:
+ r.pending = b
+ case err := <-errCh:
+ return 0, err
+ }
+ }
+ if len(r.pending) == 0 {
+ return 0, io.EOF
+ }
+ n = copy(p, r.pending)
+ r.pending = r.pending[n:]
+ return n, nil
+}
+
+func (r *arrayReader) Close() error {
+ // This is a noop
+ return nil
+}
+
+// Transport is a RoundTripper that is implemented using the WHATWG Fetch API.
+// It supports streaming response bodies.
+// See https://fetch.spec.whatwg.org/ for more information on this standard.
+type Transport struct{}
+
+// RoundTrip implements the RoundTripper interface.
+func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
+ headers := js.Global.Get("Headers").New()
+ for key, values := range req.Header {
+ for _, value := range values {
+ headers.Call("append", key, value)
+ }
+ }
+
+ ac := js.Global.Get("AbortController").New()
+
+ opt := js.Global.Get("Object").New()
+ opt.Set("headers", headers)
+ opt.Set("method", req.Method)
+ opt.Set("credentials", "same-origin")
+ opt.Set("signal", ac.Get("signal"))
+
+ if req.Body != nil {
+ // TODO(johanbrandhorst): Stream request body when possible.
+ body, err := ioutil.ReadAll(req.Body)
+ if err != nil {
+ _ = req.Body.Close() // RoundTrip must always close the body, including on errors.
+ return nil, err
+ }
+ _ = req.Body.Close()
+ opt.Set("body", body)
+ }
+ respPromise := js.Global.Call("fetch", req.URL.String(), opt)
+ var (
+ respCh = make(chan *http.Response)
+ errCh = make(chan error)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ result := args[0]
+ header := http.Header{}
+ writeHeaders := js.NewCallback(func(args []js.Value) {
+ key, value := args[0].String(), args[1].String()
+ ck := http.CanonicalHeaderKey(key)
+ header[ck] = append(header[ck], value)
+ })
+ defer writeHeaders.Close()
+ result.Get("headers").Call("forEach", writeHeaders)
+
+ contentLength := int64(-1)
+ if cl, err := strconv.ParseInt(header.Get("Content-Length"), 10, 64); err == nil {
+ contentLength = cl
+ }
+
+ b := result.Get("body")
+ var body io.ReadCloser
+ if b != js.Undefined {
+ body = &streamReader{stream: b.Call("getReader")}
+ } else {
+ // Fall back to using ArrayBuffer
+ // https://developer.mozilla.org/en-US/docs/Web/API/Body/arrayBuffer
+ body = &arrayReader{arrayPromise: result.Call("arrayBuffer")}
+ }
+
+ select {
+ case respCh <- &http.Response{
+ Status: result.Get("status").String() + " " + http.StatusText(result.Get("status").Int()),
+ StatusCode: result.Get("status").Int(),
+ Header: header,
+ ContentLength: contentLength,
+ Body: body,
+ Request: req,
+ }:
+ case <-req.Context().Done():
+ }
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ select {
+ case errCh <- fmt.Errorf("net/http: fetch() failed: %s", args[0].String()):
+ case <-req.Context().Done():
+ }
+ })
+ defer failure.Close()
+ respPromise.Call("then", success, failure)
+ select {
+ case <-req.Context().Done():
+ // Abort the Fetch request
+ ac.Call("abort")
+ return nil, req.Context().Err()
+ case resp := <-respCh:
+ return resp, nil
+ case err := <-errCh:
+ return nil, err
+ }
+}
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #2 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: b1f46d7d88dfe56c60d1529c2c1213cc5eef31b7
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/proxy.go
M src/net/http/transport.go
A src/net/http/transport_js.go
3 files changed, 408 insertions(+), 171 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #3 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 1bdaafc7e8387cac620d62838c7c7d6f3274b88a
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/proxy.go
M src/net/http/transport.go
A src/net/http/transport_js.go
3 files changed, 408 insertions(+), 171 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Thanks!
Patch set 3:Run-TryBot +1TryBot-Result +1
12 comments:
File src/net/http/transport.go:
This isn't consistent with the build tag in the other file, which says "js,wasm".
This line would break GopherJS compiling net/http.
Maybe you just want to GopherJS to use the other file? Or later we can, once there's a unified JS syscall interface or plan? For now let's keep GopherJS using this file, so make this:
// +build !js,wasm
File src/net/http/transport_js.go:
bCh = make(chan []byte)
errCh = make(chan error)make these both buffered of size 1, so sends don't block in orphaned goroutines.
Patch Set #2, Line 49:
args []js.Valueis this guaranteed to have at least 1 arg?
Patch Set #2, Line 52:
// more information on this typeend in period.
case err := <-errCh:
return 0, errcan you also note it on r.err, adding a field to streamReader:
err error // sticky read error
bCh = make(chan []byte)
errCh = make(chan error)
)
success := js.NewCallback(func(args []js.Value) {
// Wrap the input ArrayBuffer with a Uint8Array
uint8arrayWrapper := js.Global.Get("Uint8Array").New(args[0])
value := make([]byte, uint8arrayWrapper.Get("byteLength").Int())
js.ValueOf(value).Call("set", uint8arrayWrapper)
bCh <- value
})
defer success.Close()
failure := js.NewCallback(func(args []js.Value) {
// Assumes it's a DOMException.
errCh <- errors.New(args[0].Get("message").String())
})
defer failure.Close()
r.arrayPromise.Call("then", success, failure)
select {
case b := <-bCh:
r.pending = b
case err := <-errCh:
return 0, err
}
}
if len(r.pending) == 0 {same comments as above
Patch Set #2, Line 129:
type Transport struct{}this minimal definition will break any code that sets those fields explicitly:
https://golang.org/pkg/net/http/#Transport
I think we should re-use the declaration of the Transport type and only explicitly define the RoundTrip method in +build tag protected files.
In the future the wasm code could respect those knobs, or return with an error early if any options are unsupported.
That work?
Additionally, you may want to compile-in the old RoundTrip implementation into ws,asm always, so the fake network for localhost still works.
This change would preclude running a fake in-memory HTTP server on localhost and connecting to it from Go, which all the net/http tests depend on in neelance's latest CLs.
You could make rename the other RoundTrip to roundTrip and only +build protect the top-level RoundTrip methods.
Then in this one, if the request.URL is 127.0.0.1 or whatever, you can just:
return t.roundTrip(req) // use fake in-memory network for tests
you can drop the "t ".
Patch Set #2, Line 149:
Stream request body when possible.reference the upstream web/DOM/JS specs about this.
(this has been my pet peeve for years, not having such support in browsers)
drop the "_ ="
likewise
respCh = make(chan *http.Response)
errCh = make(chan error)buffered
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Dmitri Shuralyov (Gerrit)
http.Transport type is documented as:
// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.
When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.
Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.
I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.
Thoughts (Brad, Johan, Richard)?
1 comment:
File src/net/http/transport_js.go:
Patch Set #2, Line 27:
type streamReader struct {Can you please rearrange the code in this file so that the RoundTrip method comes first, and streamReader, arrayReader definitions come afterwards? I think it's more readable when the higher level functionality comes first and puts the lower level helpers in context.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #4 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 7c13eba6d4dfef8c6d47dc1b4b242c1b5eb4147d
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
3 files changed, 377 insertions(+), 128 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
Redesigned following Brad's suggestions.
7 comments:
File src/net/http/transport.go:
This isn't consistent with the build tag in the other file, which says "js,wasm". […]
Ah, of course. I was using this less restrictive tag because it seemed to be the one used elsewhere to exclude the JS target. Good point!
is this guaranteed to have at least 1 arg?
This appears to return a "TypeError" exception at closer inspection: https://streams.spec.whatwg.org/#byob-reader-read. I will adjust this accordingly.
As for the number of arguments, the best I can find is https://www.w3.org/2001/tag/doc/promises-guide/#shorthand-reacting, which I think implies that the exception used to reject the promise will be the only parameter to the onRejected callback.
end in period.
Done
can you also note it on r.err, adding a field to streamReader: […]
Done
same comments as above
Done
reference the upstream web/DOM/JS specs about this. […]
I'm actually not sure what to reference here... googling around a bit I've found a StackOverflow post from late 2016 attempting to use a ReadableStream in the _request_ body, and comments saying this may be supported in browsers soon. I might attempt to implement a ReadableStream struct for browsers that support it as a follow up to this, to see whether it is indeed possible or not.
Otherwise I guess we have to reference https://fetch.spec.whatwg.org/#concept-body?
Your thoughts?
buffered
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #5 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: f847d940aa727e707b62294f73be6b754a1247ef
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
3 files changed, 255 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #6 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 4532afaacea3b1d7bf9296e88ed59fb29dfeb135
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
3 files changed, 254 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
File src/net/http/transport_js.go:
Can you please rearrange the code in this file so that the RoundTrip method comes first, and streamR […]
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
I'm slightly confused, as with the changes applied I can't seem to run `./all.bash`. I'm seeing:
johan@johan-x1 ~/gows/src/github.com/johanbrandhorst/go/src $ ./all.bash
Building Go cmd/dist using /usr/local/go/.
Building Go toolchain1 using /usr/local/go/.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.
# net/http
src/net/http/client.go:307:3: impossible type switch case: rt (type RoundTripper) cannot have dynamic type *Transport (wrong type for RoundTrip method)
have roundTrip(*Request) (*Response, error)
want RoundTrip(*Request) (*Response, error)
src/net/http/transport.go:39:5: cannot use Transport literal (type *Transport) as type RoundTripper in assignment:
*Transport does not implement RoundTripper (missing RoundTrip method)
have roundTrip(*Request) (*Response, error)
want RoundTrip(*Request) (*Response, error)
go tool dist: FAILED: /home/johan/gows/src/github.com/johanbrandhorst/go/pkg/tool/linux_amd64/go_bootstrap install -gcflags=all= -ldflags=all= std cmd: exit status 2
Now I think I have correctly defined Transport both in the case of js,wasm and !js,wasm. Am I missing anything obvious?
Johan Brandhorst (Gerrit)
Patch Set 3:
(1 comment)
http.Transport type is documented as:
// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.
Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.
I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.
Thoughts (Brad, Johan, Richard)?
I'm not sure I understand how this would apply here - wouldn't we still need a custom Transport implementation? If we defined a js,wasm specific client presumably it'd still need to have all the same fields as the normal client.
Dmitri Shuralyov (Gerrit)
1 comment:
File src/net/http/roundtrip.go:
Patch Set #6, Line 5:
// +build !js,wasm
Now I think I have correctly defined Transport both in the case of js,wasm and !js,wasm. Am I missing anything obvious?
The inverse of +build js,wasm build constraint is +build !js !wasm.
The current constraint means "not js AND wasm", which isn't satisfied by any GOOS/GOARCH pair. Hence the error:
*Transport ... (missing RoundTrip method)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #7 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: dd97b7f0a239e63bd7862e836cd9fc0515afa215
GitHub-Pull-Request: golang/go#25550
---
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
3 files changed, 254 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
File src/net/http/roundtrip.go:
Patch Set #6, Line 5:
// +build !js !wasm> Now I think I have correctly defined Transport both in the case of js,wasm and !js,wasm. […]
This is painfully obvious when you state it like that.. thank you for correcting me!
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
6 comments:
make these both buffered of size 1, so sends don't block in orphaned goroutines.
Done
Done
Done
this minimal definition will break any code that sets those fields explicitly: […]
Having thought about this a little more, it's not entirely unreasonable for a user to want to make a request to the local loopback, right? Do we have some way to definitely tell the request is a test request rather than a user hosting their own WASM code?
you can drop the "t ".
Done
drop the "_ ="
Done
likewise
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #8 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 254d2df16ef60727b7365ce6153787b6a80cbc71
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 255 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
File src/net/http/transport_js.go:
I'm actually not sure what to reference here... […]
I had a quick go at implementing this locally and it just doesn't seem to be working even where the ReadableStream is supported (Chromium 66.0.3359.181). I think for Blink the relevant issue is https://bugs.chromium.org/p/chromium/issues/detail?id=688906, but do we just want to reference the Spec (see previous comment)?
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Agniva De Sarker (Gerrit)
A minor review from a wasm enthusiast. Thanks for your work. :)
3 comments:
File src/net/http/roundtrip_js.go:
Patch Set #8, Line 32:
AbortControllerA check with MDN compatibility table shows that this is experimental technology and WebAssembly is available in earlier versions than this.
https://developer.mozilla.org/en-US/docs/Web/API/AbortController#Browser_compatibility
https://developer.mozilla.org/en-US/docs/WebAssembly#Browser_compatibilityAre we OK with not considering the case when a user browser does not have AbortController ?
Patch Set #8, Line 34:
opt := js.Global.Get("Object").New()What about the "mode" option ? Should we allow the user to specify cors/same-origin ?
Patch Set #8, Line 66:
contentLength := int64(-1)According to RFC, a Content-Length value of -1 is invalid.
Any Content-Length greater than or equal to zero is a valid value. Section 4.4 describes how to determine the length of a message-body if a Content-Length is not given.
https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.13.
We should probably let it be 0.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
Patch Set 8:
(3 comments)
A minor review from a wasm enthusiast. Thanks for your work. :)
Thanks, great points all!
3 comments:
Patch Set #8, Line 32:
AbortController
A check with MDN compatibility table shows that this is experimental technology and WebAssembly is a […]
This is a great point, I've made a workaround for when AbortController is not available.
Patch Set #8, Line 34:
opt := js.Global.Get("Object").New()What about the "mode" option ? Should we allow the user to specify cors/same-origin ?
I'm not sure there's any way to map these settings from existing http.Transport fields? Do you have any ideas?
Patch Set #8, Line 66:
contentLength := int64(-1)
According to RFC, a Content-Length value of -1 is invalid. […]
Great point, I've changed it to 0.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #9 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: a3dde7244f9a7fbc3b975b03f7fcf28bad279d78
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 267 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
Ah, of course. […]
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
File src/net/http/transport_js.go:
Having thought about this a little more, it's not entirely unreasonable for a user to want to make a […]
Resolved by mistake.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Agniva De Sarker (Gerrit)
1 comment:
File src/net/http/roundtrip_js.go:
Patch Set #8, Line 34:
// Some browsers that support WASM don't necessarily supportI'm not sure there's any way to map these settings from existing http. […]
That's true. I don't see a way to do this without adding a new field.
Perhaps we can have a "js,wasm" specific Request struct with an additional initMap object which can contain the init fields for fetch. Then the code can look for specific keys and set the attributes properly while making the fetch request.
As an user making a fetch request, I would like to be able to set the options. The cors/same-origin options are especially essential while making requests to external API servers.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
1 comment:
Patch Set #8, Line 34:
// Some browsers that support WASM don't necessarily support
That's true. I don't see a way to do this without adding a new field. […]
I agree, and maybe we should expose these settings somehow. I don't know what the best way is to allow per-platform behavior like this, but maybe this comes back to Dmitri's point about the transport being the wrong abstraction.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #10 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: c068b7f1f188413c2dbc87296edc7b1c57522ca8
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 273 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 10:Run-TryBot +1Code-Review +1
6 comments:
File src/net/http/roundtrip_js.go:
Patch Set #10, Line 20:
// It supports streaming response bodies.This doesn't seem worth mentioning. Seems like an implementation detail. We don't say that for the other implementation, so why this one?
Only document things that might be unexpected.
Patch Set #10, Line 21:
// See https://fetch.spec.whatwg.org/ for more information on this standard.Is that necessary for using this?
Patch Set #10, Line 133:
func isLocal(req *Request) bool {document this a bit to say what its purpose is.
Patch Set #10, Line 135:
case "127.0.0.1": // TODO(johanbrandhorst): Add more local hosts?"localhost" too.
And then don't match 127.0.0.1 because 127.1.2.3 is also a valid loopback address.
Use https://golang.org/pkg/net/#IP.IsLoopback
You'll need to attempt to parse the hostname as an IP first and only if it parses as an IP, then call IsLoopback.
can omit this "r "
Patch Set #10, Line 253:
// This is a noopcan delete. pretty obvious.
Or you can do:
if r.err == nil {
r.err = errClosed
}So future Reads will fail with an error that it's closed.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=509e3082
Brad Fitzpatrick (Gerrit)
2 comments:
Resolved by mistake.
Maybe the "isLocalReq" should be called "useFakeNetwork" instead and then we can additionally make it return false in non-test binaries or something.
I had a quick go at implementing this locally and it just doesn't seem to be working even where the […]
Referencing that bug and "ReadableStream" or anything searchable is fine, thanks.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch Set 3:
(1 comment)
http.Transport type is documented as:
// A Transport is a low-level primitive for making HTTP and HTTPS requests.
// For high-level functionality, such as cookies and redirects, see Client.When I originally wrote this functionality in GopherJS, I started by doing it at the http.RoundTripper level and was hoping to implement as much of its semantics as possible. Getting response body streaming was my main motivation, and the XHR transport was already implemented at http.RoundTripper level too, so it was a quick decision.
Over time, I learned that both XHR and Fetch browser APIs are higher level and don't allow making individual HTTP requests. Due to security reasons and things like CORS, the frontend code doesn't actually ever get Redirect responses, only the final redirect destination. Fetch also deals with caching, credentials, etc.
I didn't want to touch this decision for GopherJS because it was more of a prototype, but since this is the real Go tree, perhaps we should revisit the decision of what abstraction level to implement this at. I haven't prototyped it yet, but given what I've seen so far, I suspect that implementing the HTTP client in browsers at the http.Client level may be a closer fit to what browsers allow client code to do.
Thoughts (Brad, Johan, Richard)?
Good point, but I'm more concerned with more code working by default without changes, so I think this is okay to violate for now in Go 1.11. We can revisit in Go 1.12 & later.
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/509e3082/linux-amd64_79524419.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
7 of 17 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/509e3082/linux-amd64_79524419.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/509e3082/linux-386_89d4285c.log
Failed on freebsd-amd64-11_1: https://storage.googleapis.com/go-build-log/509e3082/freebsd-amd64-11_1_7f0993ef.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/509e3082/windows-386-2008_62d01cea.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/509e3082/linux-amd64-race_4c8daf21.log
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/509e3082/openbsd-amd64-62_34429510.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/509e3082/windows-amd64-2016_6dc87715.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 10:TryBot-Result -1
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #11 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 630957c697648655766e43501ce3d4897de7e8c3
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 276 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
8 comments:
Patch Set #10, Line 20:
// It supports streaming response bodies.
This doesn't seem worth mentioning. Seems like an implementation detail. […]
Done
Patch Set #10, Line 21:
// See https://fetch.spec.whatwg.org/ for more information on this standard.Is that necessary for using this?
I've removed this from the public documentation.
Patch Set #10, Line 133:
func isLocal(req *Request) bool {document this a bit to say what its purpose is.
Done
Patch Set #10, Line 135:
case "127.0.0.1": // TODO(johanbrandhorst): Add more local hosts?
"localhost" too. […]
Done
can omit this "r "
Done
Patch Set #10, Line 253:
// This is a noop
can delete. pretty obvious. […]
Done
File src/net/http/transport_js.go:
Maybe the "isLocalReq" should be called "useFakeNetwork" instead and then we can additionally make i […]
Yeah, that sounds better. How do I check whether a test-binary is in use?
Referencing that bug and "ReadableStream" or anything searchable is fine, thanks.
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #12 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 49b5acc617a430d39fbf2db2204992ec56876573
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 279 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #13 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: a710973f4d6001933f0e55e2288602cd476a2090
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 281 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 13:Run-TryBot +1
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=80f0fd2f
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 13:TryBot-Result +1
Brad Fitzpatrick (Gerrit)
Patch set 13:Code-Review +1
2 comments:
File src/net/http/roundtrip_js.go:
Patch Set #13, Line 139:
if ip := net.ParseIP(req.Host); ip != nil {req.Host might also be of the form:
127.0.0.1:13245
Especially for tests it'll be like that.
I think you need to first attempt a net.SplitHostPort, and if that succeeds, then use its host for net.ParseIP. If it fails, then assume req.Host is just a host.
Patch Set #13, Line 142:
return req.Host == "localhost"likewise
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
2 comments:
Patch Set #13, Line 139:
if ip := net.ParseIP(req.Host); ip != nil {
req.Host might also be of the form: […]
Done
Patch Set #13, Line 142:
return req.Host == "localhost"likewise
Done
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #14 to this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 6df646745b8e0474781f4b1a3084536e573e8e8c
GitHub-Pull-Request: golang/go#25550
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 285 insertions(+), 5 deletions(-)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 14:Code-Review +2
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick merged this change.
net/http: add js/wasm compatible DefaultTransport
Adds a new Transport type for the js/wasm target that uses the
JavaScript Fetch API for sending HTTP requests. Support for
streaming response bodies is used when available, falling back
to reading the entire response into memory at once.
Updates #25506
Change-Id: Ie9ea433a1a2ed2f65b03c6cc84a16e70c06fcf5c
GitHub-Last-Rev: 6df646745b8e0474781f4b1a3084536e573e8e8c
GitHub-Pull-Request: golang/go#25550
Reviewed-on: https://go-review.googlesource.com/114515
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
---
M src/go/build/deps_test.go
A src/net/http/roundtrip.go
A src/net/http/roundtrip_js.go
M src/net/http/transport.go
4 files changed, 285 insertions(+), 5 deletions(-)
diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index 5137ccf..ce67435 100644
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -411,6 +411,7 @@
"net/http/httptrace",
"net/http/internal",
"runtime/debug",
+ "syscall/js",
},
"net/http/internal": {"L4"},
"net/http/httptrace": {"context", "crypto/tls", "internal/nettrace", "net", "reflect", "time"},
diff --git a/src/net/http/roundtrip.go b/src/net/http/roundtrip.go
new file mode 100644
index 0000000..c8e691c
--- /dev/null
+++ b/src/net/http/roundtrip.go
@@ -0,0 +1,15 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !js !wasm
+
+package http
+
+// RoundTrip implements the RoundTripper interface.
+//
+// For higher-level HTTP client support (such as handling of cookies
+// and redirects), see Get, Post, and the Client type.
+func (t *Transport) RoundTrip(req *Request) (*Response, error) {
+ return t.roundTrip(req)
+}
diff --git a/src/net/http/roundtrip_js.go b/src/net/http/roundtrip_js.go
new file mode 100644
index 0000000..e60b736
--- /dev/null
+++ b/src/net/http/roundtrip_js.go
@@ -0,0 +1,267 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build js,wasm
+
+package http
+
+import (
+ "errors"
+ "fmt"
+ "io"
+ "io/ioutil"
+ "net"
+ "strconv"
+ "syscall/js"
+)
+
+// RoundTrip implements the RoundTripper interface using the WHATWG Fetch API.
+func (*Transport) RoundTrip(req *Request) (*Response, error) {
+ if useFakeNetwork(req) {
+ return t.roundTrip(req)
+ }
+ headers := js.Global.Get("Headers").New()
+ for key, values := range req.Header {
+ for _, value := range values {
+ headers.Call("append", key, value)
+ }
+ }
+
+ ac := js.Global.Get("AbortController")
+ if ac != js.Undefined {
+ // Some browsers that support WASM don't necessarily support
+ // the AbortController. See
+ // https://developer.mozilla.org/en-US/docs/Web/API/AbortController#Browser_compatibility.
+ ac = ac.New()
+ }
+
+ opt := js.Global.Get("Object").New()
+ // See https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch
+ // for options available.
+ opt.Set("headers", headers)
+ opt.Set("method", req.Method)
+ opt.Set("credentials", "same-origin")
+ if ac != js.Undefined {
+ opt.Set("signal", ac.Get("signal"))
+ }
+
+ if req.Body != nil {
+ // 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.
+ body, err := ioutil.ReadAll(req.Body)
+ if err != nil {
+ req.Body.Close() // RoundTrip must always close the body, including on errors.
+ return nil, err
+ }
+ req.Body.Close()
+ opt.Set("body", body)
+ }
+ respPromise := js.Global.Call("fetch", req.URL.String(), opt)
+ var (
+ respCh = make(chan *Response, 1)
+ errCh = make(chan error, 1)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ result := args[0]
+ header := Header{}
+ // https://developer.mozilla.org/en-US/docs/Web/API/Headers/entries
+ headersIt := result.Get("headers").Call("entries")
+ for {
+ n := headersIt.Call("next")
+ if n.Get("done").Bool() {
+ break
+ }
+ pair := n.Get("value")
+ key, value := pair.Index(0).String(), pair.Index(1).String()
+ ck := CanonicalHeaderKey(key)
+ header[ck] = append(header[ck], value)
+ }
+
+ contentLength := int64(0)
+ if cl, err := strconv.ParseInt(header.Get("Content-Length"), 10, 64); err == nil {
+ contentLength = cl
+ }
+
+ b := result.Get("body")
+ var body io.ReadCloser
+ if b != js.Undefined {
+ body = &streamReader{stream: b.Call("getReader")}
+ } else {
+ // Fall back to using ArrayBuffer
+ // https://developer.mozilla.org/en-US/docs/Web/API/Body/arrayBuffer
+ body = &arrayReader{arrayPromise: result.Call("arrayBuffer")}
+ }
+
+ select {
+ case respCh <- &Response{
+ Status: result.Get("status").String() + " " + StatusText(result.Get("status").Int()),
+ StatusCode: result.Get("status").Int(),
+ Header: header,
+ ContentLength: contentLength,
+ Body: body,
+ Request: req,
+ }:
+ case <-req.Context().Done():
+ }
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ err := fmt.Errorf("net/http: fetch() failed: %s", args[0].String())
+ select {
+ case errCh <- err:
+ case <-req.Context().Done():
+ }
+ })
+ defer failure.Close()
+ respPromise.Call("then", success, failure)
+ select {
+ case <-req.Context().Done():
+ if ac != js.Undefined {
+ // Abort the Fetch request
+ ac.Call("abort")
+ }
+ return nil, req.Context().Err()
+ case resp := <-respCh:
+ return resp, nil
+ case err := <-errCh:
+ return nil, err
+ }
+}
+
+// useFakeNetwork is used to determine whether the request is made
+// by a test and should be made to use the fake in-memory network.
+func useFakeNetwork(req *Request) bool {
+ host, _, err := net.SplitHostPort(req.Host)
+ if err != nil {
+ host = req.Host
+ }
+ if ip := net.ParseIP(host); ip != nil {
+ return ip.IsLoopback(ip)
+ }
+ return host == "localhost"
+}
+
+// streamReader implements an io.ReadCloser wrapper for ReadableStream.
+// See https://fetch.spec.whatwg.org/#readablestream for more information.
+type streamReader struct {
+ pending []byte
+ stream js.Value
+ err error // sticky read error
+}
+
+func (r *streamReader) Read(p []byte) (n int, err error) {
+ if r.err != nil {
+ return 0, r.err
+ }
+ if len(r.pending) == 0 {
+ var (
+ bCh = make(chan []byte, 1)
+ errCh = make(chan error, 1)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ result := args[0]
+ if result.Get("done").Bool() {
+ errCh <- io.EOF
+ return
+ }
+ value := make([]byte, result.Get("value").Get("byteLength").Int())
+ js.ValueOf(value).Call("set", result.Get("value"))
+ bCh <- value
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ // Assumes it's a TypeError. See
+ // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypeError
+ // for more information on this type. See
+ // https://streams.spec.whatwg.org/#byob-reader-read for the spec on
+ // the read method.
+ errCh <- errors.New(args[0].Get("message").String())
+ })
+ defer failure.Close()
+ r.stream.Call("read").Call("then", success, failure)
+ select {
+ case b := <-bCh:
+ r.pending = b
+ case err := <-errCh:
+ r.err = err
+ return 0, err
+ }
+ }
+ n = copy(p, r.pending)
+ r.pending = r.pending[n:]
+ return n, nil
+}
+
+func (r *streamReader) Close() error {
+ // This ignores any error returned from cancel method. So far, I did not encounter any concrete
+ // situation where reporting the error is meaningful. Most users ignore error from resp.Body.Close().
+ // If there's a need to report error here, it can be implemented and tested when that need comes up.
+ r.stream.Call("cancel")
+ if r.err == nil {
+ r.err = errClosed
+ }
+ return nil
+}
+
+// arrayReader implements an io.ReadCloser wrapper for ArrayBuffer.
+// https://developer.mozilla.org/en-US/docs/Web/API/Body/arrayBuffer.
+type arrayReader struct {
+ arrayPromise js.Value
+ pending []byte
+ read bool
+ err error // sticky read error
+}
+
+func (r *arrayReader) Read(p []byte) (n int, err error) {
+ if r.err != nil {
+ return 0, r.err
+ }
+ if !r.read {
+ r.read = true
+ var (
+ bCh = make(chan []byte, 1)
+ errCh = make(chan error, 1)
+ )
+ success := js.NewCallback(func(args []js.Value) {
+ // Wrap the input ArrayBuffer with a Uint8Array
+ uint8arrayWrapper := js.Global.Get("Uint8Array").New(args[0])
+ value := make([]byte, uint8arrayWrapper.Get("byteLength").Int())
+ js.ValueOf(value).Call("set", uint8arrayWrapper)
+ bCh <- value
+ })
+ defer success.Close()
+ failure := js.NewCallback(func(args []js.Value) {
+ // Assumes it's a TypeError. See
+ // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypeError
+ // for more information on this type.
+ // See https://fetch.spec.whatwg.org/#concept-body-consume-body for reasons this might error.
+ errCh <- errors.New(args[0].Get("message").String())
+ })
+ defer failure.Close()
+ r.arrayPromise.Call("then", success, failure)
+ select {
+ case b := <-bCh:
+ r.pending = b
+ case err := <-errCh:
+ return 0, err
+ }
+ }
+ if len(r.pending) == 0 {
+ return 0, io.EOF
+ }
+ n = copy(p, r.pending)
+ r.pending = r.pending[n:]
+ return n, nil
+}
+
+func (r *arrayReader) Close() error {
+ if r.err == nil {
+ r.err = errClosed
+ }
+ return nil
+}
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 5bf9ff9..731bf17 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -311,11 +311,8 @@
tr.mu.Unlock()
}
-// RoundTrip implements the RoundTripper interface.
-//
-// For higher-level HTTP client support (such as handling of cookies
-// and redirects), see Get, Post, and the Client type.
-func (t *Transport) RoundTrip(req *Request) (*Response, error) {
+// roundTrip implements a RoundTripper over HTTP.
+func (t *Transport) roundTrip(req *Request) (*Response, error) {
t.nextProtoOnce.Do(t.onceSetNextProtoDefaults)
ctx := req.Context()
trace := httptrace.ContextClientTrace(ctx)
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.
Johan Brandhorst (Gerrit)
2 comments:
File src/net/http/roundtrip_js.go:
Patch Set #8, Line 34:
// the AbortController. See
I agree, and maybe we should expose these settings somehow. […]
I think you'll have to raise an issue specifically for this kind of functionality Agniva. Put a comment on #25506 as a start.
File src/net/http/transport_js.go:
Yeah, that sounds better. […]
I will resolve this here and allow us to continue the discussion in #25506.
To view, visit change 114515. To unsubscribe, or for help writing mail filters, visit settings.