Fix URL encoding issues - fixes #1573

This fixes the confusion between paths which were URL encoded and
paths which weren't.  In particular it allows files to have % in the
name.

http/http.go

@@ -46,11 +46,12 @@ func init() {
 
 // Fs stores the interface to the remote HTTP files
 type Fs struct {
-	name       string
+	name        string
-	root       string
+	root        string
-	features   *fs.Features // optional features
+	features    *fs.Features // optional features
-	endpoint   *url.URL
+	endpoint    *url.URL
-	httpClient *http.Client
+	endpointURL string // endpoint as a string
+	httpClient  *http.Client
 }
 
 // Object is a remote object that has been stat'd (so it exists, but is not necessarily open for reading)
@@ -63,6 +64,8 @@ type Object struct {
 }
 
 // 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 {
@@ -142,15 +145,19 @@ func NewFs(name, root string) (fs.Fs, error) {
 	}
 
 	f := &Fs{
-		name:       name,
+		name:        name,
-		root:       root,
+		root:        root,
-		httpClient: client,
+		httpClient:  client,
-		endpoint:   u,
+		endpoint:    u,
+		endpointURL: u.String(),
 	}
 	f.features = (&fs.Features{}).Fill(f)
 	if isFile {
 		return f, fs.ErrorIsFile
 	}
+	if !strings.HasSuffix(f.endpointURL, "/") {
+		return nil, errors.New("internal error: url doesn't end with /")
+	}
 	return f, nil
 }
 
@@ -166,7 +173,7 @@ func (f *Fs) Root() string {
 
 // String returns the URL for the filesystem
 func (f *Fs) String() string {
-	return f.endpoint.String()
+	return f.endpointURL
 }
 
 // Features returns the optional features of this Fs
@@ -192,6 +199,11 @@ func (f *Fs) NewObject(remote string) (fs.Object, error) {
 	return o, nil
 }
 
+// Join's the remote onto the base URL
+func (f *Fs) url(remote string) string {
+	return f.endpointURL + urlEscape(remote)
+}
+
 func parseInt64(s string) int64 {
 	n, e := strconv.ParseInt(s, 10, 64)
 	if e != nil {
@@ -263,14 +275,15 @@ func parse(base *url.URL, in io.Reader) (names []string, err error) {
 
 // Read the directory passed in
 func (f *Fs) readDir(dir string) (names []string, err error) {
-	u, err := urlJoin(f.endpoint, dir)
+	URL := f.url(dir)
+	u, err := url.Parse(URL)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to readDir")
 	}
-	if !strings.HasSuffix(u.String(), "/") {
+	if !strings.HasSuffix(URL, "/") {
-		return nil, errors.Errorf("internal error: readDir URL %q didn't end in /", u.String())
+		return nil, errors.Errorf("internal error: readDir URL %q didn't end in /", URL)
 	}
-	res, err := f.httpClient.Get(u.String())
+	res, err := f.httpClient.Get(URL)
 	if err == nil && res.StatusCode == http.StatusNotFound {
 		return nil, fs.ErrorDirNotFound
 	}
@@ -323,6 +336,7 @@ func (f *Fs) List(dir string) (entries fs.DirEntries, err error) {
 				remote: remote,
 			}
 			if err = file.stat(); err != nil {
+				fs.Debugf(remote, "skipping because of error: %v", err)
 				continue
 			}
 			entries = append(entries, file)
@@ -373,19 +387,15 @@ func (o *Object) ModTime() time.Time {
 	return o.modTime
 }
 
-// path returns the native path of the object
+// url returns the native url of the object
-func (o *Object) path() string {
+func (o *Object) url() string {
-	return path.Join(o.fs.root, o.remote)
+	return o.fs.url(o.remote)
 }
 
 // stat updates the info field in the Object
 func (o *Object) stat() error {
-	url, err := urlJoin(o.fs.endpoint, o.remote)
+	url := o.url()
-	if err != nil {
+	res, err := o.fs.httpClient.Head(url)
-		return errors.Wrap(err, "failed to stat")
-	}
-	endpoint := url.String()
-	res, err := o.fs.httpClient.Head(endpoint)
 	err = statusError(res, err)
 	if err != nil {
 		return errors.Wrap(err, "failed to stat")
@@ -414,12 +424,8 @@ func (o *Object) Storable() bool {
 
 // Open a remote http file object for reading. Seek is supported
 func (o *Object) Open(options ...fs.OpenOption) (in io.ReadCloser, err error) {
-	url, err := urlJoin(o.fs.endpoint, o.remote)
+	url := o.url()
-	if err != nil {
+	req, err := http.NewRequest("GET", url, nil)
-		return nil, errors.Wrap(err, "Open failed")
-	}
-	endpoint := url.String()
-	req, err := http.NewRequest("GET", endpoint, nil)
 	if err != nil {
 		return nil, errors.Wrap(err, "Open failed")
 	}

http/http_internal_test.go

@@ -72,7 +72,7 @@ func testListRoot(t *testing.T, f fs.Fs) {
 	assert.True(t, ok)
 
 	e = entries[1]
-	assert.Equal(t, "one.txt", e.Remote())
+	assert.Equal(t, "one%.txt", e.Remote())
 	assert.Equal(t, int64(6), e.Size())
 	_, ok = e.(*Object)
 	assert.True(t, ok)
@@ -176,7 +176,7 @@ func TestIsAFileRoot(t *testing.T) {
 	tidy := prepareServer(t)
 	defer tidy()
 
-	f, err := NewFs(remoteName, "one.txt")
+	f, err := NewFs(remoteName, "one%.txt")
 	assert.Equal(t, err, fs.ErrorIsFile)
 
 	testListRoot(t, f)
@@ -323,6 +323,8 @@ func TestParseApache(t *testing.T) {
 		"stressdisk/",
 		"timer-test",
 		"words-to-regexp.pl",
+		"Now 100% better.mp3",
+		"Now better.mp3",
 	})
 }
 

http/test/index_files/apache.html

@@ -24,5 +24,9 @@
 <tr><td valign="top"><img src="/icons/unknown.gif" alt="[   ]"></td><td><a href="timer-test">timer-test</a></td><td align="right">09-May-2017 17:05  </td><td align="right">1.5M</td><td>&nbsp;</td></tr>
 <tr><td valign="top"><img src="/icons/text.gif" alt="[TXT]"></td><td><a href="words-to-regexp.pl">words-to-regexp.pl</a></td><td align="right">01-Mar-2005 20:43  </td><td align="right">6.0K</td><td>&nbsp;</td></tr>
 <tr><th colspan="5"><hr></th></tr>
+<!-- some extras from https://github.com/ncw/rclone/issues/1573 -->
+<tr><td valign="top"><img src="/icons/sound2.gif" alt="[SND]"></td><td><a href="Now%20100%25%20better.mp3">Now 100% better.mp3</a></td><td align="right">2017-08-01 11:41 </td><td align="right"> 0 </td><td>&nbsp;</td></tr>
+<tr><td valign="top"><img src="/icons/sound2.gif" alt="[SND]"></td><td><a href="Now%20better.mp3">Now better.mp3</a></td><td align="right">2017-08-01 11:41 </td><td align="right"> 0 </td><td>&nbsp;</td></tr>
+
 </table>
 </body></html>