Skip to content

Commit 9d3f4d7

Browse files
timarbackportbot[bot]
authored andcommitted
fix(wopi): report a locked file as 423, not 500
When putContent() throws LockedException the file is locked by another operation and we have written nothing. Answering 500 says the server is broken, which it isn't, and it buries a routine, self-resolving condition in the error logs. It also misleads the editor. Collabora Online cannot tell a genuine fault from a refusal, so after the failed upload it asks for CheckFileInfo to work out whether the document in storage is still the one it knows. A 423 lets it see the upload for what it was and retry once the lock clears. 423 is what this controller already answers with when the lock manager reports a file locked in lock() and refreshLock(); use it on the write path too. Do not use 409: in WOPI that means the document changed in storage, and Collabora Online reads it that way. It would put a conflict dialog in front of the user, asking them to discard their work or overwrite a file that nobody has touched. Signed-off-by: Andras Timar <andras.timar@collabora.com>
1 parent e36a132 commit 9d3f4d7

1 file changed

Lines changed: 6 additions & 2 deletions

File tree

lib/Controller/WopiController.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,10 @@ public function putFile(string $fileId, string $access_token): JSONResponse {
675675
$this->wrappedFilesystemOperation($wopi, fn () => $file->putContent($content));
676676
} catch (LockedException $e) {
677677
$this->logger->error($e->getMessage(), ['exception' => $e]);
678-
return new JSONResponse(['message' => 'File locked'], Http::STATUS_INTERNAL_SERVER_ERROR);
678+
// The file is locked by another operation and we wrote nothing.
679+
// Report it as such, so the client can retry rather than treat
680+
// this as a server fault or as a change behind its back.
681+
return new JSONResponse(['message' => 'File locked'], Http::STATUS_LOCKED);
679682
}
680683

681684
if ($isPutRelative) {
@@ -819,7 +822,8 @@ public function postFile(string $fileId, string $access_token): JSONResponse {
819822
try {
820823
$this->wrappedFilesystemOperation($wopi, fn () => $file->putContent($content));
821824
} catch (LockedException) {
822-
return new JSONResponse(['message' => 'File locked'], Http::STATUS_INTERNAL_SERVER_ERROR);
825+
// As in putFile(): nothing was written, so this is not a server fault.
826+
return new JSONResponse(['message' => 'File locked'], Http::STATUS_LOCKED);
823827
}
824828

825829
// epub is exception (can be uploaded but not opened so don't try to get access token)

0 commit comments

Comments
 (0)