From 3832f23789f33cd8dbadf05bf82d44f360acedc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Fri, 31 Jul 2026 15:17:59 +0200 Subject: [PATCH] fix: data race won't happen when uploading files into a space --- internal/http/services/owncloud/ocdav/copy.go | 14 +++++++------- .../http/services/owncloud/ocdav/validation.go | 10 +++++++++- pkg/storage/utils/decomposedfs/decomposedfs.go | 4 +++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index f7e3c0d2de2..1800e0d5e49 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -44,7 +44,7 @@ import ( "github.com/rs/zerolog" ) -type copy struct { +type copyInfo struct { source *provider.Reference sourceInfo *provider.ResourceInfo destination *provider.Reference @@ -139,7 +139,7 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string) w.WriteHeader(cp.successCode) } -func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gateway.GatewayAPIClient], w http.ResponseWriter, r *http.Request, cp *copy) error { +func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gateway.GatewayAPIClient], w http.ResponseWriter, r *http.Request, cp *copyInfo) error { log := appctx.GetLogger(ctx) log.Debug().Str("src", cp.sourceInfo.Path).Str("dst", cp.destination.Path).Msg("descending") @@ -198,7 +198,7 @@ func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gate ResourceId: cp.destination.ResourceId, Path: utils.MakeRelativePath(filepath.Join(cp.destination.Path, child)), } - err := s.executePathCopy(ctx, selector, w, r, ©{source: src, sourceInfo: res.Infos[i], destination: childDst, depth: cp.depth, successCode: cp.successCode}) + err := s.executePathCopy(ctx, selector, w, r, ©Info{source: src, sourceInfo: res.Infos[i], destination: childDst, depth: cp.depth, successCode: cp.successCode}) if err != nil { return err } @@ -375,7 +375,7 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s w.WriteHeader(cp.successCode) } -func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, selector pool.Selectable[gateway.GatewayAPIClient], cp *copy) error { +func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, selector pool.Selectable[gateway.GatewayAPIClient], cp *copyInfo) error { log := appctx.GetLogger(ctx) log.Debug().Interface("src", cp.sourceInfo).Interface("dst", cp.destination).Msg("descending") @@ -428,7 +428,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele ResourceId: cp.destination.ResourceId, Path: utils.MakeRelativePath(path.Join(cp.destination.Path, res.Infos[i].Path)), } - err := s.executeSpacesCopy(ctx, w, selector, ©{sourceInfo: res.Infos[i], destination: childRef, depth: cp.depth, successCode: cp.successCode}) + err := s.executeSpacesCopy(ctx, w, selector, ©Info{sourceInfo: res.Infos[i], destination: childRef, depth: cp.depth, successCode: cp.successCode}) if err != nil { return err } @@ -552,7 +552,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele return nil } -func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger, destInShareJail bool) *copy { +func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Request, srcRef, dstRef *provider.Reference, log *zerolog.Logger, destInShareJail bool) *copyInfo { // restrict copy from the vault to outside of the vault. if destinationIsNotAllowed(srcRef, dstRef) { w.WriteHeader(http.StatusConflict) @@ -759,5 +759,5 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re // TODO what if intermediate is a file? } - return ©{source: srcRef, sourceInfo: srcStatRes.Info, depth: depth, successCode: successCode, destination: dstRef} + return ©Info{source: srcRef, sourceInfo: srcStatRes.Info, depth: depth, successCode: successCode, destination: dstRef} } diff --git a/internal/http/services/owncloud/ocdav/validation.go b/internal/http/services/owncloud/ocdav/validation.go index 5d257b87cef..6c66ecc280f 100644 --- a/internal/http/services/owncloud/ocdav/validation.go +++ b/internal/http/services/owncloud/ocdav/validation.go @@ -27,7 +27,15 @@ func ValidatorsFromConfig(c *config.Config) []Validator { // ValidateName will validate a file or folder name, returning an error when it is not accepted func ValidateName(name string, validators []Validator) error { - return ValidateDestination(name, append(validators, notReserved())) + // This function might be used in multiple requests at the same time, + // and the validator list usually comes from the ocdav's svc.nameValidators, + // which is shared among the requests. + // In order to prevent possible data races, instead of append, we'll + // copy the validator list into a local var and then add the extra validator. + validatorsCopy := make([]Validator, len(validators)+1) + copy(validatorsCopy, validators) + validatorsCopy[len(validators)] = notReserved() + return ValidateDestination(name, validatorsCopy) } // ValidateDestination will validate a file or folder destination name (which can be . or ..), returning an error when it is not accepted diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 8117e57dcd6..50597085e8a 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -1057,7 +1057,9 @@ func (fs *Decomposedfs) ListFolder(ctx context.Context, ref *provider.Reference, for i := 0; i < numWorkers; i++ { g.Go(func() error { for child := range work { - np := rp + // make a copy of the parent's permissions; shallow copy is good enough + np := &provider.ResourcePermissions{} + *np = *rp // add this childs permissions pset, _ := child.PermissionSet(ctx) node.AddPermissions(np, pset)