Skip to content

Commit

Permalink
fix(response): Make sure JSONResponse returns valid data
Browse files Browse the repository at this point in the history
Wrap error messages into an array when responding with `JSONResponse`.

Signed-off-by: Jonas <[email protected]>
  • Loading branch information
mejo- committed Mar 27, 2024
1 parent 9126a73 commit 28dd585
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lib/Middleware/SessionMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ private function assertUserOrShareToken(ISessionAwareController $controller): vo

public function afterException($controller, $methodName, \Exception $exception): JSONResponse|Response {
if ($exception instanceof InvalidDocumentBaseVersionEtagException) {
return new JSONResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
}

if ($exception instanceof InvalidSessionException) {
Expand Down
18 changes: 9 additions & 9 deletions lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
} catch (NotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (NotPermittedException $e) {
return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 404);
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 404);
}
} elseif ($fileId) {
try {
$file = $this->documentService->getFileById($fileId);
} catch (NotFoundException|NotPermittedException $e) {
$this->logger->error('No permission to access this file', [ 'exception' => $e ]);
return new DataResponse($this->l10n->t('No permission to access this file.'), Http::STATUS_NOT_FOUND);
return new DataResponse(['error' => $this->l10n->t('No permission to access this file.')], Http::STATUS_NOT_FOUND);
}
} else {
return new DataResponse('No valid file argument provided', Http::STATUS_PRECONDITION_FAILED);
return new DataResponse(['error' => 'No valid file argument provided'], Http::STATUS_PRECONDITION_FAILED);
}

$storage = $file->getStorage();
Expand All @@ -106,7 +106,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
$share = $storage->getShare();
$shareAttribtues = $share->getAttributes();
if ($shareAttribtues !== null && $shareAttribtues->getAttribute('permissions', 'download') === false) {
return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 403);
return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 403);
}
}

Expand All @@ -116,7 +116,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
$document = $this->documentService->getDocument($file->getId());
$freshSession = $document === null;
if ($baseVersionEtag && $baseVersionEtag !== $document?->getBaseVersionEtag()) {
return new DataResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
}

if ($freshSession) {
Expand All @@ -132,7 +132,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
}
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return new DataResponse('Failed to create the document session', 500);
return new DataResponse(['error' => 'Failed to create the document session'], 500);
}

/** @var Document $document */
Expand Down Expand Up @@ -193,18 +193,18 @@ public function push(Session $session, Document $document, int $version, array $
$session = $this->sessionService->updateSessionAwareness($session, $awareness);
} catch (DoesNotExistException $e) {
// Session was removed in the meantime. #3875
return new DataResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
}
if (empty($steps)) {
return new DataResponse([]);
}
try {
$result = $this->documentService->addStep($document, $session, $steps, $version, $token);
} catch (InvalidArgumentException $e) {
return new DataResponse($e->getMessage(), 422);
return new DataResponse(['error' => $e->getMessage()], 422);
} catch (DoesNotExistException|NotPermittedException) {
// Either no write access or session was removed in the meantime (#3875).
return new DataResponse($this->l10n->t('Editing session has expired. Please reload the page.'), Http::STATUS_PRECONDITION_FAILED);
return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
}
return new DataResponse($result);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Editor/DocumentStatus.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<div class="document-status">
<NcNoteCard v-if="hasWarning" type="warning">
<p v-if="isLoadingError">
{{ syncError.data.data }}
{{ syncError.data.data.error }}
<!-- Display reload button on PRECONDITION_FAILED response type -->
<a v-if="syncError.data.status === 412" class="button primary" @click="reload">{{ t('text', 'Reload') }}</a>
</p>
Expand Down

0 comments on commit 28dd585

Please sign in to comment.