diff --git a/box/box.go b/box/box.go index 3e3eb67c4..ea93dbb00 100644 --- a/box/box.go +++ b/box/box.go @@ -139,14 +139,6 @@ func parsePath(path string) (root string) { return } -// mimics url.PathEscape which only available from go 1.8 -func pathEscape(path string) string { - u := url.URL{ - Path: path, - } - return u.EscapedPath() -} - // retryErrorCodes is a slice of error codes that we will retry var retryErrorCodes = []int{ 429, // Too Many Requests. diff --git a/http/http.go b/http/http.go index 442f5be99..c26f616e9 100644 --- a/http/http.go +++ b/http/http.go @@ -17,6 +17,7 @@ import ( "time" "github.com/ncw/rclone/fs" + "github.com/ncw/rclone/rest" "github.com/pkg/errors" "golang.org/x/net/html" ) @@ -63,24 +64,6 @@ type Object struct { contentType string } -// Join a URL and a path returning a new URL -// -// path should be URL escaped -func urlJoin(base *url.URL, path string) (*url.URL, error) { - rel, err := url.Parse(path) - if err != nil { - return nil, errors.Wrapf(err, "Error parsing %q as URL", path) - } - return base.ResolveReference(rel), nil -} - -// urlEscape escapes URL path the in string using URL escaping rules -func urlEscape(in string) string { - var u url.URL - u.Path = in - return u.String() -} - // statusError returns an error if the res contained an error func statusError(res *http.Response, err error) error { if err != nil { @@ -106,7 +89,7 @@ func NewFs(name, root string) (fs.Fs, error) { if err != nil { return nil, err } - u, err := urlJoin(base, urlEscape(root)) + u, err := rest.URLJoin(base, rest.URLEscape(root)) if err != nil { return nil, err } @@ -203,7 +186,7 @@ func (f *Fs) NewObject(remote string) (fs.Object, error) { // Join's the remote onto the base URL func (f *Fs) url(remote string) string { - return f.endpointURL + urlEscape(remote) + return f.endpointURL + rest.URLEscape(remote) } func parseInt64(s string) int64 { @@ -216,7 +199,7 @@ func parseInt64(s string) int64 { // parseName turns a name as found in the page into a remote path or returns false func parseName(base *url.URL, name string) (string, bool) { - u, err := urlJoin(base, name) + u, err := rest.URLJoin(base, name) if err != nil { return "", false } diff --git a/http/http_internal_test.go b/http/http_internal_test.go index 607cfd831..2555f6c21 100644 --- a/http/http_internal_test.go +++ b/http/http_internal_test.go @@ -16,6 +16,7 @@ import ( "github.com/ncw/rclone/fs" "github.com/ncw/rclone/fstest" + "github.com/ncw/rclone/rest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -203,57 +204,6 @@ func TestIsAFileSubDir(t *testing.T) { assert.True(t, ok) } -func TestURLJoin(t *testing.T) { - for i, test := range []struct { - base string - path string - wantOK bool - want string - }{ - {"http://example.com/", "potato", true, "http://example.com/potato"}, - {"http://example.com/dir/", "potato", true, "http://example.com/dir/potato"}, - {"http://example.com/dir/", "../dir/potato", true, "http://example.com/dir/potato"}, - {"http://example.com/dir/", "..", true, "http://example.com/"}, - {"http://example.com/dir/", "http://example.com/", true, "http://example.com/"}, - {"http://example.com/dir/", "http://example.com/dir/", true, "http://example.com/dir/"}, - {"http://example.com/dir/", "http://example.com/dir/potato", true, "http://example.com/dir/potato"}, - {"http://example.com/dir/", "/dir/", true, "http://example.com/dir/"}, - {"http://example.com/dir/", "/dir/potato", true, "http://example.com/dir/potato"}, - {"http://example.com/dir/", "subdir/potato", true, "http://example.com/dir/subdir/potato"}, - {"http://example.com/dir/", "With percent %25.txt", true, "http://example.com/dir/With%20percent%20%25.txt"}, - {"http://example.com/dir/", "With colon :", false, ""}, - {"http://example.com/dir/", urlEscape("With colon :"), true, "http://example.com/dir/With%20colon%20:"}, - } { - u, err := url.Parse(test.base) - require.NoError(t, err) - got, err := urlJoin(u, test.path) - gotOK := err == nil - what := fmt.Sprintf("test %d base=%q, val=%q", i, test.base, test.path) - assert.Equal(t, test.wantOK, gotOK, what) - var gotString string - if gotOK { - gotString = got.String() - } - assert.Equal(t, test.want, gotString, what) - } -} - -func TestURLEscape(t *testing.T) { - for i, test := range []struct { - path string - want string - }{ - {"", ""}, - {"/hello.txt", "/hello.txt"}, - {"With Space", "With%20Space"}, - {"With Colon:", "./With%20Colon:"}, - {"With Percent%", "With%20Percent%25"}, - } { - got := urlEscape(test.path) - assert.Equal(t, test.want, got, fmt.Sprintf("Test %d path = %q", i, test.path)) - } -} - func TestParseName(t *testing.T) { for i, test := range []struct { base string @@ -273,7 +223,7 @@ func TestParseName(t *testing.T) { {"http://example.com/dir/", "subdir/potato", false, ""}, {"http://example.com/dir/", "With percent %25.txt", true, "With percent %.txt"}, {"http://example.com/dir/", "With colon :", false, ""}, - {"http://example.com/dir/", urlEscape("With colon :"), true, "With colon :"}, + {"http://example.com/dir/", rest.URLEscape("With colon :"), true, "With colon :"}, } { u, err := url.Parse(test.base) require.NoError(t, err) diff --git a/onedrive/onedrive.go b/onedrive/onedrive.go index 04d6e2806..8859a8106 100644 --- a/onedrive/onedrive.go +++ b/onedrive/onedrive.go @@ -276,14 +276,6 @@ func parsePath(path string) (root string) { return } -// mimics url.PathEscape which only available from go 1.8 -func pathEscape(path string) string { - u := url.URL{ - Path: path, - } - return u.EscapedPath() -} - // retryErrorCodes is a slice of error codes that we will retry var retryErrorCodes = []int{ 429, // Too Many Requests. @@ -310,7 +302,7 @@ func shouldRetry(resp *http.Response, err error) (bool, error) { func (f *Fs) readMetaDataForPath(path string) (info *api.Item, resp *http.Response, err error) { opts := rest.Opts{ Method: "GET", - Path: "/root:/" + pathEscape(replaceReservedChars(path)), + Path: "/root:/" + rest.URLEscape(replaceReservedChars(path)), } err = f.pacer.Call(func() (bool, error) { resp, err = f.srv.CallJSON(&opts, nil, &info) @@ -1026,7 +1018,7 @@ func (o *Object) ModTime() time.Time { func (o *Object) setModTime(modTime time.Time) (*api.Item, error) { opts := rest.Opts{ Method: "PATCH", - Path: "/root:/" + pathEscape(o.srvPath()), + Path: "/root:/" + rest.URLEscape(o.srvPath()), } update := api.SetFileSystemInfo{ FileSystemInfo: api.FileSystemInfoFacet{ @@ -1081,7 +1073,7 @@ func (o *Object) Open(options ...fs.OpenOption) (in io.ReadCloser, err error) { func (o *Object) createUploadSession() (response *api.CreateUploadResponse, err error) { opts := rest.Opts{ Method: "POST", - Path: "/root:/" + pathEscape(o.srvPath()) + ":/upload.createSession", + Path: "/root:/" + rest.URLEscape(o.srvPath()) + ":/upload.createSession", } var resp *http.Response err = o.fs.pacer.Call(func() (bool, error) { @@ -1187,7 +1179,7 @@ func (o *Object) Update(in io.Reader, src fs.ObjectInfo, options ...fs.OpenOptio var resp *http.Response opts := rest.Opts{ Method: "PUT", - Path: "/root:/" + pathEscape(o.srvPath()) + ":/content", + Path: "/root:/" + rest.URLEscape(o.srvPath()) + ":/content", ContentLength: &size, Body: in, } diff --git a/rest/url.go b/rest/url.go new file mode 100644 index 000000000..14eeac4a1 --- /dev/null +++ b/rest/url.go @@ -0,0 +1,27 @@ +package rest + +import ( + "net/url" + + "github.com/pkg/errors" +) + +// URLJoin joins a URL and a path returning a new URL +// +// path should be URL escaped +func URLJoin(base *url.URL, path string) (*url.URL, error) { + rel, err := url.Parse(path) + if err != nil { + return nil, errors.Wrapf(err, "Error parsing %q as URL", path) + } + return base.ResolveReference(rel), nil +} + +// URLEscape escapes URL path the in string using URL escaping rules +// +// This mimics url.PathEscape which only available from go 1.8 +func URLEscape(in string) string { + var u url.URL + u.Path = in + return u.String() +} diff --git a/rest/url_test.go b/rest/url_test.go new file mode 100644 index 000000000..1b7704a9d --- /dev/null +++ b/rest/url_test.go @@ -0,0 +1,63 @@ +// +build go1.8 + +package rest + +import ( + "fmt" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestURLJoin(t *testing.T) { + for i, test := range []struct { + base string + path string + wantOK bool + want string + }{ + {"http://example.com/", "potato", true, "http://example.com/potato"}, + {"http://example.com/dir/", "potato", true, "http://example.com/dir/potato"}, + {"http://example.com/dir/", "../dir/potato", true, "http://example.com/dir/potato"}, + {"http://example.com/dir/", "..", true, "http://example.com/"}, + {"http://example.com/dir/", "http://example.com/", true, "http://example.com/"}, + {"http://example.com/dir/", "http://example.com/dir/", true, "http://example.com/dir/"}, + {"http://example.com/dir/", "http://example.com/dir/potato", true, "http://example.com/dir/potato"}, + {"http://example.com/dir/", "/dir/", true, "http://example.com/dir/"}, + {"http://example.com/dir/", "/dir/potato", true, "http://example.com/dir/potato"}, + {"http://example.com/dir/", "subdir/potato", true, "http://example.com/dir/subdir/potato"}, + {"http://example.com/dir/", "With percent %25.txt", true, "http://example.com/dir/With%20percent%20%25.txt"}, + {"http://example.com/dir/", "With colon :", false, ""}, + {"http://example.com/dir/", URLEscape("With colon :"), true, "http://example.com/dir/With%20colon%20:"}, + } { + u, err := url.Parse(test.base) + require.NoError(t, err) + got, err := URLJoin(u, test.path) + gotOK := err == nil + what := fmt.Sprintf("test %d base=%q, val=%q", i, test.base, test.path) + assert.Equal(t, test.wantOK, gotOK, what) + var gotString string + if gotOK { + gotString = got.String() + } + assert.Equal(t, test.want, gotString, what) + } +} + +func TestURLEscape(t *testing.T) { + for i, test := range []struct { + path string + want string + }{ + {"", ""}, + {"/hello.txt", "/hello.txt"}, + {"With Space", "With%20Space"}, + {"With Colon:", "./With%20Colon:"}, + {"With Percent%", "With%20Percent%25"}, + } { + got := URLEscape(test.path) + assert.Equal(t, test.want, got, fmt.Sprintf("Test %d path = %q", i, test.path)) + } +}