fix(webdav): correct COPY/MOVE semantics, dead property persistence, and lock enforcement - #2903
Open
devLythen wants to merge 8 commits into
Open
fix(webdav): correct COPY/MOVE semantics, dead property persistence, and lock enforcement#2903devLythen wants to merge 8 commits into
devLythen wants to merge 8 commits into
Conversation
Signed-off-by: Lythen <intro-iu@outlook.com>
Signed-off-by: Lythen <intro-iu@outlook.com>
Signed-off-by: Lythen <intro-iu@outlook.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the server’s WebDAV behavior by aligning COPY/MOVE responses and lock handling with RFC 4918 expectations, and by introducing persistent “dead property” storage so PROPPATCH/PROPFIND can round-trip custom properties across requests and MOVE operations.
Changes:
- Add shared-lock support and adjust lock confirmation to validate against the user-resolved resource path.
- Persist dead WebDAV properties in the DB (new model + migration) and surface them via PROPFIND; migrate them on MOVE.
- Fix COPY/MOVE target naming semantics by introducing a named-copy path (
CopyTo) and extending transfer-task metadata to carry the destination name.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/webdav/xml.go | Updates lockinfo parsing/output; changes PROPFIND body parsing/validation. |
| server/webdav/webdav.go | Adjusts request dispatch, lock confirmation logic, and property lookup call signatures. |
| server/webdav/prop.go | Implements DB-backed dead property persistence and MOVE migration. |
| server/webdav/lock.go | Extends lock model and in-memory lock system to support shared locks. |
| server/webdav/file.go | Refines MOVE/COPY filesystem behavior (destination validation, overwrite semantics, named copy). |
| internal/model/webdav_property.go | Adds DB model for persisted dead properties. |
| internal/db/db.go | Auto-migrates the new WebDAVProperty table. |
| internal/fs/fs.go | Adds CopyTo API for named COPY operations. |
| internal/fs/other.go | Extends transfer task metadata with DstName. |
| internal/fs/copy_move.go | Threads dstName through transfer execution and stream naming to preserve destination name. |
Suppressed comments (1)
server/webdav/file.go:126
copyFilesverifies thatdstDirexists but does not verify it is a directory. IfdstDirexists as a file, COPY should fail with 409 (Conflict) rather than attempting a copy into a non-collection path.
if _, err = fs.Get(ctx, dstDir, &fs.GetArgs{}); err != nil {
if errs.IsObjectNotFound(err) {
return http.StatusConflict, err
}
return http.StatusMethodNotAllowed, err
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Lythen <intro-iu@outlook.com>
…D body Signed-off-by: Lythen <intro-iu@outlook.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: Lythen <intro-iu@outlook.com>
Author
P1.1 数据库迁移脚本项目使用 GORM AutoMigrate，建表在 internal/db/db.go 中已有 new(model.WebDAVProperty) 注册。无独立SQL迁移目录，也不依赖特定数据库引擎的 DDL。GORM 的 AutoMigrate 对所有支持的引擎（SQLite / MySQL / PostgreSQL）行为一致，不会产生额外问题。 P1.2 LIKE 转义缺反斜杠已修复。当前转义为 P1.3 moveDeadProps 失败未处理已处理，MOVE 操作在 HTTP 层面正确失败返回 500，不会出现数据不一致。 P2.1 重试次数过多调整为 10 次。SQLite "database is locked" 在实际负载下 2 秒内足够恢复，超时直接向上层返回错误由 WebDAV handler 转为 500。 P2.2 hasEmptyNamespacePrefix 手写解析器该函数仅扫描 xmlns: 前缀和紧随其后的空引号对（"" 或 ''）。注释和CDATA中出现此模式不会产生安全后果——最多导致合法的 PROPFIND 被误拒为400，但实际客户端不会在注释中嵌入空命名空间声明。保持当前实现以规避引入完整 XML 解析器的复杂性。 P2.3 时间戳字段WebDAVProperty 表仅作为键值存储，属性本身无创建/修改时间的语义需求。如有调试需要可后续追加。 |
Author
PIKACHUIM
reviewed
Aug 14, 2026
PIKACHUIM
left a comment
PIKACHUIM
left a comment
Member
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @devLythen 提交此PR！我已完成代码评审，以下是评审结果。
🤖 AI 自动审核声明
本评审报告由 AI 自动生成，当前使用 Claude Opus 5 模型进行分析。
⚠️ AI 分析结果仅供参考，可能存在误判或遗漏。如您发现任何问题或有不同意见，欢迎随时提出讨论和纠正。
⚠️ 重要提醒：即使 AI 评审认为代码质量良好且建议合并，最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出决策。
📖 PR背景与需求
PR标题：fix(webdav): correct COPY/MOVE semantics, dead property persistence, and lock enforcement
关联Issue：无明确关联issue
需求说明：
修复 WebDAV 服务的多个严重功能问题：
- WebDAV
COPY方法稳定返回500 Internal Server Error PROPPATCH设置的自定义属性（死属性）无法持久化，PROPFIND无法检索- WebDAV 锁机制不生效，锁路径解析错误
- 不支持共享锁（shared locks）
- COPY/MOVE 操作的 HTTP 状态码语义不符合 RFC 4918 规范
预期目标：
- COPY 操作支持同目录/跨目录、绝对/相对 Destination，覆盖语义符合 RFC 4918
- 死属性可持久化到数据库，支持 PROPFIND 检索和 MOVE 时自动迁移
- 锁检查使用正确的用户路径解析后的实际资源路径
- 支持共享锁（shared locks）
- 拒绝含非法 XML 声明的 PROPFIND 请求体
- 通过 Litmus WebDAV 测试套件验证（copymove 13/13, props 30/30, locks 40/41）
📋 问题摘要
- ✅ 功能完整性高：修复了 WebDAV 的核心功能缺陷，大幅提升标准兼容性
- ✅ 测试覆盖充分：通过 Litmus 测试套件验证，从 61/80 提升到 102/103
⚠️ 数据库迁移影响：新增webdav_properties表，需要数据库迁移- 💡 代码质量建议：部分代码逻辑可以优化，错误处理需加强
📂 逐文件分析
internal/db/db.go
改动意图：
注册新的 WebDAV 死属性数据模型，使其能够自动创建数据库表。
代码修改逻辑：
- 在
AutoMigrate中添加new(model.WebDAVProperty) - 这会在数据库初始化时自动创建
webdav_properties表
合理性评估：
✅ 优点：
- 遵循项目现有的数据库迁移模式
- 自动化表结构管理，无需手动 SQL
- 这是一个破坏性变更，需要数据库迁移
- 现有用户升级时会自动创建新表，但 PR 描述中标记为"修改了存储格式"是正确的
- 建议在文档中说明升级注意事项
internal/model/webdav_property.go（新文件）
改动意图：
定义 WebDAV 死属性的数据库模型，用于持久化用户自定义的 WebDAV 属性。
代码修改逻辑：
type WebDAVProperty struct {
ID uint `gorm:"primaryKey"`
Path string `gorm:"index:idx_path_ns_name,priority:1"`
Namespace string `gorm:"index:idx_path_ns_name,priority:2"`
Name string `gorm:"index:idx_path_ns_name,priority:3"`
Lang string
InnerXML []byte
}- 使用复合索引
(Path, Namespace, Name)确保查询性能 InnerXML存储属性的 XML 内容
合理性评估：
✅ 优点：
- 数据模型设计合理，索引策略正确
- 复合索引覆盖了主要查询场景（按路径查找属性）
- 字段命名清晰，符合 WebDAV 规范
💡 建议：
-
添加创建/更新时间字段：方便调试和数据维护
CreatedAt time.Time UpdatedAt time.Time
-
考虑添加唯一约束：防止重复插入相同的属性
`gorm:"uniqueIndex:idx_path_ns_name"`
internal/fs/copy_move.go
改动意图：
支持 WebDAV COPY 操作中的"命名复制"功能，允许复制时指定目标文件名。
代码修改逻辑：
-
新增
dstName参数（第103行）：transfer函数新增dstName string参数- 允许在复制时指定目标文件名，而不是使用源文件名
-
同目录复制的重命名处理（第120-125行）：
if err == nil && dstName != "" { srcObjName := stdpath.Base(srcObjActualPath) if srcObjName != dstName { err = op.Rename(ctx, srcStorage, stdpath.Join(dstDirActualPath, srcObjName), dstName) } }
- 当驱动支持原生 Copy 且
dstName不为空时，先复制，再重命名
- 当驱动支持原生 Copy 且
-
目录复制时的命名处理（第199-205行）：
- 如果指定了
dstName，覆盖默认的源文件名 - 创建目标目录时使用新名称
- 如果指定了
-
文件流复制时的命名（第265-268行）：
streamObj := srcObj if t.DstName != "" { streamObj = &model.ObjWrapName{Name: t.DstName, Obj: srcObj} }
- 使用包装对象替换文件名
合理性评估：
✅ 优点：
- 实现了 WebDAV COPY 的"命名复制"语义
- 支持驱动原生 Copy + Rename 的优化路径
- 兼容了目录和文件两种场景
-
重命名失败的回滚：第120-125行的逻辑中，如果 Copy 成功但 Rename 失败，会留下一个错误名称的文件，没有回滚机制。建议添加：
if err == nil && dstName != "" { srcObjName := stdpath.Base(srcObjActualPath) if srcObjName != dstName { copiedPath := stdpath.Join(dstDirActualPath, srcObjName) err = op.Rename(ctx, srcStorage, copiedPath, dstName) if err != nil { // 回滚：删除已复制的文件 _ = op.Remove(ctx, srcStorage, copiedPath) } } }
-
目录复制的提前创建（第203-206行）：
if err := op.MakeDir(t.Ctx(), t.DstStorage, dstActualPath); err != nil { return errors.WithMessagef(err, "failed create dst dir [%s]", dstActualPath) }
- 这个逻辑会提前创建目标目录，即使后续复制失败，目录也会留下
- 原有代码可能依赖"目录不存在则创建"的隐式逻辑，这里变成了显式创建，需要确认是否会影响其他场景
internal/fs/fs.go
改动意图：
新增 CopyTo 函数，支持复制时指定目标文件名。
代码修改逻辑：
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
}合理性评估：
✅ 优点：
- API 设计清晰，与现有
Copy函数保持一致 - 日志记录包含目标文件名，便于调试
💡 建议：
- 添加参数校验：
dstName不应包含路径分隔符if strings.Contains(dstName, "/") || strings.Contains(dstName, "\\") { return nil, errors.New("dstName must not contain path separators") }
internal/fs/other.go
改动意图：
在任务数据结构中新增 DstName 字段，用于支持命名复制功能。
代码修改逻辑：
type TaskData struct {
...
DstName string `json:"dst_name,omitempty"`
...
}合理性评估：
✅ 优点：
- 使用
omitempty标签，不会影响现有任务的 JSON 序列化 - 向后兼容，旧任务反序列化时
DstName为空字符串
internal/op/fs.go
改动意图：
修正日志格式字符串，将 %+s 改为 %+v。
代码修改逻辑：
- log.Debugf("remove %s: %+s", storage.GetStorage().MountPath+actualPath, err)
+ log.Debugf("remove %s: %+v", storage.GetStorage().MountPath+actualPath, err)合理性评估：
✅ 优点：
- 修复了格式化字符串的错误用法（
%+s不是有效的格式符） %+v可以打印错误的堆栈信息（如果使用了pkg/errors）
💡 建议：
- 这是一个独立的 bug 修复，与 WebDAV 功能无关，可以考虑单独提 PR，便于追溯
server/webdav/file.go
改动意图：
实现 WebDAV COPY 操作的命名复制功能，并修复 MOVE 时的死属性迁移。
代码修改逻辑：
-
COPY 操作支持 Destination 路径提取（第128-148行）：
- 解析
Destination头，支持绝对 URL 和相对路径 - 提取目标目录和目标文件名
- 判断源路径是否为目录
- 解析
-
调用
fs.CopyTo或fs.Copy（第150-174行）：if !srcIsDir && dstName != "" && dstName != stdpath.Base(reqPath) { _, err = fs.CopyTo(ctx, reqPath, dstDir, dstName) } else { _, err = fs.Copy(ctx, reqPath, dstDir) }
- 如果是文件且目标名称与源名称不同，使用
CopyTo - 否则使用传统的
Copy
- 如果是文件且目标名称与源名称不同，使用
-
MOVE 操作迁移死属性（第215行）：
if err := moveDeadProps(reqPath, dst); err != nil { return http.StatusInternalServerError, err }
合理性评估：
✅ 优点：
- 实现了 RFC 4918 规定的 COPY 语义
- 支持同目录复制（通过不同文件名区分）
- MOVE 时自动迁移自定义属性，符合用户预期
-
目标目录检查逻辑（第161-164行）：
dstDirObj, err := op.Get(ctx, user, dstDir) if err != nil || !dstDirObj.IsDir() { return http.StatusConflict, nil }
- 如果目标目录不存在，返回
409 Conflict是正确的 - 但如果
op.Get返回其他错误（如权限错误），也会返回 409，错误信息丢失 - 建议区分处理：
dstDirObj, err := op.Get(ctx, user, dstDir) if err != nil { if errors.Is(err, errs.ObjectNotFound) { return http.StatusConflict, nil } return http.StatusInternalServerError, err } if !dstDirObj.IsDir() { return http.StatusConflict, nil }
- 如果目标目录不存在，返回
-
COPY 失败时的死属性清理：
- 如果
fs.CopyTo成功但后续操作失败，死属性不会被清理 - 建议在 COPY 成功后再处理死属性（虽然当前实现中死属性是在
fs.CopyTo之外处理的，但需要确认是否有其他隐含逻辑）
- 如果
server/webdav/lock.go
改动意图：
修复锁路径解析错误，支持共享锁（shared locks）。
代码修改逻辑：
-
修复
lookup函数的变量作用域（第53行）：- func (m *memLS) lookup(name string, conditions ...Condition) (n *memLSNode) { + func (m *memLS) lookup(name string, conditions ...Condition) *memLSNode { for _, c := range conditions { - n = m.byToken[c.Token] + n := m.byToken[c.Token]
- 原 bug：
n作为返回值参数，循环中每次赋值会覆盖，最后一次循环的n会被返回，即使前面有匹配项 - 修复：将
n改为局部变量，每次循环重新声明
- 原 bug：
-
支持共享锁创建（第235-241行）：
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 } }
- 如果已存在相同路径的共享锁，生成新 token 并加入
sharedTokens集合 - 多个客户端可以同时持有共享锁
- 如果已存在相同路径的共享锁，生成新 token 并加入
-
共享锁的解锁逻辑（第295-302行）：
if n.details.Shared { delete(m.byToken, token) delete(n.sharedTokens, token) if len(n.sharedTokens) != 0 { return nil } }
- 只删除当前 token，不删除整个锁节点
- 只有当所有共享 token 都被释放后，才删除锁节点
-
锁节点删除时清理所有共享 token（第352-358行）：
if n.details.Shared { for token := range n.sharedTokens { delete(m.byToken, token) } } else { delete(m.byToken, n.token) }
合理性评估：
✅ 优点：
- 修复了严重的锁查找 bug：原
lookup函数的逻辑错误会导致锁机制完全失效 - 共享锁实现符合 WebDAV 规范（RFC 4918）
- 数据结构设计合理，使用
map[string]struct{}存储共享 token 集合，内存高效
💡 建议：
-
添加共享锁的冲突检测：当创建独占锁时，应检查是否存在共享锁，反之亦然
if !details.Shared { // 创建独占锁前，检查是否存在共享锁 if n := m.byName[details.Root]; n != nil && n.details.Shared { return "", ErrLocked } }
-
共享锁的深度一致性检查：当前代码要求
ZeroDepth必须一致，这是正确的，但可以添加注释说明原因
server/webdav/prop.go
改动意图：
实现死属性的持久化，支持 PROPPATCH 设置、PROPFIND 检索和 MOVE 迁移。
代码修改逻辑：
-
新增
getDeadProps函数（第291-306行）：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{...} } return props, nil }
- 从数据库读取指定路径的所有死属性
-
重构
props函数（第175行）：- 新增
name参数，用于查询死属性 - 调用
getDeadProps获取持久化的属性
- 新增
-
重构
propnames和allprop函数：- 同样添加
name参数，支持死属性查询
- 同样添加
-
重构
patch函数（第273-309行）：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{...} if err := tx.Where(...).Assign(...).FirstOrCreate(&row).Error; err != nil { return err } } pstat.Props = append(pstat.Props, Property{XMLName: prop.XMLName}) } } return nil })
- 使用数据库事务确保原子性
FirstOrCreate+Assign实现 UPSERT 语义- 添加重试机制处理 SQLite 的"database is locked"错误（最多重试10次）
-
新增
moveDeadProps函数（第349-381行）：func moveDeadProps(src, dst string) error { // 清理目标路径的旧属性（覆盖语义） 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 ? ESCAPE '\\'", src, escapedSrc+"/%").Find(&rows).Error; err != nil { return err } // 复制到新路径，删除旧记录 for _, row := range rows { newPath := dst + strings.TrimPrefix(row.Path, src) 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 }
- 使用
LIKE匹配子路径（支持目录 MOVE） - 使用
ESCAPE '\\'转义特殊字符（%,_,\）
- 使用
合理性评估：
✅ 优点：
- 实现了完整的死属性持久化机制
- 使用数据库事务确保数据一致性
- MOVE 时自动迁移属性，符合 WebDAV 规范
- 添加了 SQLite 锁重试机制，提升并发性能
-
LIKE查询的性能问题：moveDeadProps中的path LIKE 'src/%'查询在属性较多时性能较差- 建议添加索引优化：
CREATE INDEX idx_webdav_properties_path_prefix ON webdav_properties(path);
- 或者使用前缀索引（如果数据库支持）
-
转义逻辑的正确性（第355-356行）：
escapedSrc := strings.ReplaceAll(strings.ReplaceAll(strings.ReplaceAll(src, "\\", "\\\\"), "%", "\\%"), "_", "\\_")
- 转义顺序正确：先转义
\，再转义%和_ - 但这个逻辑只适用于 MySQL/PostgreSQL，SQLite 的 ESCAPE 语法不同
- SQLite 中，
ESCAPE '\'表示使用\作为转义字符，但\本身在字符串中也需要转义 - 建议测试 SQLite 环境下的 MOVE 操作，确保路径包含特殊字符时正常工作
- 转义顺序正确：先转义
-
MOVE 失败的回滚：
- 如果
moveDeadProps在中途失败（如创建新记录失败），已复制的属性不会回滚 - 虽然使用了事务，但如果外层调用没有正确处理错误，可能导致部分属性丢失
- 建议在
file.go中确保 MOVE 失败时回滚文件系统操作和属性操作
- 如果
-
死属性的并发写入：
- 重试机制只处理了 "database is locked" 错误
- 如果多个客户端同时修改同一属性，可能导致丢失更新
- 建议添加乐观锁或版本号机制
server/webdav/webdav.go
改动意图：
修复锁检查的路径解析逻辑，确保锁机制在用户路径解析后的实际资源路径上生效。
代码修改逻辑：
-
移除 PROPFIND 的自动降级逻辑（第85-88行）：
- case "PROPFIND": - status, err = h.handlePropfind(brw, r) - if err != nil { - status = http.StatusNotFound - } + case "PROPFIND": + status, err = h.handlePropfind(brw, r)
- 原逻辑问题：任何 PROPFIND 错误都被转换为 404，隐藏了真实错误
- 修复：返回真实的错误状态码（如 403 Forbidden, 500 Internal Server Error）
-
锁条件中的路径解析（第168-175行）：
if user != nil { lsrc, err = user.JoinPath(lsrc) if err != nil { return nil, http.StatusForbidden, err } }
- 将
If头中的资源标签（resource tag）通过user.JoinPath解析为实际路径 - 修复了锁路径解析错误的核心 bug
- 将
-
调整锁检查与路径解析的顺序：
- 在 DELETE/PUT/MKCOL/PROPPATCH 中，将
user.JoinPath调用移到confirmLocks之前 - 确保锁检查使用的是解析后的实际路径
- 在 DELETE/PUT/MKCOL/PROPPATCH 中，将
合理性评估：
✅ 优点：
- 修复了锁机制的核心 bug：原来锁检查使用的是用户请求路径，而不是实际资源路径，导致锁完全失效
- PROPFIND 错误处理更合理，不会隐藏真实错误
- 删除了冗余的注释，代码更简洁
-
路径解析顺序的影响：
- 将
user.JoinPath提前到confirmLocks之前，意味着锁检查使用的是解析后的路径 - 但
confirmLocks内部也会调用user.JoinPath（第168-175行） - 这是否会导致二次解析？建议检查
confirmLocks的调用链，确保不会重复解析
- 将
-
错误状态码的变化：
- 移除 PROPFIND 的 404 降级后，客户端收到的错误码会改变
- 需要确认这是否会影响现有客户端的兼容性（虽然返回真实错误码是正确的）
server/webdav/xml.go
改动意图：
- 支持共享锁的 XML 序列化
- 拒绝含非法 XML 命名空间声明的 PROPFIND 请求体
- 限制 PROPFIND 请求体大小，防止 DoS 攻击
代码修改逻辑：
-
共享锁的 XML 输出（第87-90行）：
scope := "exclusive" if ld.Shared { scope = "shared" }
- 根据锁类型输出
<D:exclusive/>或<D:shared/>
- 根据锁类型输出
-
修复
readLockInfo的锁类型检查（第65行）：- if li.Exclusive == nil || li.Shared != nil || li.Write == nil { + if (li.Exclusive == nil) == (li.Shared == nil) || li.Write == nil {
- 原逻辑：只允许独占写锁
- 新逻辑：允许独占锁或共享锁，但不能同时存在，且必须是写锁
- 使用 XOR 逻辑：
(A == nil) == (B == nil)等价于(A != nil) XOR (B != nil)
-
PROPFIND 请求体大小限制（第181-195行）：
const maxBody = 64 << 10 // 64KB body, err := io.ReadAll(io.LimitReader(r, maxBody)) if err != nil { return propfind{}, http.StatusBadRequest, err } if len(body) >= maxBody { _, _ = io.Copy(io.Discard, r) return propfind{}, http.StatusRequestEntityTooLarge, nil }
- 限制请求体最大 64KB
- 如果超限，返回
413 Request Entity Too Large - 排空剩余数据，保持连接可用
-
检查空命名空间前缀（第221-248行）：
func hasEmptyNamespacePrefix(body []byte) bool { // 查找 xmlns:="" 这样的非法声明 for offset := 0; ; { i := bytes.Index(body[offset:], []byte("xmlns:")) if i < 0 { return false } // 解析 xmlns:prefix="value" // 如果 prefix 为空，返回 true } }
- 防止客户端发送
xmlns:=""这样的非法 XML - 这种声明会导致 XML 解析器错误
- 防止客户端发送
合理性评估：
✅ 优点：
- 共享锁的 XML 输出符合 RFC 4918 规范
- 请求体大小限制有效防止 DoS 攻击
- 拒绝非法 XML 提升了安全性和鲁棒性
-
hasEmptyNamespacePrefix的解析逻辑：- 手写的 XML 解析器容易出错，建议添加单元测试覆盖各种边界情况
- 测试用例：
xmlns:="" <!-- 应检测 --> xmlns: ="" <!-- 应检测 --> xmlns:foo="bar" <!-- 不应检测 --> xmlns:="value" <!-- 应检测 -->
-
64KB 限制是否合理：
- 对于大型目录的 PROPFIND allprop 请求，响应可能很大，但请求体通常很小
- 64KB 对于请求体来说已经非常宽松，应该足够
- 但建议在文档中说明这个限制
🎯 总体评价
功能性：⭐⭐⭐⭐⭐ - 修复了 WebDAV 的核心功能缺陷，大幅提升标准兼容性
安全性：⭐⭐⭐⭐ - 添加了请求体大小限制和 XML 校验，但死属性的并发安全性需加强
代码质量：⭐⭐⭐⭐ - 代码结构清晰，但部分错误处理和回滚机制需完善
实现方案：⭐⭐⭐⭐⭐ - 技术方案合理，符合 WebDAV 规范，测试覆盖充分
建议操作：
- ✅ Approve（建议合并）
理由：
这是一个非常重要的功能性 PR，修复了 WebDAV 服务的多个严重问题：
-
问题定位准确：
- 准确识别了 COPY 500 错误、死属性无法持久化、锁机制失效等问题的根本原因
- 锁路径解析的 bug 修复尤其关键，直接影响 WebDAV 的安全性
-
功能完整性大幅提升：
- Litmus 测试套件通过率从 61/80 提升到 102/103（仅剩 1 个上游未实现的特性）
- copymove: 9/13 → 13/13
- props: 9/14 → 30/30
- locks: 24/34 → 40/41
- 这些数字证明了修复的有效性
-
技术方案合理：
- 死属性持久化使用数据库，设计合理
- 共享锁实现符合 RFC 4918 规范
- 命名复制的实现支持了 WebDAV 的高级特性
-
测试覆盖充分：
- 通过 Litmus 全量测试套件验证
- 在公网 Nginx HTTPS 环境下测试，接近真实场景
需要注意的改进点（非阻塞，但建议后续优化）：
-
错误处理和回滚机制：
- 复制+重命名失败时的回滚
- MOVE 失败时的属性回滚
- 错误状态码的精确区分（如 op.Get 失败时不应都返回 409）
-
性能优化：
moveDeadProps的 LIKE 查询需要索引优化- 死属性的并发写入冲突处理
-
文档和测试：
- 添加数据库迁移说明
hasEmptyNamespacePrefix需要单元测试- 参数校验（如 dstName 不应包含路径分隔符）
-
跨平台兼容性：
- 确认 SQLite 的 ESCAPE 语法是否正确工作
- 测试路径包含特殊字符时的 MOVE 操作
前置条件：
- 用户升级时会自动创建
webdav_properties表 - 建议在发布说明中标注这是数据库结构变更
- 建议手动测试 SQLite 环境下的所有修复功能
总体而言，这是一个高质量且重要的 PR，强烈建议合并 ✅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary / 摘要
修复 WebDAV
COPY方法稳定返回500 Internal Server Error的问题，同时修复了 COPY/MOVE 状态码语义、死属性持久化、锁路径解析和共享锁支持。COPY 同目录/跨目录、绝对/相对 Destination 均正常工作，覆盖语义符合 RFC 4918
PROPPATCH设置的自定义属性可持久化，PROPFIND可检索，MOVE时自动迁移锁检查使用用户路径解析后的实际资源路径，支持共享锁
拒绝含非法 XML 声明的 PROPFIND 请求体
This PR has breaking changes.
/ 此 PR 包含破坏性变更。
This PR changes public API, config, storage format, or migration behavior.
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
This PR requires corresponding changes in related repositories.
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Related Issues / 关联 Issue
Testing / 测试
go test ./server/webdav ./internal/fs其中
locks.fail_complex_cond_put未通过Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。