[Bug]: Moving a folder to SMB external storage fails half-way (HTTP 500) if a file inside has a name SMB rejects; partial copy is left behind #65715

⚠️ This issue respects the following points: ⚠️

• This is not a troubleshooting question, general support matter, or webserver/proxy problem, but likely a bug (if unsure, ask the Community Help Forum).
• This issue is not already reported on Github OR solved at the Community Help Forum (I've searched!).
• I'm using a maintained major version of Nextcloud Server and tested against the latest patch level. (Supported major versions and current patch levels).
• I agree to follow Nextcloud's Code of Conduct.
• I've tried my best to provide clear reproduction steps that someone unfamiliar with this bug could use to reproduce it.

Bug description

When a folder is moved from local storage to an SMB external storage and a file inside it has a name that icewind/smb rejects (? < > : * | ", e.g. Talk 14:15.mp4), the request fails with HTTP 500 in the middle of the copy:

• everything copied before that file stays on the target,
• the source is left untouched,
• the web UI shows a generic error. It does not say which file caused it, and offers no way to skip or rename the file.

Retrying the move raises a conflict for the folder. Folders cannot be merged (#33719), so the only way forward is to replace the target folder. That deletes the partial copy, and also any other content of the target folder (e.g. files added directly on the SMB share). Then the move fails again at the same file.

Steps to reproduce

1. Configure an SMB external storage.
2. In local storage, create a folder Event with a.mp4, b 14:15.mp4 and c.mp4.
3. In the Files app, move Event into the SMB mount.
4. HTTP 500. On the share, Event/ contains the files copied before b 14:15.mp4 (which ones depends on the readdir order). The local Event/ is unchanged.

Expected behavior

The move is rejected before anything is copied, with a 400 that names the offending file (just like an upload of that name to the SMB storage), or the file is skipped and reported. No half-moved tree and no 500.

Analysis

Each failing request logs two exceptions (paths shortened):

warning  Failed to copy stream to storage
  Icewind\SMB\Exception\InvalidPathException: Invalid path "/Event/b 14:15.mp4", ":" is not allowed
  AbstractShare::verifyPath <- NativeShare::buildUrl <- NativeShare::write <- SMB::fopen
  <- Common::writeStream <- Common::copyFromStorage <- Common::copyFromStorage <- Common::moveFromStorage
  <- ... <- View::rename <- Directory::moveInto

error    Uncaught exception (HTTP 500)
  Icewind\SMB\Exception\InvalidPathException: same path
  AbstractShare::verifyPath <- NativeShare::buildUrl <- NativeShare::rawStat <- NativeFileInfo::stat
  <- NativeShare::stat <- SMB::getFileInfo <- SMB::filetype <- Common::is_dir <- SMB::unlink
  <- Common::copyFromStorage <- Common::copyFromStorage <- ...

1. Common::copyFromStorage(): the failed writeStream() is caught and logged. The cleanup ($this->unlink($targetInternalPath)) is not guarded.
2. SMB: unlink() → is_dir() → filetype() → getFileInfo(). None of them handle Icewind\SMB\Exception\InvalidPathException. Only NotFound, Forbidden, Connect and InvalidArgument are mapped. The cleanup throws, and the exception leaves the recursive copyFromStorage() in the middle of the traversal.
3. SMB has no verifyPath() override. View::rename() and Directory::moveInto() only validate the top-level target name, and only against the global filename validator. No code checks the nested names against the target storage before the copy starts.

Possible fix

• SMB::verifyPath(): reject the characters icewind/smb refuses by throwing InvalidCharacterInPathException. Direct uploads and renames on SMB would then get a clean 400 as well.
• Cross-storage View::rename()/copy() of a directory: verify every name in the source tree against the target storage before calling moveFromStorage()/copyFromStorage(). Abort with an InvalidPathException that names the file. Directory::moveInto() maps that to InvalidPath (400) instead of a 500.
• Defense in depth: in Common::copyFromStorage(), wrap the cleanup unlink() in try/catch and only log. Map Icewind\SMB\Exception\InvalidPathException in SMB::getFileInfo() to NotFoundException (a path the share cannot represent cannot exist there).

Happy to provide a PR if this direction is fine.

Nextcloud Server version

34

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

• Default user-backend (database)
• LDAP/ Active Directory
• SSO - SAML
• Other

Configuration report

List of activated Apps

Nextcloud Signing status

Nextcloud Logs

Additional info

No response