From 55768471cd6b15d95f85e5983af856ba84b3cc9c Mon Sep 17 00:00:00 2001 From: Willy Kloucek Date: Fri, 19 Nov 2021 08:29:39 +0100 Subject: [PATCH] fix status code for WebDAV mkcol requests where an ancestor is missing --- .../fix-webdav-mkcol-deny-recursive.md | 9 +++ .../http/services/owncloud/ocdav/mkcol.go | 60 +++++++++++++++---- 2 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 changelog/unreleased/fix-webdav-mkcol-deny-recursive.md diff --git a/changelog/unreleased/fix-webdav-mkcol-deny-recursive.md b/changelog/unreleased/fix-webdav-mkcol-deny-recursive.md new file mode 100644 index 0000000000..aa06a87cde --- /dev/null +++ b/changelog/unreleased/fix-webdav-mkcol-deny-recursive.md @@ -0,0 +1,9 @@ +Bugfix: Fix status code for WebDAV mkcol requests where an ancestor is missing + +We've fixed the status code to 409 according to the WebDAV standard for MKCOL +requests where an ancestor is missing. Previously these requests would fail +with an different error code (eg. 500) because of storage driver limitations +(eg. oCIS FS cannot handle recursive creation of directories). + +https://github.com/owncloud/ocis/issues/2767 +https://github.com/cs3org/reva/pull/2293 diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index ddee72b86b..17cd6543fa 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -44,9 +44,10 @@ func (s *svc) handlePathMkcol(w http.ResponseWriter, r *http.Request, ns string) } sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() - ref := &provider.Reference{Path: fn} + parentRef := &provider.Reference{Path: path.Dir(fn)} + childRef := &provider.Reference{Path: fn} - s.handleMkcol(ctx, w, r, ref, sublog) + s.handleMkcol(ctx, w, r, parentRef, childRef, sublog) } func (s *svc) handleSpacesMkCol(w http.ResponseWriter, r *http.Request, spaceID string) { @@ -55,7 +56,7 @@ func (s *svc) handleSpacesMkCol(w http.ResponseWriter, r *http.Request, spaceID sublog := appctx.GetLogger(ctx).With().Str("path", r.URL.Path).Str("spaceid", spaceID).Str("handler", "mkcol").Logger() - ref, rpcStatus, err := s.lookUpStorageSpaceReference(ctx, spaceID, r.URL.Path) + parentRef, rpcStatus, err := s.lookUpStorageSpaceReference(ctx, spaceID, path.Dir(r.URL.Path)) if err != nil { sublog.Error().Err(err).Msg("error sending a grpc request") w.WriteHeader(http.StatusInternalServerError) @@ -67,11 +68,23 @@ func (s *svc) handleSpacesMkCol(w http.ResponseWriter, r *http.Request, spaceID return } - s.handleMkcol(ctx, w, r, ref, sublog) + childRef, rpcStatus, err := s.lookUpStorageSpaceReference(ctx, spaceID, r.URL.Path) + if err != nil { + sublog.Error().Err(err).Msg("error sending a grpc request") + w.WriteHeader(http.StatusInternalServerError) + return + } + + if rpcStatus.Code != rpc.Code_CODE_OK { + HandleErrorStatus(&sublog, w, rpcStatus) + return + } + + s.handleMkcol(ctx, w, r, parentRef, childRef, sublog) } -func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Request, ref *provider.Reference, log zerolog.Logger) { +func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Request, parentRef, childRef *provider.Reference, log zerolog.Logger) { if r.Body != http.NoBody { w.WriteHeader(http.StatusUnsupportedMediaType) return @@ -84,8 +97,35 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re return } - // check if ref exists - statReq := &provider.StatRequest{Ref: ref} + // check if parent exists + parentStatReq := &provider.StatRequest{Ref: parentRef} + parentStatRes, err := client.Stat(ctx, parentStatReq) + if err != nil { + log.Error().Err(err).Msg("error sending a grpc stat request") + w.WriteHeader(http.StatusInternalServerError) + return + } + + if parentStatRes.Status.Code != rpc.Code_CODE_OK { + if parentStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + // http://www.webdav.org/specs/rfc4918.html#METHOD_MKCOL + // When the MKCOL operation creates a new collection resource, + // all ancestors must already exist, or the method must fail + // with a 409 (Conflict) status code. + w.WriteHeader(http.StatusConflict) + b, err := Marshal(exception{ + code: SabredavNotFound, + message: "Parent node does not exist", + }) + HandleWebdavError(&log, w, b, err) + } else { + HandleErrorStatus(&log, w, parentStatRes.Status) + } + return + } + + // check if child exists + statReq := &provider.StatRequest{Ref: childRef} statRes, err := client.Stat(ctx, statReq) if err != nil { log.Error().Err(err).Msg("error sending a grpc stat request") @@ -107,7 +147,7 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re return } - req := &provider.CreateContainerRequest{Ref: ref} + req := &provider.CreateContainerRequest{Ref: childRef} res, err := client.CreateContainer(ctx, req) if err != nil { log.Error().Err(err).Msg("error sending create container grpc request") @@ -118,12 +158,12 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re case rpc.Code_CODE_OK: w.WriteHeader(http.StatusCreated) case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", ref.Path).Interface("status", statRes.Status).Msg("conflict") + log.Debug().Str("path", childRef.Path).Interface("status", statRes.Status).Msg("conflict") w.WriteHeader(http.StatusConflict) case rpc.Code_CODE_PERMISSION_DENIED: w.WriteHeader(http.StatusForbidden) // TODO path could be empty or relative... - m := fmt.Sprintf("Permission denied to create %v", ref.Path) + m := fmt.Sprintf("Permission denied to create %v", childRef.Path) b, err := Marshal(exception{ code: SabredavPermissionDenied, message: m,