`r2` sends invalid HTTP requests if `Path` does not have a leading `/`.

[dhermes]

To reproduce (at 480ad70d161406204ef58d3528b0ac17cf64bf0a), first listen to port 5007 with netcat and then run this sample program

package main

import (
	"fmt"
	"time"

	"github.com/blend/go-sdk/r2"
)

func mustNil(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	timeout := 5 * time.Second
	r := r2.New(
		"",
		r2.OptScheme("http"),
		r2.OptHost("localhost:5007"),
		r2.OptPath("v1/resource"),
		r2.OptTimeout(timeout),
	)

	res, err := r.Do()
	mustNil(err)
	fmt.Printf("%#v\n", res)

	// // Using r.Do() is equivalent to the following:
	// client := &http.Client{Timeout: timeout}
	// res, err := client.Do(&r.Request)
	// mustNil(err)
}

After running, we have

$ nc -l 5007
GET v1/resource HTTP/1.1
Host: localhost:5007
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

I will dig deeper into this at a later time, but just wanted to get the repro posted here.

[dhermes]

The equivalent with v2

package main

import (
	"fmt"
	"time"

	"github.com/blend/go-sdk/request"
)

func mustNil(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	r := request.New().
		WithScheme("http").
		WithHost("localhost:5007").
		WithPath("v1/resource").
		WithTimeout(5 * time.Second)

	res, err := r.Response()
	mustNil(err)
	fmt.Printf("%#v\n", res)

	// // Using r.Response() is equivalent to the following:
	// req, err := r.Request()
	// mustNil(err)
	// client := &http.Client{Timeout: r.Timeout()}
	// res, err := client.Do(req)
	// mustNil(err)
}

produces

$ nc -l 5007
GET /v1/resource HTTP/1.1
Host: localhost:5007
User-Agent: Go-http-client/1.1
Accept-Encoding: gzip

[dhermes]

So I think the primary difference between v2 and v3 (which is #141 and is heavily related to #142 and #143) is the usage of http.NewRequest()

func (r *Request) Request() (*http.Request, error) {
	...
	req, err := http.NewRequest(r.Method(), r.URL().String(), body)
	...
}

[dhermes]

One particular peculiarity is that net/url.URL is supposed to paper over the missing slash issue. Both with and without a leading slash

package main

import (
	"fmt"
	"net/url"
)

func main() {
	u := &url.URL{
		Scheme:   "https",
		Host:     "site.invalid",
		Path:     "v1/dogs",
		RawQuery: "a=b",
	}
	fmt.Printf("%q (%#v)\n", u.String(), u)
	u = &url.URL{
		Scheme:   "https",
		Host:     "site.invalid",
		Path:     "/v1/dogs",
		RawQuery: "a=b",
	}
	fmt.Printf("%q (%#v)\n", u.String(), u)
}

we get the same URL string even though u.Path differs

"https://site.invalid/v1/dogs?a=b" (&url.URL{... Path:"v1/dogs" ...})
"https://site.invalid/v1/dogs?a=b" (&url.URL{... Path:"/v1/dogs" ...})

[dvdliao]

just happened for me too on existing api requests

[UjjwalAyyangar]

So I think the primary difference between v2 and v3 is the usage of http.NewRequest(r.Method(), r.URL().String(), body)

That is right. In this usage, the parameter r.URL().String() handles the issue with leading /. (v1/resource -> .String() -> /v1/resource )

However in r2, the .String() function is absent from the execution flow between OptPath and New which led to this issue.