ocdav: Properly escape oc:name in propfind response (#3255)

The oc:name property in the ocdav propfind response might contain XML special characters. We now apply the proper escaping on that property. owncloud/ocis#4474
cs3org · Sep 21, 2022 · a098879 · a098879
1 parent 282e405
commit a098879
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 2 deletions.
diff --git a/changelog/unreleased/fix-propfind-escape-name.md b/changelog/unreleased/fix-propfind-escape-name.md
@@ -0,0 +1,8 @@
+Bugfix: Properly escape oc:name in propfind response
+
+The oc:name property in the ocdav propfind response might contain
+XML special characters. We now apply the proper escaping on that
+property.
+
+https://github.com/cs3org/reva/pull/3255
+https://github.com/owncloud/ocis/issues/4474
diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind.go b/internal/http/services/owncloud/ocdav/propfind/propfind.go
@@ -1010,7 +1010,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p
 		}
 
 		if md.Name != "" {
-			appendToOK(prop.Raw("oc:name", md.Name))
+			appendToOK(prop.Escaped("oc:name", md.Name))
 		}
 
 		if md.Etag != "" {
@@ -1343,7 +1343,7 @@ func mdToPropResponse(ctx context.Context, pf *XML, md *provider.ResourceInfo, p
 						}
 					}
 				case "name":
-					appendToOK(prop.Raw("oc:name", md.Name))
+					appendToOK(prop.Escaped("oc:name", md.Name))
 				case "shareid":
 					if ref, err := storagespace.ParseReference(strings.TrimPrefix(md.Path, "/")); err == nil && ref.GetResourceId().GetSpaceId() == utils.ShareStorageSpaceID {
 						appendToOK(prop.Raw("oc:shareid", ref.GetResourceId().GetOpaqueId()))

diff --git a/internal/http/services/owncloud/ocdav/propfind/propfind_test.go b/internal/http/services/owncloud/ocdav/propfind/propfind_test.go
@@ -233,6 +233,14 @@ var _ = Describe("Propfind", func() {
 				Path: "./dir",
 				Size: uint64(30),
 			})
+		mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"}, Path: "./dir&dir"},
+			&sprovider.ResourceInfo{
+				Id:   &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "dir"},
+				Type: sprovider.ResourceType_RESOURCE_TYPE_CONTAINER,
+				Path: "./dir&dir",
+				Name: "dir&dir",
+				Size: uint64(30),
+			})
 		mockStat(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "dir"}, Path: "."},
 			&sprovider.ResourceInfo{
 				Id:   &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "dir"},
@@ -249,6 +257,15 @@ var _ = Describe("Propfind", func() {
 					Size: 30,
 				},
 			})
+		mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"}, Path: "./dir&dir"},
+			[]*sprovider.ResourceInfo{
+				{
+					Id:   &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "direntry"},
+					Type: sprovider.ResourceType_RESOURCE_TYPE_FILE,
+					Path: "entry",
+					Size: 30,
+				},
+			})
 		mockListContainer(&sprovider.Reference{ResourceId: &sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "dir"}, Path: "."},
 			[]*sprovider.ResourceInfo{
 				{
@@ -793,6 +810,23 @@ var _ = Describe("Propfind", func() {
 			Expect(string(res.Responses[1].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("<d:getcontentlength>30</d:getcontentlength>"))
 		})
 
+		It("stats a directory with xml special characters", func() {
+			rr := httptest.NewRecorder()
+			req, err := http.NewRequest("GET", "/dir&dir", strings.NewReader(""))
+			Expect(err).ToNot(HaveOccurred())
+			req = req.WithContext(ctx)
+
+			spaceID := storagespace.FormatResourceID(sprovider.ResourceId{StorageId: "provider-1", SpaceId: "foospace", OpaqueId: "root"})
+			handler.HandleSpacesPropfind(rr, req, spaceID)
+			Expect(rr.Code).To(Equal(http.StatusMultiStatus))
+
+			res, _, err := readResponse(rr.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(len(res.Responses)).To(Equal(2))
+			Expect(string(res.Responses[0].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("<oc:name>dir&amp;dir</oc:name>"))
+			Expect(string(res.Responses[1].Propstat[0].Prop[0].InnerXML)).To(ContainSubstring("<d:getcontentlength>30</d:getcontentlength>"))
+		})
+
 		It("includes all the things™ when depth is infinity", func() {
 			rr := httptest.NewRecorder()
 			req, err := http.NewRequest("GET", "/", strings.NewReader(""))