From affffea4343f04c11c69a903fbb0b96a8b959dff Mon Sep 17 00:00:00 2001 From: kobergj Date: Tue, 1 Feb 2022 11:04:33 +0100 Subject: [PATCH] [tests-only] Can't create folder names which are subset of "Shares" (#2484) * test fix with easy implementation Signed-off-by: jkoberg * use same logic for other above path also Signed-off-by: jkoberg * add special case handling to isSubPath Signed-off-by: jkoberg * refine isSubpath logic Signed-off-by: jkoberg * Stat on MKCOL before creating Signed-off-by: jkoberg * update comment to not include typos Signed-off-by: jkoberg * handle error correctly for MKCOL Signed-off-by: jkoberg --- .../http/services/owncloud/ocdav/mkcol.go | 22 +++++++++++++++++++ pkg/storage/registry/spaces/spaces.go | 18 +++++++++++++-- .../expected-failures-on-OCIS-storage.md | 11 +--------- .../expected-failures-on-S3NG-storage.md | 11 +--------- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index 72d57acb5f0..23d1b3fd59b 100644 --- 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 index c909abd2567..a1fa633597c 100644 --- 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 index 00a8051aa47..c53c1fe6080 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.md +++ b/tests/acceptance/expected-failures-on-OCIS-storage.md @@ -1302,16 +1302,6 @@ _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) @@ -1319,3 +1309,4 @@ _ocs: api compatibility, return correct status code_ 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 index 70fdb231021..e117a717577 100644 --- a/tests/acceptance/expected-failures-on-S3NG-storage.md +++ b/tests/acceptance/expected-failures-on-S3NG-storage.md @@ -1304,16 +1304,6 @@ _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) @@ -1321,3 +1311,4 @@ _ocs: api compatibility, return correct status code_ 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. +