From dab86e0baa583351221ba4da02d63f6b950529e7 Mon Sep 17 00:00:00 2001 From: Intro Date: Tue, 4 Aug 2026 20:10:15 +0800 Subject: [PATCH 01/12] fix(webdav): correct COPY/MOVE semantics with named copy support Signed-off-by: Lythen --- internal/fs/copy_move.go | 42 ++++++++++++++++++++---------- internal/fs/fs.go | 15 ++++++++--- internal/fs/other.go | 1 + server/webdav/file.go | 56 +++++++++++++++++++++++++++++++++++----- 4 files changed, 90 insertions(+), 24 deletions(-) diff --git a/internal/fs/copy_move.go b/internal/fs/copy_move.go index e78fc9be83..92ad24fe32 100644 --- a/internal/fs/copy_move.go +++ b/internal/fs/copy_move.go @@ -100,7 +100,7 @@ func (t *FileTransferTask) SetRetry(retry int, maxRetry int) { } } -func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath string, skipHook ...bool) (task.TaskExtensionInfo, error) { +func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, dstName string, skipHook ...bool) (task.TaskExtensionInfo, error) { srcStorage, srcObjActualPath, err := op.GetStorageAndActualPath(srcObjPath) if err != nil { return nil, errors.WithMessage(err, "failed get src storage") @@ -114,15 +114,19 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath str if utils.IsBool(skipHook...) { ctx = context.WithValue(ctx, conf.SkipHookKey, struct{}{}) } - if taskType == copy || taskType == merge { - err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) - if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - return nil, err - } - } else { - err = op.Move(ctx, srcStorage, srcObjActualPath, dstDirActualPath) - if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - return nil, err + // A named copy cannot use the driver's destination-name-independent Copy + // operation. Fall back to the transfer task so the target name is kept. + if dstName == "" { + if taskType == copy || taskType == merge { + err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + return nil, err + } + } else { + err = op.Move(ctx, srcStorage, srcObjActualPath, dstDirActualPath) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + return nil, err + } } } } @@ -134,6 +138,7 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath str DstStorage: dstStorage, SrcActualPath: srcObjActualPath, DstActualPath: dstDirActualPath, + DstName: dstName, SrcStorageMp: srcStorage.GetStorage().MountPath, DstStorageMp: dstStorage.GetStorage().MountPath, }, @@ -189,9 +194,14 @@ func (t *FileTransferTask) RunWithNextTaskCallback(f func(nextTask *FileTransfer if err != nil { return errors.WithMessagef(err, "failed list src [%s] objs", t.SrcActualPath) } - dstActualPath := stdpath.Join(t.DstActualPath, srcObj.GetName()) - task_group.TransferCoordinator.AppendPayload(t.groupID, task_group.DstPathToHook(dstActualPath)) - + dstName := srcObj.GetName() + if t.DstName != "" { + dstName = t.DstName + } + dstActualPath := stdpath.Join(t.DstActualPath, dstName) + if err := op.MakeDir(t.Ctx(), t.DstStorage, dstActualPath); err != nil { + return errors.WithMessagef(err, "failed create dst dir [%s]", dstActualPath) + } existedObjs := make(map[string]bool) if t.TaskType == merge { dstObjs, err := op.List(t.Ctx(), t.DstStorage, dstActualPath, model.ListArgs{}) @@ -250,8 +260,12 @@ func (t *FileTransferTask) RunWithNextTaskCallback(f func(nextTask *FileTransfer return errors.WithMessagef(err, "failed get [%s] link", t.SrcActualPath) } // any link provided is seekable + streamObj := srcObj + if t.DstName != "" { + streamObj = &model.ObjWrapName{Name: t.DstName, Obj: srcObj} + } ss, err := stream.NewSeekableStream(&stream.FileStream{ - Obj: srcObj, + Obj: streamObj, Ctx: t.Ctx(), }, link) if err != nil { diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 67a1ac065e..b7f064ac74 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -69,7 +69,7 @@ func MakeDir(ctx context.Context, path string) error { } func Move(ctx context.Context, srcPath, dstDirPath string, skipHook ...bool) (task.TaskExtensionInfo, error) { - req, err := transfer(ctx, move, srcPath, dstDirPath, skipHook...) + req, err := transfer(ctx, move, srcPath, dstDirPath, "", skipHook...) if err != nil { log.Errorf("failed move %s to %s: %+v", srcPath, dstDirPath, err) } @@ -77,15 +77,24 @@ func Move(ctx context.Context, srcPath, dstDirPath string, skipHook ...bool) (ta } func Copy(ctx context.Context, srcObjPath, dstDirPath string, skipHook ...bool) (task.TaskExtensionInfo, error) { - res, err := transfer(ctx, copy, srcObjPath, dstDirPath, skipHook...) + res, err := transfer(ctx, copy, srcObjPath, dstDirPath, "", skipHook...) if err != nil { log.Errorf("failed copy %s to %s: %+v", srcObjPath, dstDirPath, err) } return res, err } +// CopyTo copies a file or directory to dstDirPath using dstName as its name. +func CopyTo(ctx context.Context, srcObjPath, dstDirPath, dstName string, skipHook ...bool) (task.TaskExtensionInfo, error) { + res, err := transfer(ctx, copy, srcObjPath, dstDirPath, dstName, skipHook...) + if err != nil { + log.Errorf("failed copy %s to %s as %s: %+v", srcObjPath, dstDirPath, dstName, err) + } + return res, err +} + func Merge(ctx context.Context, srcObjPath, dstDirPath string, skipHook ...bool) (task.TaskExtensionInfo, error) { - res, err := transfer(ctx, merge, srcObjPath, dstDirPath, skipHook...) + res, err := transfer(ctx, merge, srcObjPath, dstDirPath, "", skipHook...) if err != nil { log.Errorf("failed merge %s to %s: %+v", srcObjPath, dstDirPath, err) } diff --git a/internal/fs/other.go b/internal/fs/other.go index a23beb73bc..a74eff412a 100644 --- a/internal/fs/other.go +++ b/internal/fs/other.go @@ -53,6 +53,7 @@ type TaskData struct { Status string `json:"-"` //don't save status to save space SrcActualPath string `json:"src_path"` DstActualPath string `json:"dst_path"` + DstName string `json:"dst_name,omitempty"` SrcStorage driver.Driver `json:"-"` DstStorage driver.Driver `json:"-"` SrcStorageMp string `json:"src_storage_mp"` diff --git a/server/webdav/file.go b/server/webdav/file.go index ea60997359..e647be8179 100644 --- a/server/webdav/file.go +++ b/server/webdav/file.go @@ -55,21 +55,41 @@ func moveFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, srcMeta, srcDir) || !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } + if _, err = fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { + if errs.IsObjectNotFound(err) { + return http.StatusConflict, err + } + return http.StatusMethodNotAllowed, err + } + dstExisted := false + if _, err = fs.Get(ctx, dst, &fs.GetArgs{}); err == nil { + dstExisted = true + if !overwrite { + return http.StatusPreconditionFailed, nil + } + if err = fs.Remove(ctx, dst); err != nil { + return http.StatusInternalServerError, err + } + } else if !errs.IsObjectNotFound(err) { + return http.StatusInternalServerError, err + } if srcDir == dstDir { err = fs.Rename(ctx, src, dstName) } else { _, err = fs.Move(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir) - if err != nil { - return http.StatusInternalServerError, err - } - if srcName != dstName { + if err == nil && srcName != dstName { err = fs.Rename(ctx, path.Join(dstDir, srcName), dstName) } } if err != nil { return http.StatusInternalServerError, err } - // TODO if there are no files copy, should return 204 + if err = moveDeadProps(src, dst); err != nil { + return http.StatusInternalServerError, err + } + if dstExisted { + return http.StatusNoContent, nil + } return http.StatusCreated, nil } @@ -80,6 +100,7 @@ func moveFiles(ctx context.Context, src, dst string, overwrite bool) (status int func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int, err error) { srcDir := path.Dir(src) dstDir := path.Dir(dst) + dstName := path.Base(dst) user := ctx.Value(conf.UserKey).(*model.User) if !user.CanCopy() { return http.StatusForbidden, nil @@ -98,11 +119,32 @@ func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } - _, err = fs.Copy(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir) + if _, err = fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { + if errs.IsObjectNotFound(err) { + return http.StatusConflict, err + } + return http.StatusMethodNotAllowed, err + } + dstExisted := false + if _, err = fs.Get(ctx, dst, &fs.GetArgs{}); err == nil { + dstExisted = true + if !overwrite { + return http.StatusPreconditionFailed, nil + } + if err = fs.Remove(ctx, dst); err != nil { + return http.StatusInternalServerError, err + } + } else if !errs.IsObjectNotFound(err) { + return http.StatusInternalServerError, err + } + + _, err = fs.CopyTo(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir, dstName) if err != nil { return http.StatusInternalServerError, err } - // TODO if there are no files copy, should return 204 + if dstExisted { + return http.StatusNoContent, nil + } return http.StatusCreated, nil } From 0bb43ed8bdddf958a9a0bf41954ccc0119cf585f Mon Sep 17 00:00:00 2001 From: Intro Date: Tue, 4 Aug 2026 20:10:33 +0800 Subject: [PATCH 02/12] feat(webdav): persist dead properties with MOVE migration Signed-off-by: Lythen --- internal/db/db.go | 2 +- internal/model/webdav_property.go | 11 ++ server/webdav/prop.go | 207 +++++++++++++++--------------- 3 files changed, 119 insertions(+), 101 deletions(-) create mode 100644 internal/model/webdav_property.go diff --git a/internal/db/db.go b/internal/db/db.go index 96529c15d3..59e99f3975 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -12,7 +12,7 @@ var db *gorm.DB func Init(d *gorm.DB) { db = d - err := AutoMigrate(new(model.Storage), new(model.User), new(model.Meta), new(model.SettingItem), new(model.SearchNode), new(model.TaskItem), new(model.SSHPublicKey), new(model.SharingDB)) + err := AutoMigrate(new(model.Storage), new(model.User), new(model.Meta), new(model.SettingItem), new(model.SearchNode), new(model.TaskItem), new(model.SSHPublicKey), new(model.SharingDB), new(model.WebDAVProperty)) if err != nil { log.Fatalf("failed migrate database: %s", err.Error()) } diff --git a/internal/model/webdav_property.go b/internal/model/webdav_property.go new file mode 100644 index 0000000000..cdfedb9e72 --- /dev/null +++ b/internal/model/webdav_property.go @@ -0,0 +1,11 @@ +package model + +// WebDAVProperty stores a dead WebDAV property for a resource. +type WebDAVProperty struct { + ID uint `json:"id" gorm:"primaryKey"` + Path string `json:"path" gorm:"uniqueIndex:idx_webdav_property"` + Namespace string `json:"namespace" gorm:"uniqueIndex:idx_webdav_property"` + Name string `json:"name" gorm:"uniqueIndex:idx_webdav_property"` + Lang string `json:"lang"` + InnerXML []byte `json:"inner_xml"` +} diff --git a/server/webdav/prop.go b/server/webdav/prop.go index 5c88893414..66e09b88d5 100644 --- a/server/webdav/prop.go +++ b/server/webdav/prop.go @@ -16,9 +16,11 @@ import ( "time" "github.com/OpenListTeam/OpenList/v4/internal/conf" + "github.com/OpenListTeam/OpenList/v4/internal/db" "github.com/OpenListTeam/OpenList/v4/internal/model" "github.com/OpenListTeam/OpenList/v4/pkg/utils" "github.com/OpenListTeam/OpenList/v4/server/common" + "gorm.io/gorm" ) // Proppatch describes a property update instruction as defined in RFC 4918. @@ -170,79 +172,39 @@ var liveProps = map[xml.Name]struct { // TODO(nigeltao) merge props and allprop? // Props returns the status of the properties named pnames for resource name. -// -// Each Propstat has a unique status and each property name will only be part -// of one Propstat element. -func props(ctx context.Context, ls LockSystem, fi model.Obj, pnames []xml.Name) ([]Propstat, error) { - //f, err := fs.OpenFile(ctx, name, os.O_RDONLY, 0) - //if err != nil { - // return nil, err - //} - //defer f.Close() - //fi, err := f.Stat() - //if err != nil { - // return nil, err - //} +func props(ctx context.Context, ls LockSystem, name string, fi model.Obj, pnames []xml.Name) ([]Propstat, error) { isDir := fi.IsDir() - - var deadProps map[xml.Name]Property - // ??? what is this for? - //if dph, ok := f.(DeadPropsHolder); ok { - // deadProps, err = dph.DeadProps() - // if err != nil { - // return nil, err - // } - //} - + deadProps, err := getDeadProps(name) + if err != nil { + return nil, err + } pstatOK := Propstat{Status: http.StatusOK} pstatNotFound := Propstat{Status: http.StatusNotFound} for _, pn := range pnames { - // If this file has dead properties, check if they contain pn. if dp, ok := deadProps[pn]; ok { pstatOK.Props = append(pstatOK.Props, dp) continue } - // Otherwise, it must either be a live property or we don't know it. if prop := liveProps[pn]; prop.findFn != nil && (prop.dir || !isDir) { innerXML, err := prop.findFn(ctx, ls, fi.GetName(), fi) if err != nil { return nil, err } - pstatOK.Props = append(pstatOK.Props, Property{ - XMLName: pn, - InnerXML: []byte(innerXML), - }) + pstatOK.Props = append(pstatOK.Props, Property{XMLName: pn, InnerXML: []byte(innerXML)}) } else { - pstatNotFound.Props = append(pstatNotFound.Props, Property{ - XMLName: pn, - }) + pstatNotFound.Props = append(pstatNotFound.Props, Property{XMLName: pn}) } } return makePropstats(pstatOK, pstatNotFound), nil } // Propnames returns the property names defined for resource name. -func propnames(ctx context.Context, ls LockSystem, fi model.Obj) ([]xml.Name, error) { - //f, err := fs.OpenFile(ctx, name, os.O_RDONLY, 0) - //if err != nil { - // return nil, err - //} - //defer f.Close() - //fi, err := f.Stat() - //if err != nil { - // return nil, err - //} +func propnames(_ context.Context, _ LockSystem, name string, fi model.Obj) ([]xml.Name, error) { isDir := fi.IsDir() - - var deadProps map[xml.Name]Property - // ??? what is this for? - //if dph, ok := f.(DeadPropsHolder); ok { - // deadProps, err = dph.DeadProps() - // if err != nil { - // return nil, err - // } - //} - + deadProps, err := getDeadProps(name) + if err != nil { + return nil, err + } pnames := make([]xml.Name, 0, len(liveProps)+len(deadProps)) for pn, prop := range liveProps { if prop.findFn != nil && (prop.dir || !isDir) { @@ -255,20 +217,12 @@ func propnames(ctx context.Context, ls LockSystem, fi model.Obj) ([]xml.Name, er return pnames, nil } -// Allprop returns the properties defined for resource name and the properties -// named in include. -// -// Note that RFC 4918 defines 'allprop' to return the DAV: properties defined -// within the RFC plus dead properties. Other live properties should only be -// returned if they are named in 'include'. -// -// See http://www.webdav.org/specs/rfc4918.html#METHOD_PROPFIND -func allprop(ctx context.Context, ls LockSystem, fi model.Obj, include []xml.Name) ([]Propstat, error) { - pnames, err := propnames(ctx, ls, fi) +// Allprop returns the properties defined for resource name and the properties named in include. +func allprop(ctx context.Context, ls LockSystem, name string, fi model.Obj, include []xml.Name) ([]Propstat, error) { + pnames, err := propnames(ctx, ls, name, fi) if err != nil { return nil, err } - // Add names from include if they are not already covered in pnames. nameset := make(map[xml.Name]bool) for _, pn := range pnames { nameset[pn] = true @@ -278,11 +232,10 @@ func allprop(ctx context.Context, ls LockSystem, fi model.Obj, include []xml.Nam pnames = append(pnames, pn) } } - return props(ctx, ls, fi, pnames) + return props(ctx, ls, name, fi, pnames) } -// Patch patches the properties of resource name. The return values are -// constrained in the same manner as DeadPropsHolder.Patch. +// Patch patches the properties of resource name. func patch(ctx context.Context, ls LockSystem, name string, patches []Proppatch) ([]Propstat, error) { conflict := false loop: @@ -314,53 +267,78 @@ loop: return makePropstats(pstatForbidden, pstatFailedDep), nil } - // ------------------------------------------------------------ - //f, err := fs.OpenFile(ctx, name, os.O_RDWR, 0) - //if err != nil { - // return nil, err - //} - //defer f.Close() - //if dph, ok := f.(DeadPropsHolder); ok { - // ret, err := dph.Patch(patches) - // if err != nil { - // return nil, err - // } - // // http://www.webdav.org/specs/rfc4918.html#ELEMENT_propstat says that - // // "The contents of the prop XML element must only list the names of - // // properties to which the result in the status element applies." - // for _, pstat := range ret { - // for i, p := range pstat.Props { - // pstat.Props[i] = Property{XMLName: p.XMLName} - // } - // } - // return ret, nil - //} - // ------------------------------------------------------------ - - // The file doesn't implement the optional DeadPropsHolder interface, so - // all patches are forbidden. - pstat := Propstat{Status: http.StatusForbidden} - for _, patch := range patches { - for _, p := range patch.Props { - pstat.Props = append(pstat.Props, Property{XMLName: p.XMLName}) + database := db.GetDb() + if database == nil { + return nil, errors.New("webdav property database is not initialized") + } + pstat := Propstat{Status: http.StatusOK} + var err error + for attempt := 0; attempt < 40; attempt++ { + pstat.Props = nil + err = database.Transaction(func(tx *gorm.DB) error { + for _, patch := range patches { + for _, prop := range patch.Props { + if patch.Remove { + if err := tx.Where("path = ? AND namespace = ? AND name = ?", name, prop.XMLName.Space, prop.XMLName.Local).Delete(&model.WebDAVProperty{}).Error; err != nil { + return err + } + } else { + row := model.WebDAVProperty{ + Path: name, + Namespace: prop.XMLName.Space, + Name: prop.XMLName.Local, + } + if err := tx.Where("path = ? AND namespace = ? AND name = ?", row.Path, row.Namespace, row.Name). + Assign(model.WebDAVProperty{Lang: prop.Lang, InnerXML: prop.InnerXML}). + FirstOrCreate(&row).Error; err != nil { + return err + } + } + pstat.Props = append(pstat.Props, Property{XMLName: prop.XMLName}) + } + } + return nil + }) + if err == nil || !strings.Contains(err.Error(), "database is locked") { + break } + time.Sleep(time.Duration(attempt+1) * 50 * time.Millisecond) + } + if err != nil { + return nil, err } return []Propstat{pstat}, nil } +func getDeadProps(path string) (map[xml.Name]Property, error) { + database := db.GetDb() + if database == nil { + return nil, errors.New("webdav property database is not initialized") + } + var rows []model.WebDAVProperty + if err := database.Where("path = ?", path).Find(&rows).Error; err != nil { + return nil, err + } + props := make(map[xml.Name]Property, len(rows)) + for _, row := range rows { + props[xml.Name{Space: row.Namespace, Local: row.Name}] = Property{ + XMLName: xml.Name{Space: row.Namespace, Local: row.Name}, + Lang: row.Lang, + InnerXML: row.InnerXML, + } + } + return props, nil +} + func escapeXML(s string) string { for i := 0; i < len(s); i++ { - // As an optimization, if s contains only ASCII letters, digits or a - // few special characters, the escaped value is s itself and we don't - // need to allocate a buffer and convert between string and []byte. switch c := s[i]; { case c == ' ' || c == '_' || - ('+' <= c && c <= '9') || // Digits as well as + , - . and / + ('+' <= c && c <= '9') || ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'): continue } - // Otherwise, go through the full escaping process. var buf bytes.Buffer xml.EscapeText(&buf, []byte(s)) return buf.String() @@ -368,6 +346,35 @@ func escapeXML(s string) string { return s } +func moveDeadProps(src, dst string) error { + database := db.GetDb() + if database == nil { + return errors.New("webdav property database is not initialized") + } + return database.Transaction(func(tx *gorm.DB) error { + var rows []model.WebDAVProperty + if err := tx.Where("path = ? OR path LIKE ?", src, src+"/%").Find(&rows).Error; err != nil { + return err + } + for _, row := range rows { + newPath := dst + strings.TrimPrefix(row.Path, src) + if err := tx.Where("path = ? AND namespace = ? AND name = ?", newPath, row.Namespace, row.Name).Delete(&model.WebDAVProperty{}).Error; err != nil { + return err + } + copy := row + copy.ID = 0 + copy.Path = newPath + if err := tx.Create(©).Error; err != nil { + return err + } + if err := tx.Delete(&row).Error; err != nil { + return err + } + } + return nil + }) +} + func findResourceType(ctx context.Context, ls LockSystem, name string, fi model.Obj) (string, error) { if fi.IsDir() { return ``, nil From 2ef425354ccfaca2ab2ad5042506140689978507 Mon Sep 17 00:00:00 2001 From: Intro Date: Tue, 4 Aug 2026 20:10:34 +0800 Subject: [PATCH 03/12] fix(webdav): enforce locks on resolved paths, support shared locks Signed-off-by: Lythen --- server/webdav/lock.go | 40 +++++++++++++++++----- server/webdav/webdav.go | 74 +++++++++++++++++------------------------ server/webdav/xml.go | 72 ++++++++++++++++++++++++++++----------- 3 files changed, 115 insertions(+), 71 deletions(-) diff --git a/server/webdav/lock.go b/server/webdav/lock.go index 344ac5ceaf..b3d5ac5eed 100644 --- a/server/webdav/lock.go +++ b/server/webdav/lock.go @@ -109,6 +109,8 @@ type LockDetails struct { // ZeroDepth is whether the lock has zero depth. If it does not have zero // depth, it has infinite depth. ZeroDepth bool + // Shared is whether the lock may coexist with other shared locks on Root. + Shared bool } // NewMemLS returns a new in-memory LockSystem. @@ -184,15 +186,10 @@ func (m *memLS) Confirm(now time.Time, name0, name1 string, conditions ...Condit }, nil } -// lookup returns the node n that locks the named resource, provided that n -// matches at least one of the given conditions and that lock isn't held by -// another party. Otherwise, it returns nil. -// -// n may be a parent of the named resource, if n is an infinite depth lock. -func (m *memLS) lookup(name string, conditions ...Condition) (n *memLSNode) { +func (m *memLS) lookup(name string, conditions ...Condition) *memLSNode { // TODO: support Condition.Not and Condition.ETag. for _, c := range conditions { - n = m.byToken[c.Token] + n := m.byToken[c.Token] if n == nil || n.held { continue } @@ -235,6 +232,14 @@ func (m *memLS) Create(now time.Time, details LockDetails) (string, error) { m.collectExpiredNodes(now) details.Root = slashClean(details.Root) + if details.Shared { + if n := m.byName[details.Root]; n != nil && n.token != "" && n.details.Shared && n.details.ZeroDepth == details.ZeroDepth && !n.held { + token := m.nextToken() + n.sharedTokens[token] = struct{}{} + m.byToken[token] = n + return token, nil + } + } if !m.canCreate(details.Root, details.ZeroDepth) { return "", ErrLocked } @@ -242,6 +247,9 @@ func (m *memLS) Create(now time.Time, details LockDetails) (string, error) { n.token = m.nextToken() m.byToken[n.token] = n n.details = details + if details.Shared { + n.sharedTokens = map[string]struct{}{n.token: {}} + } if n.details.Duration >= 0 { n.expiry = now.Add(n.details.Duration) heap.Push(&m.byExpiry, n) @@ -284,6 +292,13 @@ func (m *memLS) Unlock(now time.Time, token string) error { if n.held { return ErrLocked } + if n.details.Shared { + delete(m.byToken, token) + delete(n.sharedTokens, token) + if len(n.sharedTokens) != 0 { + return nil + } + } m.remove(n) return nil } @@ -334,7 +349,13 @@ func (m *memLS) create(name string) (ret *memLSNode) { } func (m *memLS) remove(n *memLSNode) { - delete(m.byToken, n.token) + if n.details.Shared { + for token := range n.sharedTokens { + delete(m.byToken, token) + } + } else { + delete(m.byToken, n.token) + } n.token = "" walkToRoot(n.details.Root, func(name0 string, first bool) bool { x := m.byName[name0] @@ -380,7 +401,8 @@ type memLSNode struct { // if this node does not expire, or has expired. byExpiryIndex int // held is whether this node's lock is actively held by a Confirm call. - held bool + held bool + sharedTokens map[string]struct{} } type byExpiry []*memLSNode diff --git a/server/webdav/webdav.go b/server/webdav/webdav.go index 06d1431ac3..91a365daa0 100644 --- a/server/webdav/webdav.go +++ b/server/webdav/webdav.go @@ -82,10 +82,6 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { status, err = h.handleUnlock(brw, r) case "PROPFIND": status, err = h.handlePropfind(brw, r) - // if there is a error for PROPFIND, we should be as an empty folder to the client - if err != nil { - status = http.StatusNotFound - } case "PROPPATCH": status, err = h.handleProppatch(brw, r) } @@ -122,11 +118,6 @@ func (h *Handler) lock(now time.Time, root string) (token string, status int, er func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func(), status int, err error) { hdr := r.Header.Get("If") if hdr == "" { - // An empty If header means that the client hasn't previously created locks. - // Even if this client doesn't care about locks, we still need to check that - // the resources aren't locked by another client, so we create temporary - // locks that would conflict with another client's locks. These temporary - // locks are unlocked at the end of the HTTP request. now, srcToken, dstToken := time.Now(), "", "" if src != "" { srcToken, status, err = h.lock(now, src) @@ -143,7 +134,6 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() return nil, status, err } } - return func() { if dstToken != "" { h.LockSystem.Unlock(now, dstToken) @@ -158,7 +148,7 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() if !ok { return nil, http.StatusBadRequest, errInvalidIfHeader } - // ih is a disjunction (OR) of ifLists, so any ifList will do. + user, _ := r.Context().Value(conf.UserKey).(*model.User) for _, l := range ih.lists { lsrc := l.resourceTag if lsrc == "" { @@ -175,6 +165,12 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() if err != nil { return nil, status, err } + if user != nil { + lsrc, err = user.JoinPath(lsrc) + if err != nil { + return nil, http.StatusForbidden, err + } + } } release, err = h.LockSystem.Confirm(time.Now(), lsrc, dst, l.conditions...) if err == ErrConfirmationFailed { @@ -185,10 +181,6 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() } return release, 0, nil } - // Section 10.4.1 says that "If this header is evaluated and all state lists - // fail, then the request must fail with a 412 (Precondition Failed) status." - // We follow the spec even though the cond_put_corrupt_token test case from - // the litmus test warns on seeing a 412 instead of a 423 (Locked). return nil, http.StatusPreconditionFailed, ErrLocked } @@ -212,9 +204,7 @@ func (h *Handler) handleOptions(w http.ResponseWriter, r *http.Request) (status } } w.Header().Set("Allow", allow) - // http://www.webdav.org/specs/rfc4918.html#dav.compliance.classes w.Header().Set("DAV", "1, 2") - // http://msdn.microsoft.com/en-au/library/cc250217.aspx w.Header().Set("MS-Author-Via", "DAV") return 0, nil } @@ -295,12 +285,6 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) (status i if err != nil { return status, err } - release, status, err := h.confirmLocks(r, reqPath, "") - if err != nil { - return status, err - } - defer release() - ctx := r.Context() user := ctx.Value(conf.UserKey).(*model.User) if !user.CanRemove() { @@ -310,6 +294,11 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) (status i if err != nil { return http.StatusForbidden, err } + release, status, err := h.confirmLocks(r, reqPath, "") + if err != nil { + return status, err + } + defer release() // TODO: return MultiStatus where appropriate. // "godoc os RemoveAll" says that "If the path does not exist, RemoveAll @@ -350,11 +339,6 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int, if reqPath == "" { return http.StatusMethodNotAllowed, nil } - release, status, err := h.confirmLocks(r, reqPath, "") - if err != nil { - return status, err - } - defer release() // TODO(rost): Support the If-Match, If-None-Match headers? See bradfitz' // comments in http.checkEtag. ctx := r.Context() @@ -363,6 +347,11 @@ func (h *Handler) handlePut(w http.ResponseWriter, r *http.Request) (status int, if err != nil { return http.StatusForbidden, err } + release, status, err := h.confirmLocks(r, reqPath, "") + if err != nil { + return status, err + } + defer release() size := r.ContentLength if size < 0 { sizeStr := r.Header.Get("X-File-Size") @@ -428,18 +417,17 @@ func (h *Handler) handleMkcol(w http.ResponseWriter, r *http.Request) (status in if err != nil { return status, err } - release, status, err := h.confirmLocks(r, reqPath, "") - if err != nil { - return status, err - } - defer release() - ctx := r.Context() user := ctx.Value(conf.UserKey).(*model.User) reqPath, err = user.JoinPath(reqPath) if err != nil { return http.StatusForbidden, err } + release, status, err := h.confirmLocks(r, reqPath, "") + if err != nil { + return status, err + } + defer release() if r.ContentLength > 0 { return http.StatusUnsupportedMediaType, nil @@ -627,6 +615,7 @@ func (h *Handler) handleLock(w http.ResponseWriter, r *http.Request) (retStatus Duration: duration, OwnerXML: li.Owner.InnerXML, ZeroDepth: depth == 0, + Shared: li.Shared != nil, } token, err = h.LockSystem.Create(now, ld) if err != nil { @@ -758,7 +747,7 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status } var pstats []Propstat if pf.Propname != nil { - pnames, err := propnames(ctx, h.LockSystem, info) + pnames, err := propnames(ctx, h.LockSystem, reqPath, info) if err != nil { return err } @@ -768,9 +757,9 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status } pstats = append(pstats, pstat) } else if pf.Allprop != nil { - pstats, err = allprop(ctx, h.LockSystem, info, pf.Prop) + pstats, err = allprop(ctx, h.LockSystem, reqPath, info, pf.Prop) } else { - pstats, err = props(ctx, h.LockSystem, info, pf.Prop) + pstats, err = props(ctx, h.LockSystem, reqPath, info, pf.Prop) } if err != nil { return err @@ -798,18 +787,17 @@ func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (statu if err != nil { return status, err } - release, status, err := h.confirmLocks(r, reqPath, "") - if err != nil { - return status, err - } - defer release() - ctx := r.Context() user := ctx.Value(conf.UserKey).(*model.User) reqPath, err = user.JoinPath(reqPath) if err != nil { return http.StatusForbidden, err } + release, status, err := h.confirmLocks(r, reqPath, "") + if err != nil { + return status, err + } + defer release() meta, err := op.GetNearestMeta(reqPath) if err != nil && !errors.Is(errors.Cause(err), errs.MetaNotFound) { return http.StatusInternalServerError, err diff --git a/server/webdav/xml.go b/server/webdav/xml.go index c9ec61dffa..ee649bead4 100644 --- a/server/webdav/xml.go +++ b/server/webdav/xml.go @@ -62,9 +62,7 @@ func readLockInfo(r io.Reader) (li lockInfo, status int, err error) { } return lockInfo{}, http.StatusBadRequest, err } - // We only support exclusive (non-shared) write locks. In practice, these are - // the only types of locks that seem to matter. - if li.Exclusive == nil || li.Shared != nil || li.Write == nil { + if (li.Exclusive == nil) == (li.Shared == nil) || li.Write == nil { return lockInfo{}, http.StatusNotImplemented, errUnsupportedLockInfo } return li, 0, nil @@ -86,18 +84,22 @@ func writeLockInfo(w io.Writer, token string, ld LockDetails) (int, error) { if ld.ZeroDepth { depth = "0" } + scope := "exclusive" + if ld.Shared { + scope = "shared" + } timeout := ld.Duration / time.Second return fmt.Fprintf(w, "\n"+ "\n"+ - " \n"+ - " \n"+ - " %s\n"+ - " %s\n"+ - " Second-%d\n"+ - " %s\n"+ - " %s\n"+ + "\t\n"+ + "\t\n"+ + "\t%s\n"+ + "\t%s\n"+ + "\tSecond-%d\n"+ + "\t%s\n"+ + "\t%s\n"+ "", - depth, ld.OwnerXML, timeout, escape(token), escape(ld.Root), + scope, depth, ld.OwnerXML, timeout, escape(token), escape(ld.Root), ) } @@ -176,19 +178,24 @@ type propfind struct { } func readPropfind(r io.Reader) (pf propfind, status int, err error) { - c := countingReader{r: r} - if err = ixml.NewDecoder(&c).Decode(&pf); err != nil { + body, err := io.ReadAll(r) + if err != nil { + return propfind{}, http.StatusBadRequest, err + } + if len(body) == 0 { + // An empty body means to propfind allprop. + // http://www.webdav.org/specs/rfc4918.html#METHOD_PROPFIND + return propfind{Allprop: new(struct{})}, 0, nil + } + if hasEmptyNamespacePrefix(body) { + return propfind{}, http.StatusBadRequest, errInvalidPropfind + } + if err = ixml.NewDecoder(bytes.NewReader(body)).Decode(&pf); err != nil { if err == io.EOF { - if c.n == 0 { - // An empty body means to propfind allprop. - // http://www.webdav.org/specs/rfc4918.html#METHOD_PROPFIND - return propfind{Allprop: new(struct{})}, 0, nil - } err = errInvalidPropfind } return propfind{}, http.StatusBadRequest, err } - if pf.Allprop == nil && pf.Include != nil { return propfind{}, http.StatusBadRequest, errInvalidPropfind } @@ -204,6 +211,33 @@ func readPropfind(r io.Reader) (pf propfind, status int, err error) { return pf, 0, nil } +func hasEmptyNamespacePrefix(body []byte) bool { + for offset := 0; ; { + i := bytes.Index(body[offset:], []byte("xmlns:")) + if i < 0 { + return false + } + i += offset + len("xmlns:") + j := i + for j < len(body) && body[j] != '=' && body[j] != '>' && body[j] != '/' && body[j] != ' ' && body[j] != '\t' && body[j] != '\n' && body[j] != '\r' { + j++ + } + for j < len(body) && (body[j] == ' ' || body[j] == '\t' || body[j] == '\n' || body[j] == '\r') { + j++ + } + if j < len(body) && body[j] == '=' { + j++ + for j < len(body) && (body[j] == ' ' || body[j] == '\t' || body[j] == '\n' || body[j] == '\r') { + j++ + } + if j+1 < len(body) && (body[j] == '\'' || body[j] == '"') && body[j+1] == body[j] { + return true + } + } + offset = i + } +} + // Property represents a single DAV resource property as defined in RFC 4918. // See http://www.webdav.org/specs/rfc4918.html#data.model.for.resource.properties type Property struct { From c2eba42dc393effb7c60f0b97bd370247ffead70 Mon Sep 17 00:00:00 2001 From: Intro Date: Tue, 4 Aug 2026 20:55:34 +0800 Subject: [PATCH 04/12] fix(fs): fall back to transfer task for same-dir copy Signed-off-by: Lythen --- internal/fs/copy_move.go | 26 ++++++++++++++------------ internal/op/fs.go | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/internal/fs/copy_move.go b/internal/fs/copy_move.go index 92ad24fe32..3048fd38aa 100644 --- a/internal/fs/copy_move.go +++ b/internal/fs/copy_move.go @@ -114,19 +114,21 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, ds if utils.IsBool(skipHook...) { ctx = context.WithValue(ctx, conf.SkipHookKey, struct{}{}) } - // A named copy cannot use the driver's destination-name-independent Copy - // operation. Fall back to the transfer task so the target name is kept. - if dstName == "" { - if taskType == copy || taskType == merge { - err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) - if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - return nil, err - } - } else { - err = op.Move(ctx, srcStorage, srcObjActualPath, dstDirActualPath) - if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - return nil, err + if taskType == copy || taskType == merge { + err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + if err == nil && dstName != "" { + srcObjName := stdpath.Base(srcObjActualPath) + if srcObjName != dstName { + err = op.Rename(ctx, srcStorage, stdpath.Join(dstDirActualPath, srcObjName), dstName) + } } + return nil, err + } + } else { + err = op.Move(ctx, srcStorage, srcObjActualPath, dstDirActualPath) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + return nil, err } } } diff --git a/internal/op/fs.go b/internal/op/fs.go index f82a3ca8f8..90c1545bf2 100644 --- a/internal/op/fs.go +++ b/internal/op/fs.go @@ -514,7 +514,7 @@ func Copy(ctx context.Context, storage driver.Driver, srcPath, dstDirPath string srcPath = utils.FixAndCleanPath(srcPath) dstDirPath = utils.FixAndCleanPath(dstDirPath) if dstDirPath == stdpath.Dir(srcPath) { - return errors.New("copy in place") + return errors.WithStack(errs.NotImplement) } srcRawObj, err := Get(ctx, storage, srcPath, true) if err != nil { From 616c546b49d7494c14df08f5315d27493ae62546 Mon Sep 17 00:00:00 2001 From: Intro Date: Tue, 4 Aug 2026 22:16:43 +0800 Subject: [PATCH 05/12] fix(webdav): escape LIKE patterns, check dstDir is dir, bound PROPFIND body Signed-off-by: Lythen --- server/webdav/file.go | 8 ++++++-- server/webdav/prop.go | 11 +++++++---- server/webdav/xml.go | 11 +++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/server/webdav/file.go b/server/webdav/file.go index e647be8179..4d2d5a25ef 100644 --- a/server/webdav/file.go +++ b/server/webdav/file.go @@ -55,11 +55,13 @@ func moveFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, srcMeta, srcDir) || !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } - if _, err = fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { + if dstDirInfo, err := fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { if errs.IsObjectNotFound(err) { return http.StatusConflict, err } return http.StatusMethodNotAllowed, err + } else if !dstDirInfo.IsDir() { + return http.StatusConflict, nil } dstExisted := false if _, err = fs.Get(ctx, dst, &fs.GetArgs{}); err == nil { @@ -119,11 +121,13 @@ func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } - if _, err = fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { + if dstDirInfo, err := fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { if errs.IsObjectNotFound(err) { return http.StatusConflict, err } return http.StatusMethodNotAllowed, err + } else if !dstDirInfo.IsDir() { + return http.StatusConflict, nil } dstExisted := false if _, err = fs.Get(ctx, dst, &fs.GetArgs{}); err == nil { diff --git a/server/webdav/prop.go b/server/webdav/prop.go index 66e09b88d5..886f120c78 100644 --- a/server/webdav/prop.go +++ b/server/webdav/prop.go @@ -351,16 +351,19 @@ func moveDeadProps(src, dst string) error { if database == nil { return errors.New("webdav property database is not initialized") } + escapedSrc := strings.ReplaceAll(strings.ReplaceAll(src, "%", "\\%"), "_", "\\_") + escapedDst := strings.ReplaceAll(strings.ReplaceAll(dst, "%", "\\%"), "_", "\\_") return database.Transaction(func(tx *gorm.DB) error { + // Clear destination subtree for overwrite MOVE. + if err := tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", dst, escapedDst+"/%").Delete(&model.WebDAVProperty{}).Error; err != nil { + return err + } var rows []model.WebDAVProperty - if err := tx.Where("path = ? OR path LIKE ?", src, src+"/%").Find(&rows).Error; err != nil { + if err := tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", src, escapedSrc+"/%").Find(&rows).Error; err != nil { return err } for _, row := range rows { newPath := dst + strings.TrimPrefix(row.Path, src) - if err := tx.Where("path = ? AND namespace = ? AND name = ?", newPath, row.Namespace, row.Name).Delete(&model.WebDAVProperty{}).Error; err != nil { - return err - } copy := row copy.ID = 0 copy.Path = newPath diff --git a/server/webdav/xml.go b/server/webdav/xml.go index ee649bead4..d87eb863cd 100644 --- a/server/webdav/xml.go +++ b/server/webdav/xml.go @@ -178,13 +178,20 @@ type propfind struct { } func readPropfind(r io.Reader) (pf propfind, status int, err error) { - body, err := io.ReadAll(r) + // 64KB is more than enough for a well-formed PROPFIND body. + const maxBody = 64 << 10 + body, err := io.ReadAll(io.LimitReader(r, maxBody)) if err != nil { return propfind{}, http.StatusBadRequest, err } + // If the limit was reached, the body was too large. + if len(body) >= maxBody { + // Drain any remaining bytes so the connection stays usable. + _, _ = io.Copy(io.Discard, r) + return propfind{}, http.StatusRequestEntityTooLarge, nil + } if len(body) == 0 { // An empty body means to propfind allprop. - // http://www.webdav.org/specs/rfc4918.html#METHOD_PROPFIND return propfind{Allprop: new(struct{})}, 0, nil } if hasEmptyNamespacePrefix(body) { From 973ab537cfdc0f4d8135704bb9e2f33d4c29ed8e Mon Sep 17 00:00:00 2001 From: Intro Date: Wed, 12 Aug 2026 02:03:49 +0800 Subject: [PATCH 06/12] fix(webdav): escape backslash in LIKE, reduce dead prop retry Signed-off-by: Lythen --- server/webdav/prop.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/webdav/prop.go b/server/webdav/prop.go index 886f120c78..4a7f6f61af 100644 --- a/server/webdav/prop.go +++ b/server/webdav/prop.go @@ -273,7 +273,7 @@ loop: } pstat := Propstat{Status: http.StatusOK} var err error - for attempt := 0; attempt < 40; attempt++ { + for attempt := 0; attempt < 10; attempt++ { pstat.Props = nil err = database.Transaction(func(tx *gorm.DB) error { for _, patch := range patches { @@ -302,7 +302,7 @@ loop: if err == nil || !strings.Contains(err.Error(), "database is locked") { break } - time.Sleep(time.Duration(attempt+1) * 50 * time.Millisecond) + time.Sleep(time.Duration(attempt+1) * 20 * time.Millisecond) } if err != nil { return nil, err @@ -351,8 +351,8 @@ func moveDeadProps(src, dst string) error { if database == nil { return errors.New("webdav property database is not initialized") } - escapedSrc := strings.ReplaceAll(strings.ReplaceAll(src, "%", "\\%"), "_", "\\_") - escapedDst := strings.ReplaceAll(strings.ReplaceAll(dst, "%", "\\%"), "_", "\\_") + escapedSrc := strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(src, "\\", "\\\\"), "%", "\\%"), "_", "\\_") + escapedDst := strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(dst, "\\", "\\\\"), "%", "\\%"), "_", "\\_") return database.Transaction(func(tx *gorm.DB) error { // Clear destination subtree for overwrite MOVE. if err := tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", dst, escapedDst+"/%").Delete(&model.WebDAVProperty{}).Error; err != nil { From c6ea8662f9eea1f9b0fda88b4c45637e7b49c60c Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 02:38:32 +0000 Subject: [PATCH 07/12] fix(webdav): isolate shared lock instances - Track each lock token with independent details and expiry state.\n- Keep resource nodes for path conflict accounting only.\n- Cover independent shared-lock expiration in memLS tests. --- server/webdav/lock.go | 201 ++++++++++++++++++------------------- server/webdav/lock_test.go | 103 ++++++++++++------- 2 files changed, 164 insertions(+), 140 deletions(-) diff --git a/server/webdav/lock.go b/server/webdav/lock.go index b3d5ac5eed..0d279e06dd 100644 --- a/server/webdav/lock.go +++ b/server/webdav/lock.go @@ -117,7 +117,7 @@ type LockDetails struct { func NewMemLS() LockSystem { return &memLS{ byName: make(map[string]*memLSNode), - byToken: make(map[string]*memLSNode), + byToken: make(map[string]*memLSLock), gen: uint64(time.Now().Unix()), } } @@ -125,7 +125,7 @@ func NewMemLS() LockSystem { type memLS struct { mu sync.Mutex byName map[string]*memLSNode - byToken map[string]*memLSNode + byToken map[string]*memLSLock gen uint64 // byExpiry only contains those nodes whose LockDetails have a finite // Duration and are yet to expire. @@ -151,7 +151,7 @@ func (m *memLS) Confirm(now time.Time, name0, name1 string, conditions ...Condit defer m.mu.Unlock() m.collectExpiredNodes(now) - var n0, n1 *memLSNode + var n0, n1 *memLSLock if name0 != "" { if n0 = m.lookup(slashClean(name0), conditions...); n0 == nil { return nil, ErrConfirmationFailed @@ -186,43 +186,43 @@ func (m *memLS) Confirm(now time.Time, name0, name1 string, conditions ...Condit }, nil } -func (m *memLS) lookup(name string, conditions ...Condition) *memLSNode { +func (m *memLS) lookup(name string, conditions ...Condition) *memLSLock { // TODO: support Condition.Not and Condition.ETag. for _, c := range conditions { - n := m.byToken[c.Token] - if n == nil || n.held { + lock := m.byToken[c.Token] + if lock == nil || lock.held { continue } - if name == n.details.Root { - return n + if name == lock.details.Root { + return lock } - if n.details.ZeroDepth { + if lock.details.ZeroDepth { continue } - if n.details.Root == "/" || strings.HasPrefix(name, n.details.Root+"/") { - return n + if lock.details.Root == "/" || strings.HasPrefix(name, lock.details.Root+"/") { + return lock } } return nil } -func (m *memLS) hold(n *memLSNode) { - if n.held { +func (m *memLS) hold(lock *memLSLock) { + if lock.held { panic("webdav: memLS inconsistent held state") } - n.held = true - if n.details.Duration >= 0 && n.byExpiryIndex >= 0 { - heap.Remove(&m.byExpiry, n.byExpiryIndex) + lock.held = true + if lock.details.Duration >= 0 && lock.byExpiryIndex >= 0 { + heap.Remove(&m.byExpiry, lock.byExpiryIndex) } } -func (m *memLS) unhold(n *memLSNode) { - if !n.held { +func (m *memLS) unhold(lock *memLSLock) { + if !lock.held { panic("webdav: memLS inconsistent held state") } - n.held = false - if n.details.Duration >= 0 { - heap.Push(&m.byExpiry, n) + lock.held = false + if lock.details.Duration >= 0 { + heap.Push(&m.byExpiry, lock) } } @@ -232,29 +232,23 @@ func (m *memLS) Create(now time.Time, details LockDetails) (string, error) { m.collectExpiredNodes(now) details.Root = slashClean(details.Root) - if details.Shared { - if n := m.byName[details.Root]; n != nil && n.token != "" && n.details.Shared && n.details.ZeroDepth == details.ZeroDepth && !n.held { - token := m.nextToken() - n.sharedTokens[token] = struct{}{} - m.byToken[token] = n - return token, nil - } - } - if !m.canCreate(details.Root, details.ZeroDepth) { + if !m.canCreate(details.Root, details.ZeroDepth, details.Shared) { return "", ErrLocked } - n := m.create(details.Root) - n.token = m.nextToken() - m.byToken[n.token] = n - n.details = details - if details.Shared { - n.sharedTokens = map[string]struct{}{n.token: {}} + node := m.create(details.Root) + lock := &memLSLock{ + token: m.nextToken(), + details: details, + node: node, + byExpiryIndex: -1, } - if n.details.Duration >= 0 { - n.expiry = now.Add(n.details.Duration) - heap.Push(&m.byExpiry, n) + node.locks[lock.token] = lock + m.byToken[lock.token] = lock + if lock.details.Duration >= 0 { + lock.expiry = now.Add(lock.details.Duration) + heap.Push(&m.byExpiry, lock) } - return n.token, nil + return lock.token, nil } func (m *memLS) Refresh(now time.Time, token string, duration time.Duration) (LockDetails, error) { @@ -262,22 +256,22 @@ func (m *memLS) Refresh(now time.Time, token string, duration time.Duration) (Lo defer m.mu.Unlock() m.collectExpiredNodes(now) - n := m.byToken[token] - if n == nil { + lock := m.byToken[token] + if lock == nil { return LockDetails{}, ErrNoSuchLock } - if n.held { + if lock.held { return LockDetails{}, ErrLocked } - if n.byExpiryIndex >= 0 { - heap.Remove(&m.byExpiry, n.byExpiryIndex) + if lock.byExpiryIndex >= 0 { + heap.Remove(&m.byExpiry, lock.byExpiryIndex) } - n.details.Duration = duration - if n.details.Duration >= 0 { - n.expiry = now.Add(n.details.Duration) - heap.Push(&m.byExpiry, n) + lock.details.Duration = duration + if lock.details.Duration >= 0 { + lock.expiry = now.Add(lock.details.Duration) + heap.Push(&m.byExpiry, lock) } - return n.details, nil + return lock.details, nil } func (m *memLS) Unlock(now time.Time, token string) error { @@ -285,41 +279,42 @@ func (m *memLS) Unlock(now time.Time, token string) error { defer m.mu.Unlock() m.collectExpiredNodes(now) - n := m.byToken[token] - if n == nil { + lock := m.byToken[token] + if lock == nil { return ErrNoSuchLock } - if n.held { + if lock.held { return ErrLocked } - if n.details.Shared { - delete(m.byToken, token) - delete(n.sharedTokens, token) - if len(n.sharedTokens) != 0 { - return nil - } - } - m.remove(n) + m.remove(lock) return nil } -func (m *memLS) canCreate(name string, zeroDepth bool) bool { +func (m *memLS) canCreate(name string, zeroDepth bool, shared ...bool) bool { + wantShared := len(shared) > 0 && shared[0] return walkToRoot(name, func(name0 string, first bool) bool { n := m.byName[name0] if n == nil { return true } if first { - if n.token != "" { - // The target node is already locked. - return false + if len(n.locks) != 0 { + if !wantShared || (!zeroDepth && n.refCount != len(n.locks)) { + return false + } + for _, lock := range n.locks { + if !lock.details.Shared || lock.details.ZeroDepth != zeroDepth { + return false + } + } + return true } - if !zeroDepth { + if !zeroDepth && n.refCount > 0 { // The requested lock depth is infinite, and the fact that n exists // (n != nil) means that a descendent of the target node is locked. return false } - } else if n.token != "" && !n.details.ZeroDepth { + } else if n.hasInfiniteLock() { // An ancestor of the target node is locked with infinite depth. return false } @@ -332,10 +327,8 @@ func (m *memLS) create(name string) (ret *memLSNode) { n := m.byName[name0] if n == nil { n = &memLSNode{ - details: LockDetails{ - Root: name0, - }, - byExpiryIndex: -1, + details: LockDetails{Root: name0}, + locks: make(map[string]*memLSLock), } m.byName[name0] = n } @@ -348,16 +341,10 @@ func (m *memLS) create(name string) (ret *memLSNode) { return ret } -func (m *memLS) remove(n *memLSNode) { - if n.details.Shared { - for token := range n.sharedTokens { - delete(m.byToken, token) - } - } else { - delete(m.byToken, n.token) - } - n.token = "" - walkToRoot(n.details.Root, func(name0 string, first bool) bool { +func (m *memLS) remove(lock *memLSLock) { + delete(m.byToken, lock.token) + delete(lock.node.locks, lock.token) + walkToRoot(lock.details.Root, func(name0 string, first bool) bool { x := m.byName[name0] x.refCount-- if x.refCount == 0 { @@ -365,8 +352,8 @@ func (m *memLS) remove(n *memLSNode) { } return true }) - if n.byExpiryIndex >= 0 { - heap.Remove(&m.byExpiry, n.byExpiryIndex) + if lock.byExpiryIndex >= 0 { + heap.Remove(&m.byExpiry, lock.byExpiryIndex) } } @@ -387,25 +374,33 @@ func walkToRoot(name string, f func(name0 string, first bool) bool) bool { } type memLSNode struct { - // details are the lock metadata. Even if this node's name is not explicitly locked, - // details.Root will still equal the node's name. + // details identifies the resource path represented by this node. details LockDetails - // token is the unique identifier for this node's lock. An empty token means that - // this node is not explicitly locked. - token string - // refCount is the number of self-or-descendent nodes that are explicitly locked. + // locks contains the independent lock instances rooted at this resource. + locks map[string]*memLSLock + // refCount is the number of self-or-descendent lock instances. refCount int - // expiry is when this node's lock expires. - expiry time.Time - // byExpiryIndex is the index of this node in memLS.byExpiry. It is -1 - // if this node does not expire, or has expired. +} + +type memLSLock struct { + token string + details LockDetails + node *memLSNode + expiry time.Time byExpiryIndex int - // held is whether this node's lock is actively held by a Confirm call. - held bool - sharedTokens map[string]struct{} + held bool +} + +func (n *memLSNode) hasInfiniteLock() bool { + for _, lock := range n.locks { + if !lock.details.ZeroDepth { + return true + } + } + return false } -type byExpiry []*memLSNode +type byExpiry []*memLSLock func (b *byExpiry) Len() int { return len(*b) @@ -422,18 +417,18 @@ func (b *byExpiry) Swap(i, j int) { } func (b *byExpiry) Push(x interface{}) { - n := x.(*memLSNode) - n.byExpiryIndex = len(*b) - *b = append(*b, n) + lock := x.(*memLSLock) + lock.byExpiryIndex = len(*b) + *b = append(*b, lock) } func (b *byExpiry) Pop() interface{} { i := len(*b) - 1 - n := (*b)[i] + lock := (*b)[i] (*b)[i] = nil - n.byExpiryIndex = -1 + lock.byExpiryIndex = -1 *b = (*b)[:i] - return n + return lock } const infiniteTimeout = -1 diff --git a/server/webdav/lock_test.go b/server/webdav/lock_test.go index e7fe97061b..c6ffc0a553 100644 --- a/server/webdav/lock_test.go +++ b/server/webdav/lock_test.go @@ -182,7 +182,10 @@ func TestMemLSLookup(t *testing.T) { goodToken := "" base := m.byName[baseName] if base != nil && (suffix == "" || !lockTestZeroDepth(baseName)) { - goodToken = base.token + for token := range base.locks { + goodToken = token + break + } } for _, token := range []string{badToken, goodToken} { @@ -449,6 +452,46 @@ func TestMemLSExpiry(t *testing.T) { } } +func TestMemLSSharedLockExpiry(t *testing.T) { + m := NewMemLS().(*memLS) + now := time.Unix(0, 0) + first, err := m.Create(now, LockDetails{Root: "/shared", Duration: infiniteTimeout, ZeroDepth: true, Shared: true}) + if err != nil { + t.Fatal(err) + } + second, err := m.Create(now, LockDetails{Root: "/shared", Duration: 2 * time.Second, ZeroDepth: true, Shared: true}) + if err != nil { + t.Fatal(err) + } + third, err := m.Create(now, LockDetails{Root: "/shared", Duration: 4 * time.Second, ZeroDepth: true, Shared: true}) + if err != nil { + t.Fatal(err) + } + + if got := m.byToken[first].details.Duration; got != infiniteTimeout { + t.Fatalf("first lock duration = %v, want infinite", got) + } + if got := m.byToken[second].details.Duration; got != 2*time.Second { + t.Fatalf("second lock duration = %v, want 2s", got) + } + if got := m.byToken[third].details.Duration; got != 4*time.Second { + t.Fatalf("third lock duration = %v, want 4s", got) + } + + m.mu.Lock() + m.collectExpiredNodes(now.Add(2 * time.Second)) + m.mu.Unlock() + if m.byToken[second] != nil { + t.Fatal("second shared lock did not expire independently") + } + if m.byToken[first] == nil || m.byToken[third] == nil { + t.Fatal("independent shared lock expired with second lock") + } + if err := m.consistent(); err != nil { + t.Fatal(err) + } +} + func TestMemLS(t *testing.T) { now := time.Unix(0, 0) m := NewMemLS().(*memLS) @@ -578,8 +621,10 @@ func (m *memLS) consistent() error { // so strings.HasPrefix is equivalent to self-or-descendent name match. // We don't have to worry about "/foo/bar" being a false positive match // for "/foo/b". - if strings.HasPrefix(name0, name) && n0.token != "" { - list = append(list, name0) + if strings.HasPrefix(name0, name) { + for range n0.locks { + list = append(list, name0) + } } } if n.refCount != len(list) { @@ -588,50 +633,34 @@ func (m *memLS) consistent() error { name, n.refCount, list, len(list)) } - // A node n is in m.byToken if it has a non-empty token. - if n.token != "" { - if _, ok := m.byToken[n.token]; !ok { - return fmt.Errorf("node at name %q has token %q but not in m.byToken", name, n.token) - } - } - - // A node n is in m.byExpiry if it has a non-negative byExpiryIndex. - if n.byExpiryIndex >= 0 { - if n.byExpiryIndex >= len(m.byExpiry) { - return fmt.Errorf("node at name %q has byExpiryIndex %d but m.byExpiry has length %d", name, n.byExpiryIndex, len(m.byExpiry)) - } - if n != m.byExpiry[n.byExpiryIndex] { - return fmt.Errorf("node at name %q has byExpiryIndex %d but that indexes a different node", name, n.byExpiryIndex) + for token, lock := range n.locks { + if lock.token != token || lock.node != n || m.byToken[token] != lock { + return fmt.Errorf("lock %q at node %q is inconsistent", token, name) } } } - for token, n := range m.byToken { - // The map keys should be consistent with the node's copy of the key. - if n.token != token { - return fmt.Errorf("node token %q != byToken map key %q", n.token, token) + for token, lock := range m.byToken { + if lock.token != token { + return fmt.Errorf("lock token %q != byToken map key %q", lock.token, token) } - - // Every node in m.byToken is in m.byName. - if _, ok := m.byName[n.details.Root]; !ok { - return fmt.Errorf("node at name %q in m.byToken but not in m.byName", n.details.Root) + if lock.node == nil || lock.node.locks[token] != lock { + return fmt.Errorf("lock %q is missing from its node", token) + } + if m.byName[lock.details.Root] != lock.node { + return fmt.Errorf("lock %q has inconsistent root %q", token, lock.details.Root) } } - for i, n := range m.byExpiry { - // The slice indices should be consistent with the node's copy of the index. - if n.byExpiryIndex != i { - return fmt.Errorf("node byExpiryIndex %d != byExpiry slice index %d", n.byExpiryIndex, i) + for i, lock := range m.byExpiry { + if lock.byExpiryIndex != i { + return fmt.Errorf("lock byExpiryIndex %d != byExpiry slice index %d", lock.byExpiryIndex, i) } - - // Every node in m.byExpiry is in m.byName. - if _, ok := m.byName[n.details.Root]; !ok { - return fmt.Errorf("node at name %q in m.byExpiry but not in m.byName", n.details.Root) + if m.byToken[lock.token] != lock { + return fmt.Errorf("lock %q is missing from byToken", lock.token) } - - // No node in m.byExpiry should be held. - if n.held { - return fmt.Errorf("node at name %q in m.byExpiry is held", n.details.Root) + if lock.held { + return fmt.Errorf("lock at name %q is held", lock.details.Root) } } return nil From f3d555dd1028e4f0b71095c1d60b833dbcf5ce0f Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 02:39:32 +0000 Subject: [PATCH 08/12] fix(webdav): preserve dead property lifecycle - Copy and replace dead properties with COPY destinations.\n- Remove dead-property subtrees after successful DELETE.\n- Stage MOVE properties before clearing the destination subtree.\n- Add database-backed regression coverage for copy and delete. --- server/webdav/file.go | 3 ++ server/webdav/prop.go | 53 +++++++++++++++++++--- server/webdav/prop_test.go | 93 ++++++++++++++++++++++++++++++++++++++ server/webdav/webdav.go | 3 ++ 4 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 server/webdav/prop_test.go diff --git a/server/webdav/file.go b/server/webdav/file.go index 4d2d5a25ef..99d9809f3d 100644 --- a/server/webdav/file.go +++ b/server/webdav/file.go @@ -146,6 +146,9 @@ func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int if err != nil { return http.StatusInternalServerError, err } + if err = copyDeadProps(src, dst); err != nil { + return http.StatusInternalServerError, err + } if dstExisted { return http.StatusNoContent, nil } diff --git a/server/webdav/prop.go b/server/webdav/prop.go index 4a7f6f61af..be2edfe340 100644 --- a/server/webdav/prop.go +++ b/server/webdav/prop.go @@ -346,20 +346,61 @@ func escapeXML(s string) string { return s } -func moveDeadProps(src, dst string) error { +func deadPropsLikePath(path string) string { + return strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(path, "\\", "\\\\"), "%", "\\%"), "_", "\\_") +} + +func deadPropsSubtree(tx *gorm.DB, path string) *gorm.DB { + return tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", path, deadPropsLikePath(path)+"/%") +} + +func deleteDeadProps(path string) error { + database := db.GetDb() + if database == nil { + return errors.New("webdav property database is not initialized") + } + return database.Transaction(func(tx *gorm.DB) error { + return deadPropsSubtree(tx, path).Delete(&model.WebDAVProperty{}).Error + }) +} + +func copyDeadProps(src, dst string) error { database := db.GetDb() if database == nil { return errors.New("webdav property database is not initialized") } - escapedSrc := strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(src, "\\", "\\\\"), "%", "\\%"), "_", "\\_") - escapedDst := strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(dst, "\\", "\\\\"), "%", "\\%"), "_", "\\_") return database.Transaction(func(tx *gorm.DB) error { - // Clear destination subtree for overwrite MOVE. - if err := tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", dst, escapedDst+"/%").Delete(&model.WebDAVProperty{}).Error; err != nil { + var rows []model.WebDAVProperty + if err := deadPropsSubtree(tx, src).Find(&rows).Error; err != nil { return err } + if err := deadPropsSubtree(tx, dst).Delete(&model.WebDAVProperty{}).Error; err != nil { + return err + } + for _, row := range rows { + copy := row + copy.ID = 0 + copy.Path = dst + strings.TrimPrefix(row.Path, src) + if err := tx.Create(©).Error; err != nil { + return err + } + } + return nil + }) +} + +func moveDeadProps(src, dst string) error { + database := db.GetDb() + if database == nil { + return errors.New("webdav property database is not initialized") + } + return database.Transaction(func(tx *gorm.DB) error { var rows []model.WebDAVProperty - if err := tx.Where("path = ? OR path LIKE ? ESCAPE '\\'", src, escapedSrc+"/%").Find(&rows).Error; err != nil { + if err := deadPropsSubtree(tx, src).Find(&rows).Error; err != nil { + return err + } + // Clear destination only after the source rows are staged in memory. + if err := deadPropsSubtree(tx, dst).Delete(&model.WebDAVProperty{}).Error; err != nil { return err } for _, row := range rows { diff --git a/server/webdav/prop_test.go b/server/webdav/prop_test.go new file mode 100644 index 0000000000..035f8b6df8 --- /dev/null +++ b/server/webdav/prop_test.go @@ -0,0 +1,93 @@ +package webdav + +import ( + "encoding/xml" + "testing" + + "github.com/OpenListTeam/OpenList/v4/internal/conf" + "github.com/OpenListTeam/OpenList/v4/internal/db" + "github.com/OpenListTeam/OpenList/v4/internal/model" + "github.com/glebarez/sqlite" + "gorm.io/gorm" +) + +func setupWebDAVPropertyDB(t *testing.T) *gorm.DB { + t.Helper() + conf.Conf = conf.DefaultConfig(t.TempDir()) + database, err := gorm.Open(sqlite.Open("file:webdav-property-test?mode=memory&cache=shared"), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + db.Init(database) + t.Cleanup(func() { + sqlDB, err := database.DB() + if err == nil { + _ = sqlDB.Close() + } + }) + return database +} + +func TestDeadPropsCopyAndDelete(t *testing.T) { + database := setupWebDAVPropertyDB(t) + rows := []model.WebDAVProperty{ + {Path: "/src", Namespace: "urn:test", Name: "root", InnerXML: []byte("")}, + {Path: "/src/child", Namespace: "urn:test", Name: "child", InnerXML: []byte("")}, + {Path: "/dst", Namespace: "urn:test", Name: "stale", InnerXML: []byte("")}, + {Path: "/dst/old", Namespace: "urn:test", Name: "old", InnerXML: []byte("")}, + {Path: "/other", Namespace: "urn:test", Name: "keep", InnerXML: []byte("")}, + } + if err := database.Create(&rows).Error; err != nil { + t.Fatal(err) + } + + if err := copyDeadProps("/src", "/dst"); err != nil { + t.Fatal(err) + } + if _, ok := mustDeadProp(t, "/dst", "root"); !ok { + t.Fatal("root property was not copied") + } + if _, ok := mustDeadProp(t, "/dst/child", "child"); !ok { + t.Fatal("child property was not copied") + } + if _, ok := mustDeadProp(t, "/dst", "stale"); ok { + t.Fatal("stale destination property was not replaced") + } + if _, ok := mustDeadProp(t, "/dst/old", "old"); ok { + t.Fatal("stale destination subtree property was not removed") + } + if _, ok := mustDeadProp(t, "/src", "root"); !ok { + t.Fatal("source property was changed by copy") + } + + if err := deleteDeadProps("/dst"); err != nil { + t.Fatal(err) + } + if props, err := getDeadProps("/dst"); err != nil { + t.Fatal(err) + } else if len(props) != 0 { + t.Fatalf("destination properties remain after delete: %v", props) + } + if props, err := getDeadProps("/dst/child"); err != nil { + t.Fatal(err) + } else if len(props) != 0 { + t.Fatalf("destination subtree properties remain after delete: %v", props) + } + if _, ok := mustDeadProp(t, "/other", "keep"); !ok { + t.Fatal("delete removed an unrelated property") + } +} + +func mustDeadProp(t *testing.T, path, name string) (Property, bool) { + t.Helper() + props, err := getDeadProps(path) + if err != nil { + t.Fatal(err) + } + prop, ok := props[xmlName("urn:test", name)] + return prop, ok +} + +func xmlName(namespace, name string) xml.Name { + return xml.Name{Space: namespace, Local: name} +} diff --git a/server/webdav/webdav.go b/server/webdav/webdav.go index 91a365daa0..fe9ebd272d 100644 --- a/server/webdav/webdav.go +++ b/server/webdav/webdav.go @@ -321,6 +321,9 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) (status i if err := fs.Remove(ctx, reqPath); err != nil { return http.StatusMethodNotAllowed, err } + if err := deleteDeadProps(reqPath); err != nil { + return http.StatusInternalServerError, err + } //fs.ClearCache(path.Dir(reqPath)) return http.StatusNoContent, nil } From 65833a8cc615f3800638410ed68d1cbced392289 Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 02:44:04 +0000 Subject: [PATCH 09/12] fix(webdav): protect overwrite copy and move - Validate source objects before touching overwrite destinations.\n- Stage COPY content under a collision-free name and restore destinations on failure.\n- Preserve MOVE destinations with a backup until the resource operation succeeds.\n- Route named same-storage copies through the direct destination-name transfer path. --- internal/fs/copy_move.go | 18 +++--- internal/fs/copy_move_test.go | 23 ++++++++ server/webdav/file.go | 100 +++++++++++++++++++++++++++++----- 3 files changed, 120 insertions(+), 21 deletions(-) create mode 100644 internal/fs/copy_move_test.go diff --git a/internal/fs/copy_move.go b/internal/fs/copy_move.go index 3048fd38aa..0d42529713 100644 --- a/internal/fs/copy_move.go +++ b/internal/fs/copy_move.go @@ -100,6 +100,10 @@ func (t *FileTransferTask) SetRetry(retry int, maxRetry int) { } } +func canUseNativeCopy(sameStorage bool, dstName string) bool { + return sameStorage && dstName == "" +} + func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, dstName string, skipHook ...bool) (task.TaskExtensionInfo, error) { srcStorage, srcObjActualPath, err := op.GetStorageAndActualPath(srcObjPath) if err != nil { @@ -110,19 +114,16 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, ds return nil, errors.WithMessage(err, "failed get dst storage") } - if srcStorage.GetStorage() == dstStorage.GetStorage() { + // A named copy must not stage under the source basename: that path may be + // an unrelated destination object. Use the transfer path so DstName is + // applied directly by the upload. + if canUseNativeCopy(srcStorage.GetStorage() == dstStorage.GetStorage(), dstName) { if utils.IsBool(skipHook...) { ctx = context.WithValue(ctx, conf.SkipHookKey, struct{}{}) } if taskType == copy || taskType == merge { err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - if err == nil && dstName != "" { - srcObjName := stdpath.Base(srcObjActualPath) - if srcObjName != dstName { - err = op.Rename(ctx, srcStorage, stdpath.Join(dstDirActualPath, srcObjName), dstName) - } - } return nil, err } } else { @@ -133,7 +134,8 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, ds } } - // not in the same storage + // Use the transfer path when native copy cannot preserve the destination + // name or the source and destination storages differ. t := &FileTransferTask{ TaskData: TaskData{ SrcStorage: srcStorage, diff --git a/internal/fs/copy_move_test.go b/internal/fs/copy_move_test.go new file mode 100644 index 0000000000..75277aa407 --- /dev/null +++ b/internal/fs/copy_move_test.go @@ -0,0 +1,23 @@ +package fs + +import "testing" + +func TestCanUseNativeCopy(t *testing.T) { + tests := []struct { + name string + sameStorage bool + dstName string + want bool + }{ + {name: "unnamed same-storage copy", sameStorage: true, want: true}, + {name: "named same-storage copy", sameStorage: true, dstName: "bar", want: false}, + {name: "unnamed cross-storage copy", sameStorage: false, want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := canUseNativeCopy(tt.sameStorage, tt.dstName); got != tt.want { + t.Fatalf("canUseNativeCopy(%t, %q) = %t, want %t", tt.sameStorage, tt.dstName, got, tt.want) + } + }) + } +} diff --git a/server/webdav/file.go b/server/webdav/file.go index 99d9809f3d..7f69f1c1ff 100644 --- a/server/webdav/file.go +++ b/server/webdav/file.go @@ -16,6 +16,7 @@ import ( "github.com/OpenListTeam/OpenList/v4/internal/model" "github.com/OpenListTeam/OpenList/v4/internal/op" "github.com/OpenListTeam/OpenList/v4/server/common" + "github.com/google/uuid" "github.com/pkg/errors" ) @@ -28,6 +29,72 @@ func slashClean(name string) string { return path.Clean(name) } +func moveResource(ctx context.Context, src, dstDir, srcDir, srcName, dstName string) error { + if srcDir == dstDir { + return fs.Rename(ctx, src, dstName) + } + if _, err := fs.Move(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir); err != nil { + return err + } + if srcName != dstName { + return fs.Rename(ctx, path.Join(dstDir, srcName), dstName) + } + return nil +} + +func copyWithOverwrite(ctx context.Context, src, dstDir, dstName string) error { + stageName := ".openlist-copy-" + uuid.NewString() + stagePath := path.Join(dstDir, stageName) + stageReady := false + defer func() { + if stageReady { + _ = fs.Remove(ctx, stagePath) + } + }() + + if _, err := fs.CopyTo(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir, stageName); err != nil { + return err + } + stageReady = true + + backupName := ".openlist-backup-" + uuid.NewString() + backupPath := path.Join(dstDir, backupName) + if err := fs.Rename(ctx, path.Join(dstDir, dstName), backupName); err != nil { + return errors.WithMessage(err, "failed to stage overwrite destination") + } + if err := fs.Rename(ctx, stagePath, dstName); err != nil { + restoreErr := fs.Rename(ctx, backupPath, dstName) + if restoreErr != nil { + return errors.WithMessagef(err, "failed to install staged copy and restore destination: %v", restoreErr) + } + return errors.WithMessage(err, "failed to install staged copy") + } + stageReady = false + if err := fs.Remove(ctx, backupPath); err != nil { + return errors.WithMessage(err, "failed to remove overwrite backup") + } + return nil +} + +func moveWithOverwrite(ctx context.Context, src, dst, dstDir, srcDir, srcName, dstName string) error { + backupName := ".openlist-backup-" + uuid.NewString() + backupPath := path.Join(dstDir, backupName) + if err := fs.Rename(ctx, dst, backupName); err != nil { + return errors.WithMessage(err, "failed to stage overwrite destination") + } + if err := moveResource(ctx, src, dstDir, srcDir, srcName, dstName); err != nil { + restoreErr := fs.Rename(ctx, backupPath, dstName) + if restoreErr != nil { + return errors.WithMessagef(err, "move failed and destination restore failed: %v", restoreErr) + } + return err + } + if err := fs.Remove(ctx, backupPath); err != nil { + return errors.WithMessage(err, "failed to remove overwrite backup") + } + return nil +} + // moveFiles moves files and/or directories from src to dst. // Individual item permission checks are skipped for performance reasons. // @@ -55,6 +122,12 @@ func moveFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, srcMeta, srcDir) || !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } + if _, err = fs.Get(ctx, src, &fs.GetArgs{}); err != nil { + if errs.IsObjectNotFound(err) { + return http.StatusNotFound, err + } + return http.StatusInternalServerError, err + } if dstDirInfo, err := fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { if errs.IsObjectNotFound(err) { return http.StatusConflict, err @@ -69,19 +142,13 @@ func moveFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !overwrite { return http.StatusPreconditionFailed, nil } - if err = fs.Remove(ctx, dst); err != nil { - return http.StatusInternalServerError, err - } } else if !errs.IsObjectNotFound(err) { return http.StatusInternalServerError, err } - if srcDir == dstDir { - err = fs.Rename(ctx, src, dstName) + if dstExisted && overwrite { + err = moveWithOverwrite(ctx, src, dst, dstDir, srcDir, srcName, dstName) } else { - _, err = fs.Move(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir) - if err == nil && srcName != dstName { - err = fs.Rename(ctx, path.Join(dstDir, srcName), dstName) - } + err = moveResource(ctx, src, dstDir, srcDir, srcName, dstName) } if err != nil { return http.StatusInternalServerError, err @@ -121,6 +188,12 @@ func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !common.CanWrite(user, dstMeta, dstDir) { return http.StatusForbidden, nil } + if _, err = fs.Get(ctx, src, &fs.GetArgs{}); err != nil { + if errs.IsObjectNotFound(err) { + return http.StatusNotFound, err + } + return http.StatusInternalServerError, err + } if dstDirInfo, err := fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil { if errs.IsObjectNotFound(err) { return http.StatusConflict, err @@ -135,14 +208,15 @@ func copyFiles(ctx context.Context, src, dst string, overwrite bool) (status int if !overwrite { return http.StatusPreconditionFailed, nil } - if err = fs.Remove(ctx, dst); err != nil { - return http.StatusInternalServerError, err - } } else if !errs.IsObjectNotFound(err) { return http.StatusInternalServerError, err } - _, err = fs.CopyTo(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir, dstName) + if dstExisted && overwrite { + err = copyWithOverwrite(ctx, src, dstDir, dstName) + } else { + _, err = fs.CopyTo(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir, dstName) + } if err != nil { return http.StatusInternalServerError, err } From 7c691f7f968ce47e10a5d3761ec29f766c6494df Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 02:47:55 +0000 Subject: [PATCH 10/12] fix(webdav): harden overwrite staging - Skip NoOverwriteUpload backup handling for collision-free internal COPY staging.\n- Treat post-commit overwrite backup cleanup failures as warnings. --- internal/conf/const.go | 1 + internal/op/fs.go | 2 +- server/webdav/file.go | 9 ++++++--- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/conf/const.go b/internal/conf/const.go index cc8a51d416..0ba441418e 100644 --- a/internal/conf/const.go +++ b/internal/conf/const.go @@ -196,4 +196,5 @@ const ( PathKey SharingIDKey SkipHookKey + SkipNoOverwriteKey ) diff --git a/internal/op/fs.go b/internal/op/fs.go index 90c1545bf2..566794d98a 100644 --- a/internal/op/fs.go +++ b/internal/op/fs.go @@ -632,7 +632,7 @@ func Put(ctx context.Context, storage driver.Driver, dstDirPath string, file mod if err != nil { return errors.WithMessagef(err, "while uploading, failed remove existing file which size = 0") } - } else if storage.Config().NoOverwriteUpload { + } else if storage.Config().NoOverwriteUpload && ctx.Value(conf.SkipNoOverwriteKey) == nil { // try to rename old obj err = Rename(ctx, storage, dstPath, tempName) if err != nil { diff --git a/server/webdav/file.go b/server/webdav/file.go index 7f69f1c1ff..8851444321 100644 --- a/server/webdav/file.go +++ b/server/webdav/file.go @@ -18,6 +18,7 @@ import ( "github.com/OpenListTeam/OpenList/v4/server/common" "github.com/google/uuid" "github.com/pkg/errors" + log "github.com/sirupsen/logrus" ) // slashClean is equivalent to but slightly more efficient than @@ -52,7 +53,9 @@ func copyWithOverwrite(ctx context.Context, src, dstDir, dstName string) error { } }() - if _, err := fs.CopyTo(context.WithValue(ctx, conf.NoTaskKey, struct{}{}), src, dstDir, stageName); err != nil { + stageCtx := context.WithValue(ctx, conf.NoTaskKey, struct{}{}) + stageCtx = context.WithValue(stageCtx, conf.SkipNoOverwriteKey, struct{}{}) + if _, err := fs.CopyTo(stageCtx, src, dstDir, stageName); err != nil { return err } stageReady = true @@ -71,7 +74,7 @@ func copyWithOverwrite(ctx context.Context, src, dstDir, dstName string) error { } stageReady = false if err := fs.Remove(ctx, backupPath); err != nil { - return errors.WithMessage(err, "failed to remove overwrite backup") + log.Warnf("failed to remove COPY overwrite backup %s: %v", backupPath, err) } return nil } @@ -90,7 +93,7 @@ func moveWithOverwrite(ctx context.Context, src, dst, dstDir, srcDir, srcName, d return err } if err := fs.Remove(ctx, backupPath); err != nil { - return errors.WithMessage(err, "failed to remove overwrite backup") + log.Warnf("failed to remove MOVE overwrite backup %s: %v", backupPath, err) } return nil } From 82383d291539efbd792f18fcc8a6cdb8ae77b935 Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 02:50:58 +0000 Subject: [PATCH 11/12] test(webdav): update lock lookup expectations - Compare lookup results with independent lock instances.\n- Keep invalid token expectations nil after the lock state split. --- server/webdav/lock_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/webdav/lock_test.go b/server/webdav/lock_test.go index c6ffc0a553..15b6839b77 100644 --- a/server/webdav/lock_test.go +++ b/server/webdav/lock_test.go @@ -194,9 +194,9 @@ func TestMemLSLookup(t *testing.T) { } got := m.lookup(name, Condition{Token: token}) - want := base - if token == badToken { - want = nil + var want *memLSLock + if token != badToken { + want = m.byToken[token] } if got != want { t.Errorf("name=%-20qtoken=%q (bad=%t): got %p, want %p", From 77fa7293aeb6b629ea81679bb94fd353904756b3 Mon Sep 17 00:00:00 2001 From: Lythen Date: Tue, 25 Aug 2026 04:16:06 +0000 Subject: [PATCH 12/12] fix(webdav): complete litmus compliance - Preserve named same-storage collection copies with isolated native staging.\n- Evaluate WebDAV ETag conditions and distinguish 412 from 423 lock failures.\n- Return 201 for lock-null resources and cover the new condition behavior. --- internal/fs/copy_move.go | 88 +++++++++++++++++++++++++++---- internal/fs/copy_move_test.go | 20 +++---- server/webdav/lock_test.go | 19 +++++++ server/webdav/webdav.go | 98 ++++++++++++++++++++++++++++++++++- 4 files changed, 204 insertions(+), 21 deletions(-) diff --git a/internal/fs/copy_move.go b/internal/fs/copy_move.go index 0d42529713..fad2cef104 100644 --- a/internal/fs/copy_move.go +++ b/internal/fs/copy_move.go @@ -7,6 +7,7 @@ import ( "time" "github.com/OpenListTeam/OpenList/v4/internal/conf" + "github.com/OpenListTeam/OpenList/v4/internal/driver" "github.com/OpenListTeam/OpenList/v4/internal/errs" "github.com/OpenListTeam/OpenList/v4/internal/model" "github.com/OpenListTeam/OpenList/v4/internal/op" @@ -16,7 +17,9 @@ import ( "github.com/OpenListTeam/OpenList/v4/pkg/utils" "github.com/OpenListTeam/OpenList/v4/server/common" "github.com/OpenListTeam/tache" + "github.com/google/uuid" "github.com/pkg/errors" + log "github.com/sirupsen/logrus" ) type taskType uint8 @@ -100,8 +103,69 @@ func (t *FileTransferTask) SetRetry(retry int, maxRetry int) { } } -func canUseNativeCopy(sameStorage bool, dstName string) bool { - return sameStorage && dstName == "" +func supportsNamedNativeCopy(storage driver.Driver) bool { + _, hasMkdir := storage.(driver.Mkdir) + if !hasMkdir { + _, hasMkdir = storage.(driver.MkdirResult) + } + _, hasCopy := storage.(driver.Copy) + if !hasCopy { + _, hasCopy = storage.(driver.CopyResult) + } + _, hasMove := storage.(driver.Move) + if !hasMove { + _, hasMove = storage.(driver.MoveResult) + } + _, hasRename := storage.(driver.Rename) + if !hasRename { + _, hasRename = storage.(driver.RenameResult) + } + _, hasRemove := storage.(driver.Remove) + return hasMkdir && hasCopy && hasMove && hasRename && hasRemove +} + +func namedCopyStageObjectPath(stageDirPath, srcPath, dstName string) string { + name := stdpath.Base(srcPath) + if dstName != "" { + name = dstName + } + return stdpath.Join(stageDirPath, name) +} + +func copyNamedInStorage(ctx context.Context, storage driver.Driver, srcPath, dstDirPath, dstName string) error { + stageDirPath := stdpath.Join(dstDirPath, ".openlist-copy-"+uuid.NewString()) + stageCtx := context.WithValue(ctx, conf.SkipHookKey, struct{}{}) + if err := op.MakeDir(stageCtx, storage, stageDirPath); err != nil { + return errors.WithMessage(err, "failed create copy staging directory") + } + stageCreated := true + defer func() { + if stageCreated { + if err := op.Remove(stageCtx, storage, stageDirPath); err != nil { + log.Warnf("failed remove copy staging directory %s: %v", stageDirPath, err) + } + } + }() + + if err := op.Copy(stageCtx, storage, srcPath, stageDirPath); err != nil { + return err + } + srcName := stdpath.Base(srcPath) + stageObjPath := namedCopyStageObjectPath(stageDirPath, srcPath, "") + if srcName != dstName { + if err := op.Rename(stageCtx, storage, stageObjPath, dstName); err != nil { + return err + } + stageObjPath = namedCopyStageObjectPath(stageDirPath, srcPath, dstName) + } + if err := op.Move(ctx, storage, stageObjPath, dstDirPath); err != nil { + return err + } + if err := op.Remove(stageCtx, storage, stageDirPath); err != nil { + return errors.WithMessage(err, "failed remove copy staging directory") + } + stageCreated = false + return nil } func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, dstName string, skipHook ...bool) (task.TaskExtensionInfo, error) { @@ -114,17 +178,23 @@ func transfer(ctx context.Context, taskType taskType, srcObjPath, dstDirPath, ds return nil, errors.WithMessage(err, "failed get dst storage") } - // A named copy must not stage under the source basename: that path may be - // an unrelated destination object. Use the transfer path so DstName is - // applied directly by the upload. - if canUseNativeCopy(srcStorage.GetStorage() == dstStorage.GetStorage(), dstName) { + if srcStorage.GetStorage() == dstStorage.GetStorage() { if utils.IsBool(skipHook...) { ctx = context.WithValue(ctx, conf.SkipHookKey, struct{}{}) } if taskType == copy || taskType == merge { - err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) - if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { - return nil, err + if dstName != "" { + if supportsNamedNativeCopy(srcStorage) { + err = copyNamedInStorage(ctx, srcStorage, srcObjActualPath, dstDirActualPath, dstName) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + return nil, err + } + } + } else { + err = op.Copy(ctx, srcStorage, srcObjActualPath, dstDirActualPath) + if !errors.Is(err, errs.NotImplement) && !errors.Is(err, errs.NotSupport) { + return nil, err + } } } else { err = op.Move(ctx, srcStorage, srcObjActualPath, dstDirActualPath) diff --git a/internal/fs/copy_move_test.go b/internal/fs/copy_move_test.go index 75277aa407..95f6575107 100644 --- a/internal/fs/copy_move_test.go +++ b/internal/fs/copy_move_test.go @@ -2,21 +2,21 @@ package fs import "testing" -func TestCanUseNativeCopy(t *testing.T) { +func TestNamedCopyStageObjectPath(t *testing.T) { tests := []struct { - name string - sameStorage bool - dstName string - want bool + name string + stageDir string + srcPath string + dstName string + want string }{ - {name: "unnamed same-storage copy", sameStorage: true, want: true}, - {name: "named same-storage copy", sameStorage: true, dstName: "bar", want: false}, - {name: "unnamed cross-storage copy", sameStorage: false, want: false}, + {name: "source basename", stageDir: "/dst/.stage", srcPath: "/src/foo", want: "/dst/.stage/foo"}, + {name: "requested name", stageDir: "/dst/.stage", srcPath: "/src/foo", dstName: "bar", want: "/dst/.stage/bar"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := canUseNativeCopy(tt.sameStorage, tt.dstName); got != tt.want { - t.Fatalf("canUseNativeCopy(%t, %q) = %t, want %t", tt.sameStorage, tt.dstName, got, tt.want) + if got := namedCopyStageObjectPath(tt.stageDir, tt.srcPath, tt.dstName); got != tt.want { + t.Fatalf("namedCopyStageObjectPath(%q, %q, %q) = %q, want %q", tt.stageDir, tt.srcPath, tt.dstName, got, tt.want) } }) } diff --git a/server/webdav/lock_test.go b/server/webdav/lock_test.go index 15b6839b77..8353c15897 100644 --- a/server/webdav/lock_test.go +++ b/server/webdav/lock_test.go @@ -762,3 +762,22 @@ func TestParseTimeout(t *testing.T) { } } } + +func TestHasNegatedTokenCondition(t *testing.T) { + tests := []struct { + name string + conditions []Condition + want bool + }{ + {name: "positive token", conditions: []Condition{{Token: "DAV:no-lock"}}}, + {name: "negated token", conditions: []Condition{{Not: true, Token: "DAV:no-lock"}}, want: true}, + {name: "negated etag", conditions: []Condition{{Not: true, ETag: `"etag"`}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasNegatedTokenCondition(tt.conditions); got != tt.want { + t.Fatalf("hasNegatedTokenCondition(%v) = %t, want %t", tt.conditions, got, tt.want) + } + }) + } +} diff --git a/server/webdav/webdav.go b/server/webdav/webdav.go index fe9ebd272d..a1029cda22 100644 --- a/server/webdav/webdav.go +++ b/server/webdav/webdav.go @@ -115,6 +115,61 @@ func (h *Handler) lock(now time.Time, root string) (token string, status int, er return token, 0, nil } +func ifETagConditionsMatch(ctx context.Context, ls LockSystem, name string, conditions []Condition) (bool, error) { + var obj model.Obj + objLoaded, objMissing := false, false + for _, condition := range conditions { + if condition.ETag == "" { + continue + } + if !objLoaded { + var err error + obj, err = fs.Get(ctx, name, &fs.GetArgs{}) + objLoaded = true + if err != nil { + if !errs.IsObjectNotFound(err) { + return false, err + } + objMissing = true + } + } + matches := false + if !objMissing { + etag, err := findETag(ctx, ls, name, obj) + if err != nil { + return false, err + } + matches = etag == condition.ETag + } + if condition.Not { + matches = !matches + } + if !matches { + return false, nil + } + } + return true, nil +} + +func positiveTokenConditions(conditions []Condition) []Condition { + ret := make([]Condition, 0, len(conditions)) + for _, condition := range conditions { + if condition.Token != "" && !condition.Not { + ret = append(ret, condition) + } + } + return ret +} + +func hasNegatedTokenCondition(conditions []Condition) bool { + for _, condition := range conditions { + if condition.Token != "" && condition.Not { + return true + } + } + return false +} + func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func(), status int, err error) { hdr := r.Header.Get("If") if hdr == "" { @@ -148,7 +203,9 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() if !ok { return nil, http.StatusBadRequest, errInvalidIfHeader } - user, _ := r.Context().Value(conf.UserKey).(*model.User) + ctx := r.Context() + user, _ := ctx.Value(conf.UserKey).(*model.User) + etagMismatch, hasNegatedToken := false, false for _, l := range ih.lists { lsrc := l.resourceTag if lsrc == "" { @@ -172,7 +229,16 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() } } } - release, err = h.LockSystem.Confirm(time.Now(), lsrc, dst, l.conditions...) + matches, err := ifETagConditionsMatch(ctx, h.LockSystem, lsrc, l.conditions) + if err != nil { + return nil, http.StatusInternalServerError, err + } + if !matches { + etagMismatch = true + continue + } + hasNegatedToken = hasNegatedToken || hasNegatedTokenCondition(l.conditions) + release, err = h.LockSystem.Confirm(time.Now(), lsrc, dst, positiveTokenConditions(l.conditions)...) if err == ErrConfirmationFailed { continue } @@ -181,6 +247,28 @@ func (h *Handler) confirmLocks(r *http.Request, src, dst string) (release func() } return release, 0, nil } + if etagMismatch || !hasNegatedToken { + return nil, http.StatusPreconditionFailed, ErrLocked + } + + // Litmus expects a corrupt token paired with Not on an + // actively locked resource to report the lock conflict. + now := time.Now() + for _, name := range []string{src, dst} { + if name == "" { + continue + } + token, lockStatus, lockErr := h.lock(now, name) + if lockErr == ErrLocked { + return nil, lockStatus, lockErr + } + if lockErr != nil { + return nil, lockStatus, lockErr + } + if unlockErr := h.LockSystem.Unlock(now, token); unlockErr != nil { + return nil, http.StatusInternalServerError, unlockErr + } + } return nil, http.StatusPreconditionFailed, ErrLocked } @@ -613,6 +701,12 @@ func (h *Handler) handleLock(w http.ResponseWriter, r *http.Request) (retStatus if !common.CanWrite(user, meta, reqPath) { return http.StatusForbidden, errs.PermissionDenied } + if _, err := fs.Get(ctx, reqPath, &fs.GetArgs{}); err != nil { + if !errs.IsObjectNotFound(err) { + return http.StatusInternalServerError, err + } + created = true + } ld = LockDetails{ Root: reqPath, Duration: duration,