Skip to content

fix(ApiService): Catch NotPermittedException and return 404 #6019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mejo- merged 2 commits into from
Jul 9, 2024
Merged

fix(ApiService): Catch NotPermittedException and return 404 #6019

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74aa530
chore(ApiService): Use constants for http status codes everywhere
mejo- Jul 9, 2024
e178b63
fix(ApiService): Catch NotPermittedException and return 404
mejo- Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
chore(ApiService): Use constants for http status codes everywhere 
Signed-off-by: Jonas <[email protected]>
  • Loading branch information
@mejo-
mejo- committed Jul 9, 2024
commit 74aa530fb4ca922d3577b64cc01676d28c449988
24 changes: 12 additions & 12 deletions lib/Service/ApiService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ 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(['error' => $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')], Http::STATUS_NOT_FOUND);
}
} elseif ($fileId !== null) {
try {
Expand All @@ -79,7 +79,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(['error' => $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')], Http::STATUS_FORBIDDEN);
}
}

Expand All @@ -105,7 +105,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(['error' => 'Failed to create the document session'], 500);
return new DataResponse(['error' => 'Failed to create the document session'], Http::STATUS_INTERNAL_SERVER_ERROR);
}

/** @var Document $document */
Expand Down Expand Up @@ -180,7 +180,7 @@ public function push(Session $session, Document $document, int $version, array $
try {
$result = $this->documentService->addStep($document, $session, $steps, $version, $token);
} catch (InvalidArgumentException $e) {
return new DataResponse(['error' => $e->getMessage()], 422);
return new DataResponse(['error' => $e->getMessage()], Http::STATUS_UNPROCESSABLE_ENTITY);
} catch (DoesNotExistException|NotPermittedException) {
// Either no write access or session was removed in the meantime (#3875).
return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED);
Expand All @@ -205,12 +205,12 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'File not found'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DoesNotExistException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Document no longer exists'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DocumentSaveConflictException) {
try {
/** @psalm-suppress PossiblyUndefinedVariable */
Expand All @@ -220,7 +220,7 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
}
}

return new DataResponse($result, isset($result['outsideChange']) ? 409 : 200);
return new DataResponse($result, isset($result['outsideChange']) ? Http::STATUS_CONFLICT : Http::STATUS_OK);
}

public function save(Session $session, Document $document, int $version = 0, ?string $autosaveContent = null, ?string $documentState = null, bool $force = false, bool $manualSave = false, ?string $shareToken = null): DataResponse {
Expand All @@ -230,12 +230,12 @@ public function save(Session $session, Document $document, int $version = 0, ?st
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'File not found'
], 404);
], Http::STATUS_NOT_FOUND);
} catch (DoesNotExistException $e) {
$this->logger->info($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Document no longer exists'
], 404);
], Http::STATUS_NOT_FOUND);
}

$result = [];
Expand All @@ -248,15 +248,15 @@ public function save(Session $session, Document $document, int $version = 0, ?st
// Ignore locked exception since it might happen due to an autosave action happening at the same time
}
} catch (NotFoundException) {
return new DataResponse([], 404);
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
return new DataResponse([
'message' => 'Failed to autosave document'
], 500);
], Http::STATUS_INTERNAL_SERVER_ERROR);
}

return new DataResponse($result, isset($result['outsideChange']) ? 409 : 200);
return new DataResponse($result, isset($result['outsideChange']) ? Http::STATUS_CONFLICT : Http::STATUS_OK);
}

public function updateSession(Session $session, string $guestName): DataResponse {
Expand Down
1 change: 1 addition & 0 deletions lib/Service/DocumentService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ public function getAll(): \Generator {
}

/**
* @throws NotPermittedException
* @throws NotFoundException
*/
public function getFileForSession(Session $session, ?string $shareToken = null): File {
Expand Down