More dav unit tests (#3461)

* cover naming rules Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * some requests expect no body Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * update changelog Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * add mkcol to pending empty body tests Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * start reorganizing tests per endpoint, method, successful path and error cases Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * update unexpected passes Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> * incorporate feedback Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de> Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Nov 23, 2022 · 8c4890d · 8c4890d
1 parent cf8ce44
commit 8c4890d
Show file tree

Hide file tree

Showing 10 changed files with 635 additions and 84 deletions.
diff --git a/changelog/unreleased/dav-unit-tests.md b/changelog/unreleased/dav-unit-tests.md
@@ -3,9 +3,11 @@ Enhancement: Cover ocdav with more unit tests
 We added unit tests to cover more ocdav handlers:
 -   delete
 -   mkcol
+-   fixes https://github.com/owncloud/ocis/issues/4332
 
 https://github.com/cs3org/reva/pull/3441
 https://github.com/cs3org/reva/pull/3443
 https://github.com/cs3org/reva/pull/3445
 https://github.com/cs3org/reva/pull/3447
-https://github.com/cs3org/reva/pull/3454
+https://github.com/cs3org/reva/pull/3454
+https://github.com/cs3org/reva/pull/3461
diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go
@@ -54,6 +54,13 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
 	ctx, span := s.tracerProvider.Tracer(tracerName).Start(r.Context(), "copy")
 	defer span.End()
 
+	if r.Body != http.NoBody {
+		w.WriteHeader(http.StatusUnsupportedMediaType)
+		b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
+		return
+	}
+
 	if s.c.EnableHTTPTpc {
 		if r.Header.Get("Source") != "" {
 			// HTTP Third-Party Copy Pull mode
@@ -73,15 +80,23 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
 	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
 	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
-		appctx.GetLogger(ctx).Warn().Msg("HTTP COPY: failed to extract destination")
 		w.WriteHeader(http.StatusBadRequest)
+		b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 		return
 	}
 
 	for _, r := range nameRules {
+		if !r.Test(src) {
+			w.WriteHeader(http.StatusBadRequest)
+			b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "")
+			errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
+			return
+		}
 		if !r.Test(dst) {
-			appctx.GetLogger(ctx).Warn().Msgf("HTTP COPY: destination %s failed validation", dst)
 			w.WriteHeader(http.StatusBadRequest)
+			b, err := errors.Marshal(http.StatusBadRequest, "destination failed naming rules", "")
+			errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 			return
 		}
 	}
@@ -290,6 +305,13 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s
 	ctx, span := s.tracerProvider.Tracer(tracerName).Start(r.Context(), "spaces_copy")
 	defer span.End()
 
+	if r.Body != http.NoBody {
+		w.WriteHeader(http.StatusUnsupportedMediaType)
+		b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
+		return
+	}
+
 	dh := r.Header.Get(net.HeaderDestination)
 	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
 	dst, err := net.ParseDestination(baseURI, dh)

diff --git a/internal/http/services/owncloud/ocdav/delete.go b/internal/http/services/owncloud/ocdav/delete.go
@@ -38,6 +38,10 @@ func (s *svc) handlePathDelete(w http.ResponseWriter, r *http.Request, ns string
 	ctx, span := s.tracerProvider.Tracer(tracerName).Start(ctx, "path_delete")
 	defer span.End()
 
+	if r.Body != http.NoBody {
+		return http.StatusUnsupportedMediaType, errors.New("body must be empty")
+	}
+
 	fn := path.Join(ns, r.URL.Path)
 
 	space, rpcStatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, s.gwClient, fn)
@@ -113,6 +117,10 @@ func (s *svc) handleSpacesDelete(w http.ResponseWriter, r *http.Request, spaceID
 	ctx, span := s.tracerProvider.Tracer(tracerName).Start(ctx, "spaces_delete")
 	defer span.End()
 
+	if r.Body != http.NoBody {
+		return http.StatusUnsupportedMediaType, errors.New("body must be empty")
+	}
+
 	ref, err := spacelookup.MakeStorageSpaceReference(spaceID, r.URL.Path)
 	if err != nil {
 		return http.StatusBadRequest, err

diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go
@@ -44,6 +44,8 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)
 
 	if r.Body != http.NoBody {
 		w.WriteHeader(http.StatusUnsupportedMediaType)
+		b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 		return
 	}
 
@@ -53,12 +55,22 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)
 	dstPath, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
+		b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 		return
 	}
 
 	for _, r := range nameRules {
+		if !r.Test(srcPath) {
+			w.WriteHeader(http.StatusBadRequest)
+			b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "")
+			errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
+			return
+		}
 		if !r.Test(dstPath) {
 			w.WriteHeader(http.StatusBadRequest)
+			b, err := errors.Marshal(http.StatusBadRequest, "destination naming rules", "")
+			errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 			return
 		}
 	}
@@ -97,6 +109,8 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI
 
 	if r.Body != http.NoBody {
 		w.WriteHeader(http.StatusUnsupportedMediaType)
+		b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
+		errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
 		return
 	}