From c72303b1fd07838a0231c519ebdf4c7b95329d5f Mon Sep 17 00:00:00 2001
From: Kyle Larose <kyle@agilicus.com>
Date: Wed, 15 Feb 2023 15:49:24 -0500
Subject: [PATCH 1/2] webdav: path-escape lockroot

We ran in to an issue with various microsoft products accessing files
over WebDAV which contain `%` characters in the name. The requests are
all properly escaped -- e.g. the `%` is replaced with `%25`. Further
the basic list functionality of the propfind responses contain encoded
urls.

The lock response/discovery includes the URL (lockroot) to the locked
file. The existing implementation does not path escape that url. This
seems to cause problems with the microsoft applications. When faced with
the unencoded response, they try to lock again and fail. One explanation
for this behaviour is that they maintain a table of locks indexed by
encoded url. When the response from the server doesn't align with the
URL they locked, the application fails.

Update the lock discovery response code to escape the lockroot so that
it matches everything else.
---
 webdav/xml.go | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/webdav/xml.go b/webdav/xml.go
index fbd43cf22..8eda4b0a9 100644
--- a/webdav/xml.go
+++ b/webdav/xml.go
@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"time"
 
 	// As of https://go-review.googlesource.com/#/c/12772/ which was submitted
@@ -87,6 +88,10 @@ func writeLockInfo(w io.Writer, token string, ld LockDetails) (int, error) {
 		depth = "0"
 	}
 	timeout := ld.Duration / time.Second
+	// 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)
 	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"+
@@ -97,7 +102,7 @@ func writeLockInfo(w io.Writer, token string, ld LockDetails) (int, error) {
 		"	<D:locktoken><D:href>%s</D:href></D:locktoken>\n"+
 		"	<D:lockroot><D:href>%s</D:href></D:lockroot>\n"+
 		"</D:activelock></D:lockdiscovery></D:prop>",
-		depth, ld.OwnerXML, timeout, escape(token), escape(ld.Root),
+		depth, ld.OwnerXML, timeout, escape(token), escape(root),
 	)
 }
 

From 806b1f1d96253151ec23158a4ee28fa4c719c7da Mon Sep 17 00:00:00 2001
From: Kyle Larose <kyle@agilicus.com>
Date: Tue, 21 Feb 2023 08:55:57 -0500
Subject: [PATCH 2/2] 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.
---
 webdav/webdav_test.go | 111 +++++++++++++++++++++++++++++++++++++-----
 webdav/xml.go         |   3 +-
 2 files changed, 102 insertions(+), 12 deletions(-)

diff --git a/webdav/webdav_test.go b/webdav/webdav_test.go
index 2baebe3c9..5499b334f 100644
--- a/webdav/webdav_test.go
+++ b/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)
+		}
+	}
+}
diff --git a/webdav/xml.go b/webdav/xml.go
index 8eda4b0a9..2c1e00a5c 100644
--- a/webdav/xml.go
+++ b/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"+