rest: factor URLJoin and URLEscape from http remote

2025-10-30 23:17:59 +02:00 · 2017-10-02 17:37:36 +01:00
parent 00fe6d95da
commit 4966611866
6 changed files with 100 additions and 93 deletions
--- 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.
--- 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
 	}
--- 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)
--- 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,
 		}
--- a/rest/url.go
+++ 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()
+}
--- a/rest/url_test.go
+++ 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))
+	}
+}