Harden bootstrap context registrations when apps are missing

It's not expected that an app would be unavailable when the app container is created but when services are registered, but Sentry tells me on Nextcloud 21 there is an edge case where this can happen. Therefore this patch hardens the code a bit to log a meaningful error message and skipping the next code instead of logging a php notice for the undefined index and an exception for calling a method on null. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
nextcloud · Aug 31, 2021 · e3ae557 · e3ae557
1 parent 249e19b
commit e3ae557
Showing 1 changed file with 51 additions and 11 deletions.
diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php
@@ -302,12 +302,20 @@ public function registerTemplateProvider(string $appId, string $class): void {
 	 */
 	public function delegateCapabilityRegistrations(array $apps): void {
 		while (($registration = array_shift($this->capabilities)) !== null) {
+			$appId = $registration['appId'];
+			if (!isset($apps[$appId])) {
+				// If we land here something really isn't right. But at least we caught the
+				// notice that is otherwise emitted for the undefined index
+				$this->logger->error("App $appId not loaded for the capability registration");
+
+				continue;
+			}
+
 			try {
-				$apps[$registration['appId']]
+				$apps[$appId]
 					->getContainer()
 					->registerCapability($registration['capability']);
 			} catch (Throwable $e) {
-				$appId = $registration['appId'];
 				$this->logger->logException($e, [
 					'message' => "Error during capability registration of $appId: " . $e->getMessage(),
 					'level' => ILogger::ERROR,
@@ -380,19 +388,27 @@ public function delegateEventListenerRegistrations(IEventDispatcher $eventDispat
 	 */
 	public function delegateContainerRegistrations(array $apps): void {
 		while (($registration = array_shift($this->services)) !== null) {
+			$appId = $registration['appId'];
+			if (!isset($apps[$appId])) {
+				// If we land here something really isn't right. But at least we caught the
+				// notice that is otherwise emitted for the undefined index
+				$this->logger->error("App $appId not loaded for the container service registration");
+
+				continue;
+			}
+
 			try {
 				/**
 				 * Register the service and convert the callable into a \Closure if necessary
 				 */
-				$apps[$registration['appId']]
+				$apps[$appId]
 					->getContainer()
 					->registerService(
 						$registration['name'],
 						Closure::fromCallable($registration['factory']),
 						$registration['shared'] ?? true
 					);
 			} catch (Throwable $e) {
-				$appId = $registration['appId'];
 				$this->logger->logException($e, [
 					'message' => "Error during service registration of $appId: " . $e->getMessage(),
 					'level' => ILogger::ERROR,
@@ -401,15 +417,23 @@ public function delegateContainerRegistrations(array $apps): void {
 		}
 
 		while (($registration = array_shift($this->aliases)) !== null) {
+			$appId = $registration['appId'];
+			if (!isset($apps[$appId])) {
+				// If we land here something really isn't right. But at least we caught the
+				// notice that is otherwise emitted for the undefined index
+				$this->logger->error("App $appId not loaded for the container alias registration");
+
+				continue;
+			}
+
 			try {
-				$apps[$registration['appId']]
+				$apps[$appId]
 					->getContainer()
 					->registerAlias(
 						$registration['alias'],
 						$registration['target']
 					);
 			} catch (Throwable $e) {
-				$appId = $registration['appId'];
 				$this->logger->logException($e, [
 					'message' => "Error during service alias registration of $appId: " . $e->getMessage(),
 					'level' => ILogger::ERROR,
@@ -418,17 +442,25 @@ public function delegateContainerRegistrations(array $apps): void {
 		}
 
 		while (($registration = array_shift($this->parameters)) !== null) {
+			$appId = $registration['appId'];
+			if (!isset($apps[$appId])) {
+				// If we land here something really isn't right. But at least we caught the
+				// notice that is otherwise emitted for the undefined index
+				$this->logger->error("App $appId not loaded for the container parameter registration");
+
+				continue;
+			}
+
 			try {
-				$apps[$registration['appId']]
+				$apps[$appId]
 					->getContainer()
 					->registerParameter(
 						$registration['name'],
 						$registration['value']
 					);
 			} catch (Throwable $e) {
-				$appId = $registration['appId'];
 				$this->logger->logException($e, [
-					'message' => "Error during service alias registration of $appId: " . $e->getMessage(),
+					'message' => "Error during service parameter registration of $appId: " . $e->getMessage(),
 					'level' => ILogger::ERROR,
 				]);
 			}
@@ -440,12 +472,20 @@ public function delegateContainerRegistrations(array $apps): void {
 	 */
 	public function delegateMiddlewareRegistrations(array $apps): void {
 		while (($middleware = array_shift($this->middlewares)) !== null) {
+			$appId = $middleware['appId'];
+			if (!isset($apps[$appId])) {
+				// If we land here something really isn't right. But at least we caught the
+				// notice that is otherwise emitted for the undefined index
+				$this->logger->error("App $appId not loaded for the container middleware registration");
+
+				continue;
+			}
+
 			try {
-				$apps[$middleware['appId']]
+				$apps[$appId]
 					->getContainer()
 					->registerMiddleWare($middleware['class']);
 			} catch (Throwable $e) {
-				$appId = $middleware['appId'];
 				$this->logger->logException($e, [
 					'message' => "Error during capability registration of $appId: " . $e->getMessage(),
 					'level' => ILogger::ERROR,