Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0-length byte arrays not properly externalized? (Empty HTTP body causes errors with xhr2 polyfill.) #597

Closed
flimzy opened this issue Feb 26, 2017 · 2 comments

Comments

@flimzy
Copy link
Member

flimzy commented Feb 26, 2017

I'm trying to get an XHR polyfill to work for testing in node.js, and discovered that with the xhr2 library, an empty body on a PUT request generates an error. To reproduce:

main.go:

package main

import (
	"fmt"
	"net/http"
)

func main() {
	req, _ := http.NewRequest("PUT", "http://bogusurl.com/", nil)
	client := &http.Client{}
	_, err := client.Do(req)
	if err != nil {
		fmt.Printf("Error: %s\n", err)
	}
}

main.inc.js:

try {
    global.XMLHttpRequest = require('xhr2');
} catch(e) {}

Then run these commands:

npm install xhr2
gopherjs run main.go main.inc.js

And observe the following output:

/home/jonhall/go/src/github.com/flimzy/jstest/node_modules/xhr2/lib/xhr2.js:844                                                                                                                                                  
throw new Error("Unsupported send() data " + data);                                                                                                                                                                      
^                                                                                                                                                                                                                  
Error: Unsupported send() data                                                                                                                                                                                                   
at XMLHttpRequestUpload._setData (/home/jonhall/go/src/github.com/flimzy/jstest/node_modules/xhr2/lib/xhr2.js:844:15)                                                                                                        
at XMLHttpRequest._sendHttp (/home/jonhall/go/src/github.com/flimzy/jstest/node_modules/xhr2/lib/xhr2.js:375:19)                                                                                                             
at XMLHttpRequest.send (/home/jonhall/go/src/github.com/flimzy/jstest/node_modules/xhr2/lib/xhr2.js:203:16)
at $packages.net/http.XHRTransport.ptr.RoundTrip (/home/jonhall/go/src/github.com/flimzy/jstest/http.go:99:3)
at send (/net/http/client.go:249:3)
at $packages.net/http.Client.ptr.send (/net/http/client.go:173:3)
at $packages.net/http.Client.ptr.Do (/net/http/client.go:595:7)
at main (/github.com/flimzy/jstest/main.go:11:3)
at $init (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:99004:9)
at $goroutine (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:1474:19)
at $runScheduled (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:1514:7)
at $schedule (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:1530:5)
at $go (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:1506:3)
at Object.<anonymous> (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:99015:1)
at Object.<anonymous> (/home/jonhall/go/src/github.com/flimzy/jstest/main.go.863915836:99018:4)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.runMain (module.js:604:10)
at run (bootstrap_node.js:394:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:509:3

If I make a minor change to the GopherJS's (*XHRTransport).RoundTrip implementation, the problem goes away:

@@ -96,7 +96,7 @@ func (t *XHRTransport) RoundTrip(req *Request) (*Response, error) {
                }
                req.Body.Close()
        }
-       xhr.Call("send", body)
+       xhr.Call("send", string(body))
 
        select {
        case resp := <-respCh:

I don't know if this is an appropriate fix, or if it's just working around/obscuring a larger problem. I also haven't tested that this doesn't break other functionality (it probably does).

@flimzy
Copy link
Member Author

flimzy commented Feb 26, 2017

This is probably a much more appropriate change. I'll probably turn this into a PR:

@@ -86,17 +86,17 @@ func (t *XHRTransport) RoundTrip(req *Request) (*Response, error) {
                        xhr.Call("setRequestHeader", key, value)
                }
        }
-       var body []byte
-       if req.Body != nil {
-               var err error
-               body, err = ioutil.ReadAll(req.Body)
+       if req.Body == nil {
+               xhr.Call("send")
+       } else {
+               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()
+               xhr.Call("send", body)
        }
-       xhr.Call("send", body)
 
        select {
        case resp := <-respCh:

@flimzy
Copy link
Member Author

flimzy commented Feb 26, 2017

Regardless of any change to XHRTransport, perhaps the underlying problem is demonstrated in this program:

package main

import "honnef.co/go/js/console"

func main() {
	x := []byte{}
	var y []byte
	z := []byte{0x01}
	console.Log(x)
	console.Log(y)
	console.Log(z)
}

Output:

[]
[]
Uint8Array [ 1 ]

Based on the type conversion table, I would have expected all three values to be converted to Uint8Arrays.

@flimzy flimzy changed the title Empty HTTP body causes errors with xhr2 polyfill. 0-length byte arrays not properly externalized? (Empty HTTP body causes errors with xhr2 polyfill.) Feb 26, 2017
@dmitshur dmitshur marked this as a duplicate of #399 Jul 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant