Merge pull request #53740 from nextcloud/fix/properly-fail-on-invalid-json

Properly fail on invalid json

4 files changed, 34 insertions, 6 deletions

diff --git a/lib/base.php b/lib/base.php
index 8613ab62ef1..7bfef10fec2 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -987,6 +987,7 @@ class OC {
 		}
 
 		$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 d177221556c..e662cb8679a 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',
@@ -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:
@@ -356,13 +357,13 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	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();
@@ -389,7 +390,14 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 
 		// '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);
+			$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;
 				if ($this->method === 'POST') {
@@ -413,6 +421,12 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 		$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 2639a234ad5..cbac143d775 100644
--- a/lib/public/IRequest.php
+++ b/lib/public/IRequest.php
@@ -305,4 +305,14 @@ interface IRequest {
 	 * @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 3fc800e198a..97032e63740 100644
--- a/ocs/v1.php
+++ b/ocs/v1.php
@@ -50,11 +50,14 @@ try {
 	// 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) {