webdav: use url.URL to escape lockroot

The lockroot is a path in the webdav library, meaning it may have
slashes in it. Use the url.URL struct to handle this case when escaping
the lockroot. Also add some unit tests.

Author: klarose

webdav/webdav_test.go

@@ -21,21 +21,21 @@ import (
 	"testing"
 )
 
+// createLockBody comes from the example in Section 9.10.7.
+const createLockBody = `<?xml version="1.0" encoding="utf-8" ?>
+	<D:lockinfo xmlns:D='DAV:'>
+		<D:lockscope><D:exclusive/></D:lockscope>
+		<D:locktype><D:write/></D:locktype>
+		<D:owner>
+			<D:href>http://example.org/~ejw/contact.html</D:href>
+		</D:owner>
+	</D:lockinfo>
+`
+
 // TODO: add tests to check XML responses with the expected prefix path
 func TestPrefix(t *testing.T) {
 	const dst, blah = "Destination", "blah blah blah"
 
-	// createLockBody comes from the example in Section 9.10.7.
-	const createLockBody = `<?xml version="1.0" encoding="utf-8" ?>
-		<D:lockinfo xmlns:D='DAV:'>
-			<D:lockscope><D:exclusive/></D:lockscope>
-			<D:locktype><D:write/></D:locktype>
-			<D:owner>
-				<D:href>http://example.org/~ejw/contact.html</D:href>
-			</D:owner>
-		</D:lockinfo>
-	`
-
 	do := func(method, urlStr string, body string, wantStatusCode int, headers ...string) (http.Header, error) {
 		var bodyReader io.Reader
 		if body != "" {
@@ -347,3 +347,92 @@ func TestFilenameEscape(t *testing.T) {
 		}
 	}
 }
+
+func TestLockrootEscape(t *testing.T) {
+	lockrootRe := regexp.MustCompile(`<D:lockroot><D:href>([^<]*)</D:href></D:lockroot>`)
+	do := func(urlStr string) (string, error) {
+		bodyReader := strings.NewReader(createLockBody)
+		req, err := http.NewRequest("LOCK", urlStr, bodyReader)
+		if err != nil {
+			return "", err
+		}
+		res, err := http.DefaultClient.Do(req)
+		if err != nil {
+			return "", err
+		}
+		defer res.Body.Close()
+
+		b, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			return "", err
+		}
+		lockrootMatch := lockrootRe.FindStringSubmatch(string(b))
+		if len(lockrootMatch) != 2 {
+			return "", errors.New("D:lockroot not found")
+		}
+
+		return lockrootMatch[1], nil
+	}
+
+	testCases := []struct {
+		name, wantLockroot string
+	}{{
+		name:         `/foo%bar`,
+		wantLockroot: `/foo%25bar`,
+	}, {
+		name:         `/こんにちわ世界`,
+		wantLockroot: `/%E3%81%93%E3%82%93%E3%81%AB%E3%81%A1%E3%82%8F%E4%B8%96%E7%95%8C`,
+	}, {
+		name:         `/Program Files/`,
+		wantLockroot: `/Program%20Files/`,
+	}, {
+		name:         `/go+lang`,
+		wantLockroot: `/go+lang`,
+	}, {
+		name:         `/go&lang`,
+		wantLockroot: `/go&amp;lang`,
+	}, {
+		name:         `/go<lang`,
+		wantLockroot: `/go%3Clang`,
+	}}
+	ctx := context.Background()
+	fs := NewMemFS()
+	for _, tc := range testCases {
+		if tc.name != "/" {
+			if strings.HasSuffix(tc.name, "/") {
+				if err := fs.Mkdir(ctx, tc.name, 0755); err != nil {
+					t.Fatalf("name=%q: Mkdir: %v", tc.name, err)
+				}
+			} else {
+				f, err := fs.OpenFile(ctx, tc.name, os.O_CREATE, 0644)
+				if err != nil {
+					t.Fatalf("name=%q: OpenFile: %v", tc.name, err)
+				}
+				f.Close()
+			}
+		}
+	}
+
+	srv := httptest.NewServer(&Handler{
+		FileSystem: fs,
+		LockSystem: NewMemLS(),
+	})
+	defer srv.Close()
+
+	u, err := url.Parse(srv.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tc := range testCases {
+		u.Path = tc.name
+		gotLockroot, err := do(u.String())
+		if err != nil {
+			t.Errorf("name=%q: LOCK: %v", tc.name, err)
+			continue
+		}
+		if gotLockroot != tc.wantLockroot {
+			t.Errorf("name=%q: got lockroot %q, want %q", tc.name, gotLockroot, tc.wantLockroot)
+		}
+	}
+}

webdav/xml.go

@@ -91,7 +91,8 @@ func writeLockInfo(w io.Writer, token string, ld LockDetails) (int, error) {
 	// PathEscape the root. Any URLs in this response body should match data on the wire
 	// meaning if a request came in escaped (which it should have), it should go out that
 	// way as well.
-	root := url.PathEscape(ld.Root)
+	rootUrl := url.URL{Path: ld.Root}
+	root := rootUrl.EscapedPath()
 	return fmt.Fprintf(w, "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"+
 		"<D:prop xmlns:D=\"DAV:\"><D:lockdiscovery><D:activelock>\n"+
 		"	<D:locktype><D:write/></D:locktype>\n"+