From 041918c8abaec9e4313d81104dc1d07b7594a321 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Tue, 7 Jul 2026 09:54:08 +0200 Subject: [PATCH] fix(dav): release part-file lock when an upload fails Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- apps/dav/lib/Connector/Sabre/Directory.php | 15 ++++--- .../unit/Connector/Sabre/DirectoryTest.php | 42 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index f4e8ee1d99a77..7ceecf731e453 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -123,11 +123,16 @@ public function createFile($name, $data = null) { $node->acquireLock(ILockingProvider::LOCK_SHARED); $this->fileView->lockFile($this->path . '/' . $name . '.upload.part', ILockingProvider::LOCK_EXCLUSIVE); - $result = $node->put($data); - - $this->fileView->unlockFile($this->path . '/' . $name . '.upload.part', ILockingProvider::LOCK_EXCLUSIVE); - $node->releaseLock(ILockingProvider::LOCK_SHARED); - return $result; + try { + return $node->put($data); + } finally { + // Always release the locks, even when the upload failed or was + // interrupted. Otherwise a failed attempt leaves the exclusive + // part-file lock behind and every later upload to the same path + // keeps getting rejected with 423 until the lock TTL expires. + $this->fileView->unlockFile($this->path . '/' . $name . '.upload.part', ILockingProvider::LOCK_EXCLUSIVE); + $node->releaseLock(ILockingProvider::LOCK_SHARED); + } } catch (StorageNotAvailableException $e) { throw new \Sabre\DAV\Exception\ServiceUnavailable($e->getMessage(), $e->getCode(), $e); } catch (InvalidPathException $ex) { diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php index 4144c79821ef5..c196627a81907 100644 --- a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php @@ -24,6 +24,7 @@ use OCP\Files\Mount\IMountPoint; use OCP\Files\Storage\IStorage; use OCP\Files\StorageNotAvailableException; +use OCP\Lock\ILockingProvider; use PHPUnit\Framework\MockObject\MockObject; use Sabre\DAV\Exception\NotFound; use Test\Traits\UserTrait; @@ -178,6 +179,47 @@ public function testDeleteFolderThrowsWhenDeletionFailed(): void { $dir->delete(); } + /** + * A failed or interrupted upload must not leave the exclusive part-file + * lock behind. Otherwise every later upload to the same path keeps getting + * rejected with "423 Locked" until the lock TTL expires (up to an hour), + * createFile() therefore releases the locks in a finally block. + */ + public function testCreateFileReleasesPartFileLockOnFailure(): void { + $name = 'foo.txt'; + + $this->view->method('getRelativePath')->willReturnArgument(0); + $this->view->method('getAbsolutePath')->willReturnArgument(0); + $this->view->method('isCreatable')->willReturn(true); + // the target does not exist yet + $this->view->method('getFileInfo')->willReturn(false); + // make File::put() fail right after the locks have been acquired + $this->view->method('resolvePath')->willReturn([null, null]); + + $released = []; + $this->view->method('unlockFile') + ->willReturnCallback(function (string $path, int $type) use (&$released): bool { + $released[] = [$path, $type]; + return true; + }); + + $dir = new Directory($this->view, $this->info); + $partLockPath = $dir->getPath() . '/' . $name . '.upload.part'; + + try { + $dir->createFile($name, 'test data'); + $this->fail('Expected the failing upload to throw'); + } catch (\Sabre\DAV\Exception\ServiceUnavailable) { + // expected: File::put() cannot resolve the storage + } + + $this->assertContains( + [$partLockPath, ILockingProvider::LOCK_EXCLUSIVE], + $released, + 'The exclusive .upload.part lock must be released after a failed upload', + ); + } + public function testGetChildren(): void { $info1 = $this->createMock(FileInfo::class); $info2 = $this->createMock(FileInfo::class);