From 87f98c88ab03f5196e697052482d996ebfaa36fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 1 Jul 2025 10:25:14 +0200 Subject: [PATCH 1/3] chore: Rename property and type it to match its current use in Request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/AppFramework/Http/Request.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index d177221556c66..bc45a653e7fb6 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -45,7 +45,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { public const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost|\[::1\])$/'; protected string $inputStream; - protected $content; + private bool $isPutStreamContentAlreadySent = false; protected array $items = []; protected array $allowedKeys = [ 'get', @@ -356,13 +356,13 @@ public function getCookie(string $key) { protected function getContent() { // If the content can't be parsed into an array then return a stream resource. if ($this->isPutStreamContent()) { - if ($this->content === false) { + if ($this->isPutStreamContentAlreadySent) { throw new \LogicException( '"put" can only be accessed once if not ' . 'application/x-www-form-urlencoded or application/json.' ); } - $this->content = false; + $this->isPutStreamContentAlreadySent = true; return fopen($this->inputStream, 'rb'); } else { $this->decodeContent(); From e8bc35ec0a6e8eed2208fdba3b4671771ee626f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 1 Jul 2025 10:42:29 +0200 Subject: [PATCH 2/3] fix(ocs): Return a proper error on JSON decoding failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/base.php | 9 +++++---- lib/private/AppFramework/Http/Request.php | 13 ++++++++++++- lib/public/IRequest.php | 10 ++++++++++ ocs/v1.php | 7 +++++-- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/lib/base.php b/lib/base.php index 876d62c359600..7bfef10fec2aa 100644 --- a/lib/base.php +++ b/lib/base.php @@ -141,8 +141,8 @@ public static function initPaths(): void { // Resolve /nextcloud to /nextcloud/ to ensure to always have a trailing // slash which is required by URL generation. - if (isset($_SERVER['REQUEST_URI']) && $_SERVER['REQUEST_URI'] === \OC::$WEBROOT && - substr($_SERVER['REQUEST_URI'], -1) !== '/') { + if (isset($_SERVER['REQUEST_URI']) && $_SERVER['REQUEST_URI'] === \OC::$WEBROOT + && substr($_SERVER['REQUEST_URI'], -1) !== '/') { header('Location: ' . \OC::$WEBROOT . '/'); exit(); } @@ -285,8 +285,8 @@ private static function printUpgradePage(\OC\SystemConfig $systemConfig): void { $tooBig = ($totalUsers > 50); } } - $ignoreTooBigWarning = isset($_GET['IKnowThatThisIsABigInstanceAndTheUpdateRequestCouldRunIntoATimeoutAndHowToRestoreABackup']) && - $_GET['IKnowThatThisIsABigInstanceAndTheUpdateRequestCouldRunIntoATimeoutAndHowToRestoreABackup'] === 'IAmSuperSureToDoThis'; + $ignoreTooBigWarning = isset($_GET['IKnowThatThisIsABigInstanceAndTheUpdateRequestCouldRunIntoATimeoutAndHowToRestoreABackup']) + && $_GET['IKnowThatThisIsABigInstanceAndTheUpdateRequestCouldRunIntoATimeoutAndHowToRestoreABackup'] === 'IAmSuperSureToDoThis'; if ($disableWebUpdater || ($tooBig && !$ignoreTooBigWarning)) { // send http status 503 @@ -987,6 +987,7 @@ public static function handleRequest(): void { } $request = Server::get(IRequest::class); + $request->throwDecodingExceptionIfAny(); $requestPath = $request->getRawPathInfo(); if ($requestPath === '/heartbeat') { return; diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index bc45a653e7fb6..348aeb0b7c233 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -64,6 +64,7 @@ class Request implements \ArrayAccess, \Countable, IRequest { protected ?CsrfTokenManager $csrfTokenManager; protected bool $contentDecoded = false; + private ?\JsonException $decodingException = null; /** * @param array $vars An associative array with the following optional values: @@ -389,7 +390,11 @@ protected function decodeContent() { // 'application/json' and other JSON-related content types must be decoded manually. if (preg_match(self::JSON_CONTENT_TYPE_REGEX, $this->getHeader('Content-Type')) === 1) { - $params = json_decode(file_get_contents($this->inputStream), true); + try { + $params = json_decode(file_get_contents($this->inputStream), true, flags:JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + $this->decodingException = $e; + } if (\is_array($params) && \count($params) > 0) { $this->items['params'] = $params; if ($this->method === 'POST') { @@ -413,6 +418,12 @@ protected function decodeContent() { $this->contentDecoded = true; } + public function throwDecodingExceptionIfAny(): void { + if ($this->decodingException !== null) { + throw $this->decodingException; + } + } + /** * Checks if the CSRF check was correct diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php index 2639a234ad573..cbac143d77575 100644 --- a/lib/public/IRequest.php +++ b/lib/public/IRequest.php @@ -305,4 +305,14 @@ public function getInsecureServerHost(): string; * @since 8.1.0 */ public function getServerHost(): string; + + /** + * If decoding the request content failed, throw an exception. + * Currently only \JsonException for json decoding errors, + * but in the future may throw other exceptions for other decoding issues. + * + * @throws \Exception + * @since 32.0.0 + */ + public function throwDecodingExceptionIfAny(): void; } diff --git a/ocs/v1.php b/ocs/v1.php index 3fc800e198a44..97032e6374036 100644 --- a/ocs/v1.php +++ b/ocs/v1.php @@ -50,11 +50,14 @@ // side effects in existing apps OC_App::loadApps(); + $request = Server::get(IRequest::class); + $request->throwDecodingExceptionIfAny(); + if (!Server::get(IUserSession::class)->isLoggedIn()) { - OC::handleLogin(Server::get(IRequest::class)); + OC::handleLogin($request); } - Server::get(Router::class)->match('/ocsapp' . Server::get(IRequest::class)->getRawPathInfo()); + Server::get(Router::class)->match('/ocsapp' . $request->getRawPathInfo()); } catch (MaxDelayReached $ex) { ApiHelper::respond(Http::STATUS_TOO_MANY_REQUESTS, $ex->getMessage()); } catch (ResourceNotFoundException $e) { From 79f4e0de7620e3ed7e0c4dca0b41a7c342cd67ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Tue, 1 Jul 2025 11:24:27 +0200 Subject: [PATCH 3/3] fix: Only attempt to decode JSON input if it is not an empty string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/AppFramework/Http/Request.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index 348aeb0b7c233..e662cb8679a38 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -390,10 +390,13 @@ protected function decodeContent() { // 'application/json' and other JSON-related content types must be decoded manually. if (preg_match(self::JSON_CONTENT_TYPE_REGEX, $this->getHeader('Content-Type')) === 1) { - try { - $params = json_decode(file_get_contents($this->inputStream), true, flags:JSON_THROW_ON_ERROR); - } catch (\JsonException $e) { - $this->decodingException = $e; + $content = file_get_contents($this->inputStream); + if ($content !== '') { + try { + $params = json_decode($content, true, flags:JSON_THROW_ON_ERROR); + } catch (\JsonException $e) { + $this->decodingException = $e; + } } if (\is_array($params) && \count($params) > 0) { $this->items['params'] = $params;