Skip to content

fix(drivers): keep existing files when uploads fail or are canceled (smb, chunk, op) - #3330

Open
thetapilla wants to merge 4 commits into
OpenListTeam:mainfrom
thetapilla:fix/upload-cancel-safety
Open

fix(drivers): keep existing files when uploads fail or are canceled (smb, chunk, op)#3330
thetapilla wants to merge 4 commits into
OpenListTeam:mainfrom
thetapilla:fix/upload-cancel-safety

Conversation

@thetapilla

@thetapilla thetapilla commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Summary / 摘要

#3178 的根因是：上传被取消时（客户端断开，或分片会话被中止、取代、回收），驱动按路径删除目标文件，而这时这个路径可能已经属于另一次写完的上传。#3305 预期将修复 Local 驱动；本 PR 修复 SMB、Chunk 驱动和 op.Put 中的同类问题，以及 SMB 在无法替换文件的服务端上覆盖失败时毁掉原文件的问题。每处一个 commit。

用户可感知的变化：

  • SMB：被取消的上传不再删掉同名的、已经写完的另一次上传（已在 Samba 4.23 上复现）；覆盖失败时旧版本保持完好；一个占着目标文件的空闲上传不再让后来的同名上传失败
  • SMB 服务端无法替换已存在的文件时（如在不支持 xattr 的文件系统上启用了 vfs_streams_xattr 的 Samba），覆盖上传被拒绝、原文件保留；此前服务端会先把原文件截成 0 字节再报错
  • Chunk：覆盖失败不再删掉旧版本；用更小的文件覆盖不再残留旧文件多出的分片（原来会导致新文件的大小和内容出错）
  • 设置了 NoOverwriteUpload 的 12 个存储（aliyundrive_open、chaoxing、cloudreve_v4、degoo、dropbox、guangyapan、ilanzou、mediafire、quark_open、quark_uc、quark_uc_tv、wopan）：覆盖上传被取消后，原文件不再滞留为 <名字>.openlist_to_delete

实现要点：

  • SMB 暂存后发布：先写到目标旁的唯一命名暂存文件，完成后改名覆盖目标（go-smb2 改名时带 ReplaceIfExists）；失败或取消只删除自己的暂存文件，不再按路径删除目标
  • SMB 能力探测：Init 后第一次上传时，在该上传的目录里探测服务端能否改名覆盖文件、能否原地覆盖文件。能改名覆盖 → 暂存发布；只能原地覆盖 → 原地写入，失败时不删除任何文件；两样都不行 → 只创建新文件（O_EXCL），覆盖在碰到原文件之前就拒绝，因为这类服务端拒绝覆盖前会先截断文件
  • Chunk 暂存目录：分片先写到唯一命名的暂存目录，完成后移到正式位置；旧版本到这时才挪开，之后删除，中途失败会回滚。不支持改名的远端继续原地写入，但失败时不再删除分片目录
  • op.Put 恢复原文件：失败后改回原名改用脱离上传的 context，限时 1 分钟；只在原名仍空闲时才改回。如果期间另一次上传占用了这个名字，就保留较新的文件、原文件留在一旁，因为部分驱动（如 aliyundrive_open）改名后会删除同名的"重复"文件
  • 暂存项命名：两个驱动的暂存项都命名为 .openlist-upload- 加 16 位随机字母数字，前缀与 fix(drivers/local): isolate canceled multipart uploads #3305 一致。List 只隐藏恰好符合该格式的条目，并回收超过 24 小时的残留（进程被杀时留下的）；用户自己以该前缀开头的文件照常显示，也不会被回收

兼容性说明：不改动驱动接口和设置项。覆盖期间 SMB 和 Chunk 需要同时放得下新旧两个版本；上传进行中，其他 SMB 客户端能看到隐藏的暂存文件。不能删除文件的 SMB 服务端上，每次 Init 后的探测会留下两个 0 字节的隐藏文件。

本 PR 未覆盖：SFTP、FTP 仍是原地写入（能否改名覆盖取决于服务端）；139（旧上传路径）和 Teldrive 在上传前就删除原文件，已在 #3329 中报告。

  • 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:

  • OpenList-Frontend: 无

Related Issues / 关联 Issue

Relates to #3178

Testing / 测试

测试平台：macOS arm64，Go 1.27.3；Samba 4.23（servercontainers/samba，分别为正常配置，以及 macOS 绑定挂载 + vfs_streams_xattr 的错误配置）；MinIO。

  • go test ./...
  • Manual test / 手动测试:

自动化测试：

  • go test -race -count=1 ./internal/op/ ./drivers/smb/ ./drivers/chunk/
  • SMB 实机测试（OPENLIST_SMB_INTEGRATION=1，见 drivers/smb/integration_test.go）：正常配置的 Samba 上 7/7 通过；错误配置的 Samba 上探测为只创建新文件，新文件照常上传，覆盖被拒绝且原文件完好（修复前原文件被截成 0 字节）
  • Chunk 的测试基于真实的 Local 远端（op.CreateStorage + 内存数据库）
  • 仓库外的黑盒用例：main 上 SMB 三项（被回收的上传删掉后来者、空闲上传阻塞后来者、覆盖失败毁掉旧版本）全部失败，本分支全部通过

浏览器实测：

  • 分片会话挂起期间，用流式上传同名文件，再中止挂起的会话：SMB 上后来的文件完好，只清掉了暂存文件（main 上后来的文件会被删）
  • 覆盖上传进行中：用户看到的列表里没有暂存文件，旧版本完整可读；完成后内容为新版本，无残留
  • 错误配置的 Samba 上覆盖：提示 the SMB server cannot replace existing files, so the existing file was left as it was，原文件完好

Checklist / 检查清单

  • I have read CONTRIBUTING.
    / 我已阅读 CONTRIBUTING。
  • I confirm this contribution follows the repository license, contribution policy, and code of conduct.
    / 我确认此贡献符合仓库许可证、贡献规范和行为准则。
  • I have formatted the changed code with gofmt, go fmt, or prettier where applicable.
    / 我已按适用情况使用 gofmt、go fmt 或 prettier 格式化变更代码。
  • I have requested review from relevant maintainers or code owners where applicable.
    / 我已在适用情况下请求相关维护者或代码所有者审查。

AI Disclosure / AI 使用声明

  • This PR includes AI-assisted content.
    / 此 PR 包含 AI 辅助内容。

Tools used / 使用工具:

  • ChatGPT
  • Codex
  • GitHub Copilot
  • Claude
  • Gemini
  • Other (please specify) / 其他（请注明）:

Usage scope / 使用范围:

  • Code generation / 代码生成

  • Refactoring / 重构

  • Documentation / 文档

  • Tests / 测试

  • Translation / 翻译

  • Review assistance / 审查辅助

  • I have reviewed and validated all AI-assisted content included in this PR.
    / 我已审核并验证此 PR 中的所有 AI 辅助内容。

  • I have ensured that all AI-assisted commits include Co-Authored-By attribution.
    / 我已确保所有 AI 辅助提交都包含 Co-Authored-By 归属信息。

  • I can reproduce all AI-assisted content included in this PR without any AI tools.
    / 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。

thetapilla and others added 4 commits October 30, 2026 14:56
@thetapilla @claude
fix(op): restore the original after a canceled no-overwrite upload
5b4b4b0 
- On storages with NoOverwriteUpload, Put renames the existing file to
  <name>.openlist_to_delete before uploading and renames it back if the
  upload fails. The rename-back used the upload's own context, so when the
  upload was canceled (client gone, multipart abort, supersede or expiry)
  it failed too and the original stayed hidden under the temporary name.
- Recover (and clean up after success) with a context detached from the
  upload, bounded to one minute.
- Only rename the original back while its name is still free. If another
  upload took the name meanwhile, keep the newer file and leave the
  original aside, since some drivers delete same-name "duplicates" after a
  rename.
- Add TestPutRestoresOriginalWhenCanceled and
  TestPutDoesNotRestoreOverNewerUpload.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(smb): stage uploads instead of deleting the destination on cancel
e1ee143 
- Put wrote the destination in place and removed it by path when its
  context was canceled. If a later upload of the same name had finished
  meanwhile (e.g. a multipart session reaped by GC, aborted or superseded),
  that file was deleted. A failed overwrite also destroyed the previous
  version, and an idle upload holding the destination open made a later
  upload of the same name fail.
- Write to a uniquely named staging file next to the destination and rename
  it over the destination once complete (go-smb2 renames with
  ReplaceIfExists). A failed or canceled upload only removes its own
  staging file.
- On the first upload after Init, probe whether the server can rename over
  an existing file. Servers that cannot (e.g. Samba with vfs_streams_xattr
  on a filesystem without xattr support) keep in-place writes, but no
  longer remove anything on failure.
- List hides staging files and reclaims those older than 24 hours.
- Add unit tests, and live tests gated by OPENLIST_SMB_INTEGRATION=1.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(chunk): stage uploads instead of removing the chunk folder on fai…
6c5c988 
…lure

- Put wrote the parts into the chunk folder in place, and when a part
  failed or the upload was canceled it removed the whole chunk folder by
  path. A failed overwrite thus deleted the previous version, and if
  another upload of the same name had completed meanwhile (e.g. while a
  multipart session waited to be reaped), that upload was deleted too.
  Overwriting with a smaller file also left the old file's extra parts
  behind, which corrupted the new file's size and content.
- Upload the parts into a uniquely named staging folder and move it into
  place once complete. The previous version is only moved aside then, and
  removed afterwards. A failed or canceled upload removes only its own
  staging folder.
- Remotes without Rename keep in-place writes, but no longer remove the
  chunk folder on failure.
- List hides staging folders and reclaims those older than 24 hours.
- Add tests against a Local remote.

Co-Authored-By: Claude <noreply@anthropic.com>
@thetapilla @claude
fix(smb): keep the previous version on servers that cannot replace files
957887e 
- Samba with vfs_streams_xattr on a filesystem without xattr support
  refuses to overwrite an existing file, but truncates it first. The
  in-place fallback therefore still destroyed the previous version on a
  failed overwrite.
- When renaming over a file fails, also probe whether the server can
  overwrite one in place. A server that can do neither only gets new
  files: they are created exclusively, and an overwrite is refused before
  the existing file is touched.
- Probe with fresh staging names: the fixed probe files a server could not
  remove made later probes fail before they learned anything.
- Add TestIntegrationOverwriteNeverDestroysPrevious.

Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Oct 30, 2026
Open
Open
jyxjjj
jyxjjj requested changes Nov 4, 2026
View reviewed changes

@jyxjjj jyxjjj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] SMB.List 和 Chunk.List 仅根据文件名和修改时间就执行 Remove。用户原本就能创建名为 .openlist-upload-0123456789abcdef 的文件或目录，超过 24 小时后，只要列一次目录就会被删除。Chunk 甚至在判断 IsDir 前就执行清理。请仅回收能确认由本次暂存机制创建的对象，并测试名称完全匹配的既有用户文件。

Chunk.publish 还会把旧版本改成同样格式的备份名。如果发布和恢复改名都失败，恢复错误被忽略，留下的旧版本也会进入上述清理路径。请保留无法恢复的备份并返回其位置。

审核提交 957887e。

Reviewed or refined by GPT-6-Astra High

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jyxjjj jyxjjj jyxjjj requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thetapilla @jyxjjj