[tests-only] Can't create folder names which are subset of "Shares" (#…

…2484) * test fix with easy implementation Signed-off-by: jkoberg <[email protected]> * use same logic for other above path also Signed-off-by: jkoberg <[email protected]> * add special case handling to isSubPath Signed-off-by: jkoberg <[email protected]> * refine isSubpath logic Signed-off-by: jkoberg <[email protected]> * Stat on MKCOL before creating Signed-off-by: jkoberg <[email protected]> * update comment to not include typos Signed-off-by: jkoberg <[email protected]> * handle error correctly for MKCOL Signed-off-by: jkoberg <[email protected]>
cs3org · Feb 14, 2022 · affffea · affffea
1 parent a230234
commit affffea
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 22 deletions.
diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go
@@ -52,6 +52,28 @@ func (s *svc) handlePathMkcol(w http.ResponseWriter, r *http.Request, ns string)
 		return
 	}
 
+	// stat requested path to make sure it isn't existing yet
+	// NOTE: It could be on another storage provider than the 'parent' of it
+	sr, err := client.Stat(ctx, &provider.StatRequest{
+		Ref: &provider.Reference{
+			Path: fn,
+		},
+	})
+	if err != nil {
+		sublog.Error().Err(err).Str("path", fn).Msg("failed to look up storage space")
+		w.WriteHeader(http.StatusInternalServerError)
+		return
+	}
+
+	if sr.Status.Code != rpc.Code_CODE_NOT_FOUND {
+		sublog.Info().Err(err).Str("path", fn).Interface("code", sr.Status.Code).Msg("response code for stat was unexpected")
+		// tests want this errorcode. StatusConflict would be more logical
+		w.WriteHeader(http.StatusMethodNotAllowed)
+		b, err := errors.Marshal(errors.SabredavMethodNotAllowed, "The resource you tried to create already exists", "")
+		errors.HandleWebdavError(&sublog, w, b, err)
+		return
+	}
+
 	parentPath := path.Dir(fn)
 
 	space, status, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, parentPath)

diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go
@@ -510,7 +510,7 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa
 				deepestMountSpace = space
 				deepestMountPathProvider = p
 
-			case strings.HasPrefix(spacePath, path) && !unique:
+			case !unique && isSubpath(spacePath, path):
 				// and add all providers below and exactly matching the path
 				// requested /foo, mountPath /foo/sub
 				validSpaces = append(validSpaces, space)
@@ -520,7 +520,7 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa
 					deepestMountPathProvider = p
 				}
 
-			case strings.HasPrefix(path, spacePath) && len(spacePath) > len(deepestMountPath):
+			case isSubpath(path, spacePath) && len(spacePath) > len(deepestMountPath):
 				// eg. three providers: /foo, /foo/sub, /foo/sub/bar
 				// requested /foo/sub/mob
 				deepestMountPath = spacePath
@@ -613,3 +613,17 @@ func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string,
 	}
 	return res.StorageSpaces, nil
 }
+
+// isSubpath determines if `p` is a subpath of `path`
+func isSubpath(p string, path string) bool {
+	if p == path {
+		return true
+	}
+
+	r, err := filepath.Rel(path, p)
+	if err != nil {
+		return false
+	}
+
+	return r != ".." && !strings.HasPrefix(r, "../")
+}
diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.md b/tests/acceptance/expected-failures-on-OCIS-storage.md
@@ -1302,20 +1302,11 @@ _ocs: api compatibility, return correct status code_
 ### [send PUT requests to another user's webDav endpoints as normal user](https://github.com/owncloud/ocis/issues/2893)
 - [apiAuthWebDav/webDavPUTAuth.feature:58](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavPUTAuth.feature#L58)
 
-#### [Creating a new folder which is a substring of Shares leads to Unknown Error](https://github.com/owncloud/ocis/issues/3033)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:27](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L27)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:29](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L29)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:30](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L30)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:32](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L32)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:45](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L45)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L48)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:59](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L59)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:60](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L60)
-
 #### [moveShareInsideAnotherShare behaves differently on oCIS than oC10](https://github.com/owncloud/ocis/issues/3047)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:25](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L25)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:86](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L86)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:100](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L100)
 
 Note: always have an empty line at the end of this file.
 The bash script that processes this file may not process a scenario reference on the last line.
+
diff --git a/tests/acceptance/expected-failures-on-S3NG-storage.md b/tests/acceptance/expected-failures-on-S3NG-storage.md
@@ -1304,20 +1304,11 @@ _ocs: api compatibility, return correct status code_
 ### [send PUT requests to another user's webDav endpoints as normal user](https://github.com/owncloud/ocis/issues/2893)
 -   [apiAuthWebDav/webDavPUTAuth.feature:58](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiAuthWebDav/webDavPUTAuth.feature#L58)
 
-#### [Creating a new folder which is a substring of Shares leads to Unknown Error](https://github.com/owncloud/ocis/issues/3033)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:27](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L27)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:29](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L29)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:30](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L30)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:32](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L32)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:45](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L45)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:48](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L48)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:59](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L59)
--   [apiWebdavProperties1/createFileFolderWhenSharesExist.feature:60](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiWebdavProperties1/createFileFolderWhenSharesExist.feature#L60)
-
 #### [moveShareInsideAnotherShare behaves differently on oCIS than oC10](https://github.com/owncloud/ocis/issues/3047)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:25](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L25)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:86](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L86)
 - [apiShareManagementToShares/moveShareInsideAnotherShare.feature:100](https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiShareManagementToShares/moveShareInsideAnotherShare.feature#L100)
 
 Note: always have an empty line at the end of this file.
 The bash script that processes this file may not process a scenario reference on the last line.
+