Skip to content

Commit ac2dfef

Browse files
refactor(ocs): clarify v1 request flow and exception handling
Signed-off-by: Josh <josh.t.richards@gmail.com>
1 parent 7f7d5fe commit ac2dfef

1 file changed

Lines changed: 49 additions & 36 deletions

File tree

ocs/v1.php

Lines changed: 49 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -28,60 +28,73 @@
2828
use Symfony\Component\Routing\Exception\MethodNotAllowedException;
2929
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
3030

31-
$request = Server::get(IRequest::class);
31+
try {
32+
$logger = Server::get(LoggerInterface::class);
33+
$debug = Server::get(SystemConfig::class)->getValue('debug', false);
34+
$request = Server::get(IRequest::class);
35+
$requestPath = $request->getPathInfo();
36+
$rawRequestPath = $request->getRawPathInfo();
37+
$isCoreUpdateRequest = $requestPath === '/core/update';
38+
$upgrade = Util::needUpgrade();
39+
$maintenance = Server::get(IConfig::class)->getSystemValueBool('maintenance');
3240

33-
if ((Util::needUpgrade() || Server::get(IConfig::class)->getSystemValueBool('maintenance')) && $request->getPathInfo() !== '/core/update') {
34-
// since the behavior of apps or remotes are unpredictable during
35-
// an upgrade, return a 503 directly
36-
ApiHelper::respond(503, 'Service unavailable', ['X-Nextcloud-Maintenance-Mode' => '1'], 503);
37-
exit;
38-
}
41+
// During maintenance mode or while an upgrade is pending, return a 503 for OCS
42+
// requests directly. The core update endpoint is the only exception.
43+
if (($upgrade || $maintenance) && !$isCoreUpdateRequest) {
44+
ApiHelper::respond(503, 'Service unavailable', ['X-Nextcloud-Maintenance-Mode' => '1'], 503);
45+
exit();
46+
}
3947

40-
/*
41-
* Try the appframework routes
42-
*/
43-
try {
4448
$appManager = Server::get(IAppManager::class);
49+
// Load the baseline apps needed before route dispatch and authentication.
4550
$appManager->loadApps(['session']);
46-
$appManager->loadApps(['authentication']);
47-
$appManager->loadApps(['extended_authentication']);
51+
$appManager->loadApps(['authentication', 'extended_authentication']);
4852

53+
// Reject malformed request payloads before continuing.
4954
$request->throwDecodingExceptionIfAny();
5055

51-
if ($request->getPathInfo() !== '/core/update') {
52-
// load all apps to get all api routes properly setup
56+
$loggedIn = Server::get(IUserSession::class)->isLoggedIn();
57+
58+
if ($isCoreUpdateRequest) {
59+
// The update endpoint only needs the core app.
60+
$appManager->loadApps(['core']);
61+
} else {
62+
// Load all apps so that all OCS API routes are registered.
5363
// FIXME: this should ideally appear after handleLogin but will cause
5464
// side effects in existing apps
5565
$appManager->loadApps();
56-
if (!Server::get(IUserSession::class)->isLoggedIn()) {
66+
67+
// Attempt login only if there is no active session yet.
68+
if (!$loggedIn) {
5769
OC::handleLogin($request);
5870
}
59-
} else {
60-
$appManager->loadApps(['core']);
6171
}
6272

63-
Server::get(Router::class)->match('/ocsapp' . $request->getRawPathInfo());
73+
// OCS routes are registered under the /ocsapp prefix internally.
74+
Server::get(Router::class)->match('/ocsapp' . $rawRequestPath);
75+
} catch (LoginException $ex) {
76+
// Expected client-side failure; return an appropriate response without logging.
77+
ApiHelper::respond(OCSController::RESPOND_UNAUTHORISED, 'Unauthorised');
6478
} catch (MaxDelayReached $ex) {
79+
// Expected client-side failure; return an appropriate response without logging.
6580
ApiHelper::respond(Http::STATUS_TOO_MANY_REQUESTS, $ex->getMessage());
66-
} catch (ResourceNotFoundException $e) {
67-
$txt = 'Invalid query, please check the syntax. API specifications are here:'
68-
. ' http://www.freedesktop.org/wiki/Specifications/open-collaboration-services.' . "\n";
69-
ApiHelper::respond(OCSController::RESPOND_NOT_FOUND, $txt);
70-
} catch (MethodNotAllowedException $e) {
81+
} catch (ResourceNotFoundException $ex) {
82+
// Expected client-side failure; return an appropriate response without logging.
83+
$message = "Invalid query, please check the syntax. API specifications are here:\n"
84+
. "http://www.freedesktop.org/wiki/Specifications/open-collaboration-services\n";
85+
ApiHelper::respond(OCSController::RESPOND_NOT_FOUND, $message);
86+
} catch (MethodNotAllowedException $ex) {
87+
// Expected client-side failure; return an appropriate response without logging.
7188
ApiHelper::setContentType();
7289
http_response_code(405);
73-
} catch (LoginException $e) {
74-
ApiHelper::respond(OCSController::RESPOND_UNAUTHORISED, 'Unauthorised');
75-
} catch (\Exception $e) {
76-
Server::get(LoggerInterface::class)->error($e->getMessage(), ['exception' => $e]);
90+
} catch (\Exception $ex) {
91+
// Server-side failure: log it because it may require admin attention.
92+
$logger->error($ex->getMessage(), ['exception' => $ex]);
7793

78-
$txt = 'Internal Server Error' . "\n";
79-
try {
80-
if (Server::get(SystemConfig::class)->getValue('debug', false)) {
81-
$txt .= $e->getMessage();
82-
}
83-
} catch (\Throwable $e) {
84-
// Just to be save
94+
$message = "Internal Server Error \n";
95+
if ($debug) {
96+
$message .= $ex->getMessage();
8597
}
86-
ApiHelper::respond(OCSController::RESPOND_SERVER_ERROR, $txt);
98+
99+
ApiHelper::respond(OCSController::RESPOND_SERVER_ERROR, $message);
87100
}

0 commit comments

Comments
 (0)