Merge pull request #31235 from nextcloud/techdebt/noid/extract-request-id

Extract request id handling to dedicated class so it can be injected without DB dependency

8 files changed, 127 insertions, 24 deletions

diff --git a/lib/base.php b/lib/base.php
index 2dd878fdbbc..f3c3e4f31cb 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -161,7 +161,11 @@ class OC {
 				'SCRIPT_FILENAME' => $_SERVER['SCRIPT_FILENAME'],
 			],
 		];
-		$fakeRequest = new \OC\AppFramework\Http\Request($params, new \OC\Security\SecureRandom(), new \OC\AllConfig(new \OC\SystemConfig(self::$config)));
+		$fakeRequest = new \OC\AppFramework\Http\Request(
+			$params,
+			new \OC\AppFramework\Http\RequestId($_SERVER['UNIQUE_ID'] ?? '', new \OC\Security\SecureRandom()),
+			new \OC\AllConfig(new \OC\SystemConfig(self::$config))
+		);
 		$scriptName = $fakeRequest->getScriptName();
 		if (substr($scriptName, -1) == '/') {
 			$scriptName .= 'index.php';
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index b6af8b8cef4..2ce5d448518 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -412,6 +412,7 @@ return array(
     'OCP\\INavigationManager' => $baseDir . '/lib/public/INavigationManager.php',
     'OCP\\IPreview' => $baseDir . '/lib/public/IPreview.php',
     'OCP\\IRequest' => $baseDir . '/lib/public/IRequest.php',
+    'OCP\\IRequestId' => $baseDir . '/lib/public/IRequestId.php',
     'OCP\\ISearch' => $baseDir . '/lib/public/ISearch.php',
     'OCP\\IServerContainer' => $baseDir . '/lib/public/IServerContainer.php',
     'OCP\\ISession' => $baseDir . '/lib/public/ISession.php',
@@ -635,6 +636,7 @@ return array(
     'OC\\AppFramework\\Http\\Dispatcher' => $baseDir . '/lib/private/AppFramework/Http/Dispatcher.php',
     'OC\\AppFramework\\Http\\Output' => $baseDir . '/lib/private/AppFramework/Http/Output.php',
     'OC\\AppFramework\\Http\\Request' => $baseDir . '/lib/private/AppFramework/Http/Request.php',
+    'OC\\AppFramework\\Http\\RequestId' => $baseDir . '/lib/private/AppFramework/Http/RequestId.php',
     'OC\\AppFramework\\Logger' => $baseDir . '/lib/private/AppFramework/Logger.php',
     'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
     'OC\\AppFramework\\Middleware\\CompressionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index b1e9912f506..4c35c1d1f85 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -441,6 +441,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OCP\\INavigationManager' => __DIR__ . '/../../..' . '/lib/public/INavigationManager.php',
         'OCP\\IPreview' => __DIR__ . '/../../..' . '/lib/public/IPreview.php',
         'OCP\\IRequest' => __DIR__ . '/../../..' . '/lib/public/IRequest.php',
+        'OCP\\IRequestId' => __DIR__ . '/../../..' . '/lib/public/IRequestId.php',
         'OCP\\ISearch' => __DIR__ . '/../../..' . '/lib/public/ISearch.php',
         'OCP\\IServerContainer' => __DIR__ . '/../../..' . '/lib/public/IServerContainer.php',
         'OCP\\ISession' => __DIR__ . '/../../..' . '/lib/public/ISession.php',
@@ -664,6 +665,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\AppFramework\\Http\\Dispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Dispatcher.php',
         'OC\\AppFramework\\Http\\Output' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Output.php',
         'OC\\AppFramework\\Http\\Request' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/Request.php',
+        'OC\\AppFramework\\Http\\RequestId' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/RequestId.php',
         'OC\\AppFramework\\Logger' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Logger.php',
         'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php',
         'OC\\AppFramework\\Middleware\\CompressionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php',
diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php
index 21af2bc46f4..f896b825f2d 100644
--- a/lib/private/AppFramework/Http/Request.php
+++ b/lib/private/AppFramework/Http/Request.php
@@ -48,8 +48,8 @@ use OC\Security\CSRF\CsrfTokenManager;
 use OC\Security\TrustedDomainHelper;
 use OCP\IConfig;
 use OCP\IRequest;
+use OCP\IRequestId;
 use OCP\Security\ICrypto;
-use OCP\Security\ISecureRandom;
 
 /**
  * Class for accessing variables in the request.
@@ -92,12 +92,10 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 		'method',
 		'requesttoken',
 	];
-	/** @var ISecureRandom */
-	protected $secureRandom;
+	/** @var RequestId */
+	protected $requestId;
 	/** @var IConfig */
 	protected $config;
-	/** @var string */
-	protected $requestId = '';
 	/** @var ICrypto */
 	protected $crypto;
 	/** @var CsrfTokenManager|null */
@@ -117,20 +115,20 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	 *        - array 'cookies' the $_COOKIE array
 	 *        - string 'method' the request method (GET, POST etc)
 	 *        - string|false 'requesttoken' the requesttoken or false when not available
-	 * @param ISecureRandom $secureRandom
+	 * @param IRequestId $requestId
 	 * @param IConfig $config
 	 * @param CsrfTokenManager|null $csrfTokenManager
 	 * @param string $stream
 	 * @see https://www.php.net/manual/en/reserved.variables.php
 	 */
 	public function __construct(array $vars,
-								ISecureRandom $secureRandom,
+								IRequestId $requestId,
 								IConfig $config,
 								CsrfTokenManager $csrfTokenManager = null,
 								string $stream = 'php://input') {
 		$this->inputStream = $stream;
 		$this->items['params'] = [];
-		$this->secureRandom = $secureRandom;
+		$this->requestId = $requestId;
 		$this->config = $config;
 		$this->csrfTokenManager = $csrfTokenManager;
 
@@ -571,16 +569,7 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 	 * @return string
 	 */
 	public function getId(): string {
-		if (isset($this->server['UNIQUE_ID'])) {
-			return $this->server['UNIQUE_ID'];
-		}
-
-		if (empty($this->requestId)) {
-			$validChars = ISecureRandom::CHAR_ALPHANUMERIC;
-			$this->requestId = $this->secureRandom->generate(20, $validChars);
-		}
-
-		return $this->requestId;
+		return $this->requestId->getId();
 	}
 
 	/**
diff --git a/lib/private/AppFramework/Http/RequestId.php b/lib/private/AppFramework/Http/RequestId.php
new file mode 100644
index 00000000000..70032873a75
--- /dev/null
+++ b/lib/private/AppFramework/Http/RequestId.php
@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2022, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OC\AppFramework\Http;
+
+use OCP\IRequestId;
+use OCP\Security\ISecureRandom;
+
+class RequestId implements IRequestId {
+	protected ISecureRandom $secureRandom;
+	protected string $requestId;
+
+	public function __construct(string $uniqueId,
+								ISecureRandom $secureRandom) {
+		$this->requestId = $uniqueId;
+		$this->secureRandom = $secureRandom;
+	}
+
+	/**
+	 * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
+	 * If `mod_unique_id` is installed this value will be taken.
+	 * @return string
+	 */
+	public function getId(): string {
+		if (empty($this->requestId)) {
+			$validChars = ISecureRandom::CHAR_ALPHANUMERIC;
+			$this->requestId = $this->secureRandom->generate(20, $validChars);
+		}
+
+		return $this->requestId;
+	}
+}
diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php
index e32c530c19e..6f89ba64e80 100644
--- a/lib/private/DB/Connection.php
+++ b/lib/private/DB/Connection.php
@@ -53,6 +53,7 @@ use OC\DB\QueryBuilder\QueryBuilder;
 use OC\SystemConfig;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\ILogger;
+use OCP\IRequestId;
 use OCP\PreConditionNotMetException;
 
 class Connection extends \Doctrine\DBAL\Connection {
@@ -281,11 +282,16 @@ class Connection extends \Doctrine\DBAL\Connection {
 	}
 
 	protected function logQueryToFile(string $sql): void {
-		$logFile = $this->systemConfig->getValue('query_log_file', '');
+		$logFile = $this->systemConfig->getValue('query_log_file');
 		if ($logFile !== '' && is_writable(dirname($logFile)) && (!file_exists($logFile) || is_writable($logFile))) {
+			$prefix = '';
+			if ($this->systemConfig->getValue('query_log_file_requestid') === 'yes') {
+				$prefix .= \OC::$server->get(IRequestId::class)->getId() . "\t";
+			}
+
 			file_put_contents(
 				$this->systemConfig->getValue('query_log_file', ''),
-				$sql . "\n",
+				$prefix . $sql . "\n",
 				FILE_APPEND
 			);
 		}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 00afaf1d6a9..b189905cfaf 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -60,6 +60,7 @@ use OC\App\AppStore\Fetcher\AppFetcher;
 use OC\App\AppStore\Fetcher\CategoryFetcher;
 use OC\AppFramework\Bootstrap\Coordinator;
 use OC\AppFramework\Http\Request;
+use OC\AppFramework\Http\RequestId;
 use OC\AppFramework\Utility\TimeFactory;
 use OC\Authentication\Events\LoginFailed;
 use OC\Authentication\Listeners\LoginFailedListener;
@@ -205,6 +206,7 @@ use OCP\ILogger;
 use OCP\INavigationManager;
 use OCP\IPreview;
 use OCP\IRequest;
+use OCP\IRequestId;
 use OCP\ISearch;
 use OCP\IServerContainer;
 use OCP\ISession;
@@ -1033,7 +1035,7 @@ class Server extends ServerContainer implements IServerContainer {
 						: '',
 					'urlParams' => $urlParams,
 				],
-				$this->get(ISecureRandom::class),
+				$this->get(IRequestId::class),
 				$this->get(\OCP\IConfig::class),
 				$this->get(CsrfTokenManager::class),
 				$stream
@@ -1042,6 +1044,13 @@ class Server extends ServerContainer implements IServerContainer {
 		/** @deprecated 19.0.0 */
 		$this->registerDeprecatedAlias('Request', \OCP\IRequest::class);
 
+		$this->registerService(IRequestId::class, function (ContainerInterface $c): IRequestId {
+			return new RequestId(
+				$_SERVER['UNIQUE_ID'] ?? '',
+				$this->get(ISecureRandom::class)
+			);
+		});
+
 		$this->registerService(IMailer::class, function (Server $c) {
 			return new Mailer(
 				$c->get(\OCP\IConfig::class),
@@ -1214,7 +1223,7 @@ class Server extends ServerContainer implements IServerContainer {
 		$this->registerAlias(EventDispatcherInterface::class, \OC\EventDispatcher\SymfonyAdapter::class);
 
 		$this->registerService('CryptoWrapper', function (ContainerInterface $c) {
-			// FIXME: Instantiiated here due to cyclic dependency
+			// FIXME: Instantiated here due to cyclic dependency
 			$request = new Request(
 				[
 					'get' => $_GET,
@@ -1227,7 +1236,7 @@ class Server extends ServerContainer implements IServerContainer {
 						? $_SERVER['REQUEST_METHOD']
 						: null,
 				],
-				$c->get(ISecureRandom::class),
+				$c->get(IRequestId::class),
 				$c->get(\OCP\IConfig::class)
 			);
 
diff --git a/lib/public/IRequestId.php b/lib/public/IRequestId.php
new file mode 100644
index 00000000000..dba06088cc9
--- /dev/null
+++ b/lib/public/IRequestId.php
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2022, Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCP;
+
+/**
+ * @since 24.0.0
+ */
+interface IRequestId {
+	/**
+	 * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
+	 * If `mod_unique_id` is installed this value will be taken.
+	 *
+	 * @return string
+	 * @since 24.0.0
+	 */
+	public function getId(): string;
+}