From 4585b765efa40babff6b518a14a8192a19965b44 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Fri, 17 Nov 2023 11:35:28 +0100 Subject: [PATCH 1/2] [full-ci] bump reva fix disable depth infinity --- changelog/fix-disable-depth-infinity.md | 10 +++++ go.mod | 2 +- go.sum | 4 +- .../owncloud/ocdav/propfind/propfind.go | 43 +++++++++++++------ .../services/owncloud/ocdav/publicfile.go | 24 ++++++++++- .../http/services/owncloud/ocdav/trashbin.go | 26 ++++++++++- vendor/modules.txt | 2 +- 7 files changed, 91 insertions(+), 20 deletions(-) create mode 100644 changelog/fix-disable-depth-infinity.md diff --git a/changelog/fix-disable-depth-infinity.md b/changelog/fix-disable-depth-infinity.md new file mode 100644 index 00000000000..2bd508fb0ea --- /dev/null +++ b/changelog/fix-disable-depth-infinity.md @@ -0,0 +1,10 @@ +Bugfix: Disable DEPTH infinity in PROPFIND + +We fixed the Disabled DEPTH infinity in PROPFIND for: +Personal /remote.php/dav/files/admin +Public link share /remote.php/dav/public-files/ +Trashbin /remote.php/dav/spaces/trash-bin/ + +https://github.com/owncloud/ocis/pull/7746 +https://github.com/cs3org/reva/pull/4278 +https://github.com/owncloud/ocis/issues/7359 diff --git a/go.mod b/go.mod index a2b7a7eb7ab..fe6e1dd2a1a 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.7.0 github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 - github.com/cs3org/reva/v2 v2.16.1-0.20231115174649-4c665a7f03ac + github.com/cs3org/reva/v2 v2.16.1-0.20231116135502-a5f1195341b9 github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25 github.com/disintegration/imaging v1.6.2 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e diff --git a/go.sum b/go.sum index 1a92f50b2bc..25b22b65978 100644 --- a/go.sum +++ b/go.sum @@ -1013,8 +1013,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c= github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY= github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY= -github.com/cs3org/reva/v2 v2.16.1-0.20231115174649-4c665a7f03ac h1:0JZeSa52mBW4aC5wFO7+MPTka4ccH3H1HBD9HL5z/+4= -github.com/cs3org/reva/v2 v2.16.1-0.20231115174649-4c665a7f03ac/go.mod h1:utPCNSrWDdAwz2biLrKvzO6nDH9L7vRVGNzof13r8Kw= +github.com/cs3org/reva/v2 v2.16.1-0.20231116135502-a5f1195341b9 h1:K8PZ8z4EHBG5IUY5xDVzmYqgnPaZqWbHo+PjqDANwPs= +github.com/cs3org/reva/v2 v2.16.1-0.20231116135502-a5f1195341b9/go.mod h1:utPCNSrWDdAwz2biLrKvzO6nDH9L7vRVGNzof13r8Kw= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind/propfind.go b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind/propfind.go index 0aa094f3246..648ded62100 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind/propfind.go +++ b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind/propfind.go @@ -190,6 +190,33 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns fn := path.Join(ns, r.URL.Path) // TODO do we still need to jail if we query the registry about the spaces? sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + dh := r.Header.Get(net.HeaderDepth) + + depth, err := net.ParseDepth(dh) + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "Invalid Depth header value") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) + sublog.Debug().Str("depth", dh).Msg(err.Error()) + w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) + return + } + + if depth == net.DepthInfinity && !p.c.AllowPropfindDepthInfinitiy { + span.RecordError(errors.ErrInvalidDepth) + span.SetStatus(codes.Error, "DEPTH: infinity is not supported") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) + sublog.Debug().Str("depth", dh).Msg(errors.ErrInvalidDepth.Error()) + w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) + return + } + pf, status, err := ReadPropfind(r.Body) if err != nil { sublog.Debug().Err(err).Msg("error reading propfind request") @@ -218,7 +245,7 @@ func (p *Handler) HandlePathPropfind(w http.ResponseWriter, r *http.Request, ns return } - resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, sublog) + resourceInfos, sendTusHeaders, ok := p.getResourceInfos(ctx, w, r, pf, spaces, fn, depth, sublog) if !ok { // getResourceInfos handles responses in case of an error so we can just return here. return @@ -462,21 +489,11 @@ func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient return res.GetInfo(), res.GetStatus(), nil } -func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, spaces []*provider.StorageSpace, requestPath string, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) { +func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, spaces []*provider.StorageSpace, requestPath string, depth net.Depth, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) { ctx, span := appctx.GetTracerProvider(ctx).Tracer(tracerName).Start(ctx, "get_resource_infos") span.SetAttributes(attribute.KeyValue{Key: "requestPath", Value: attribute.StringValue(requestPath)}) - defer span.End() - - dh := r.Header.Get(net.HeaderDepth) - depth, err := net.ParseDepth(dh) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - m := fmt.Sprintf("Invalid Depth header value: %v", dh) - b, err := errors.Marshal(http.StatusBadRequest, m, "") - errors.HandleWebdavError(&log, w, b, err) - return nil, false, false - } span.SetAttributes(attribute.KeyValue{Key: "depth", Value: attribute.StringValue(depth.String())}) + defer span.End() client, err := p.selector.Next() if err != nil { diff --git a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/publicfile.go b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/publicfile.go index 7b918268cdf..31c92616b2b 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/publicfile.go +++ b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/publicfile.go @@ -20,15 +20,19 @@ package ocdav import ( "context" + "fmt" "net/http" "path" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typesv1beta1 "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind" "github.com/cs3org/reva/v2/pkg/appctx" "github.com/cs3org/reva/v2/pkg/rhttp/router" + "go.opentelemetry.io/otel/codes" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" ) // PublicFileHandler handles requests on a shared file. it needs to be wrapped in a collection @@ -99,8 +103,26 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s dh := r.Header.Get(net.HeaderDepth) depth, err := net.ParseDepth(dh) if err != nil { - sublog.Debug().Msg(err.Error()) + span.RecordError(err) + span.SetStatus(codes.Error, "Invalid Depth header value") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) + sublog.Debug().Str("depth", dh).Msg(err.Error()) w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) + return + } + + if depth == net.DepthInfinity && !s.c.AllowPropfindDepthInfinitiy { + span.RecordError(errors.ErrInvalidDepth) + span.SetStatus(codes.Error, "DEPTH: infinity is not supported") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) + sublog.Debug().Str("depth", dh).Msg(errors.ErrInvalidDepth.Error()) + w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) return } diff --git a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/trashbin.go b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/trashbin.go index 6e7f19aef15..51205a3a051 100644 --- a/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/trashbin.go +++ b/vendor/github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/trashbin.go @@ -36,6 +36,7 @@ import ( "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind" "github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup" "github.com/cs3org/reva/v2/pkg/storagespace" + "go.opentelemetry.io/otel/codes" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -44,17 +45,20 @@ import ( rstatus "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/rhttp/router" "github.com/cs3org/reva/v2/pkg/utils" + semconv "go.opentelemetry.io/otel/semconv/v1.20.0" ) // TrashbinHandler handles trashbin requests type TrashbinHandler struct { - gatewaySvc string - namespace string + gatewaySvc string + namespace string + allowPropfindDepthInfinitiy bool } func (h *TrashbinHandler) init(c *config.Config) error { h.gatewaySvc = c.GatewaySvc h.namespace = path.Join("/", c.FilesNamespace) + h.allowPropfindDepthInfinitiy = c.AllowPropfindDepthInfinitiy return nil } @@ -186,8 +190,26 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s dh := r.Header.Get(net.HeaderDepth) depth, err := net.ParseDepth(dh) if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, "Invalid Depth header value") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) sublog.Debug().Str("depth", dh).Msg(err.Error()) w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) + return + } + + if depth == net.DepthInfinity && !h.allowPropfindDepthInfinitiy { + span.RecordError(errors.ErrInvalidDepth) + span.SetStatus(codes.Error, "DEPTH: infinity is not supported") + span.SetAttributes(semconv.HTTPStatusCodeKey.Int(http.StatusBadRequest)) + sublog.Debug().Str("depth", dh).Msg(errors.ErrInvalidDepth.Error()) + w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", dh) + b, err := errors.Marshal(http.StatusBadRequest, m, "") + errors.HandleWebdavError(&sublog, w, b, err) return } diff --git a/vendor/modules.txt b/vendor/modules.txt index 139e221f408..360768268ba 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -357,7 +357,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.16.1-0.20231115174649-4c665a7f03ac +# github.com/cs3org/reva/v2 v2.16.1-0.20231116135502-a5f1195341b9 ## explicit; go 1.20 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime From 14f7790574a7bb1f8eda775b823afc208e5db417 Mon Sep 17 00:00:00 2001 From: Roman Perekhod Date: Fri, 17 Nov 2023 13:24:45 +0100 Subject: [PATCH 2/2] update the tests --- tests/acceptance/features/apiDepthInfinity/propfind.feature | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/acceptance/features/apiDepthInfinity/propfind.feature b/tests/acceptance/features/apiDepthInfinity/propfind.feature index 3f127977098..6e7be9e9f31 100644 --- a/tests/acceptance/features/apiDepthInfinity/propfind.feature +++ b/tests/acceptance/features/apiDepthInfinity/propfind.feature @@ -73,7 +73,10 @@ Feature: PROPFIND with depth:infinity Scenario: get the list of resources in a folder shared through public link with depth infinity Given using new DAV path - And the config "OCIS_SHARING_PUBLIC_SHARE_MUST_HAVE_PASSWORD" has been set to "false" + And the following configs have been set: + | config | value | + | OCDAV_ALLOW_PROPFIND_DEPTH_INFINITY | true | + | OCIS_SHARING_PUBLIC_SHARE_MUST_HAVE_PASSWORD | false | And user "Alice" has created a public link share of folder "simple-folder" When the public lists the resources in the last created public link with depth "infinity" using the WebDAV API Then the HTTP status code should be "207"