Skip to content

Commit 2d0c5cf

Browse files
sorbaughsorbaugh
authored
Merge pull request #56218 from nextcloud/feat/di/abort-infinite-loop
feat(DI): Abort querying if infinite loop is detected
2 parents b0af462 + 3dbf848 commit 2d0c5cf

File tree

3 files changed

+53
-27
lines changed

3 files changed

+53
-27
lines changed

‎lib/private/AppFramework/DependencyInjection/DIContainer.php‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -310,21 +310,29 @@ public function has($id): bool {
310310
return false;
311311
}
312312

313-
public function query(string $name, bool $autoload = true) {
313+
/**
314+
* @inheritDoc
315+
* @param list<class-string> $chain
316+
*/
317+
public function query(string $name, bool $autoload = true, array $chain = []) {
314318
if ($name === 'AppName' || $name === 'appName') {
315319
return $this->appName;
316320
}
317321

318322
$isServerClass = str_starts_with($name, 'OCP\\') || str_starts_with($name, 'OC\\');
319323
if ($isServerClass && !$this->has($name)) {
320-
return $this->getServer()->query($name, $autoload);
324+
/** @var ServerContainer $server */
325+
$server = $this->getServer();
326+
return $server->query($name, $autoload, $chain);
321327
}
322328

323329
try {
324-
return $this->queryNoFallback($name);
330+
return $this->queryNoFallback($name, $chain);
325331
} catch (QueryException $firstException) {
326332
try {
327-
return $this->getServer()->query($name, $autoload);
333+
/** @var ServerContainer $server */
334+
$server = $this->getServer();
335+
return $server->query($name, $autoload, $chain);
328336
} catch (QueryException $secondException) {
329337
if ($firstException->getCode() === 1) {
330338
throw $secondException;
@@ -339,23 +347,23 @@ public function query(string $name, bool $autoload = true) {
339347
* @return mixed
340348
* @throws QueryException if the query could not be resolved
341349
*/
342-
public function queryNoFallback($name) {
350+
public function queryNoFallback($name, array $chain) {
343351
$name = $this->sanitizeName($name);
344352

345353
if ($this->offsetExists($name)) {
346-
return parent::query($name);
354+
return parent::query($name, chain: $chain);
347355
} elseif ($this->appName === 'settings' && str_starts_with($name, 'OC\\Settings\\')) {
348-
return parent::query($name);
356+
return parent::query($name, chain: $chain);
349357
} elseif ($this->appName === 'core' && str_starts_with($name, 'OC\\Core\\')) {
350-
return parent::query($name);
358+
return parent::query($name, chain: $chain);
351359
} elseif (str_starts_with($name, \OC\AppFramework\App::buildAppNamespace($this->appName) . '\\')) {
352-
return parent::query($name);
360+
return parent::query($name, chain: $chain);
353361
} elseif (
354362
str_starts_with($name, 'OC\\AppFramework\\Services\\')
355363
|| str_starts_with($name, 'OC\\AppFramework\\Middleware\\')
356364
) {
357365
/* AppFramework services are scoped to the application */
358-
return parent::query($name);
366+
return parent::query($name, chain: $chain);
359367
}
360368

361369
throw new QueryException('Could not resolve ' . $name . '!'

‎lib/private/AppFramework/Utility/SimpleContainer.php‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use ReflectionException;
2020
use ReflectionNamedType;
2121
use ReflectionParameter;
22+
use RuntimeException;
2223
use function class_exists;
2324

2425
/**
@@ -52,10 +53,11 @@ public function has(string $id): bool {
5253

5354
/**
5455
* @param ReflectionClass $class the class to instantiate
56+
* @param list<class-string> $chain
5557
* @return object the created class
5658
* @suppress PhanUndeclaredClassInstanceof
5759
*/
58-
private function buildClass(ReflectionClass $class): object {
60+
private function buildClass(ReflectionClass $class, array $chain): object {
5961
$constructor = $class->getConstructor();
6062
if ($constructor === null) {
6163
/* No constructor, return a instance directly */
@@ -64,17 +66,20 @@ private function buildClass(ReflectionClass $class): object {
6466
if (PHP_VERSION_ID >= 80400 && self::$useLazyObjects && !$class->isInternal()) {
6567
/* For PHP>=8.4, use a lazy ghost to delay constructor and dependency resolving */
6668
/** @psalm-suppress UndefinedMethod */
67-
return $class->newLazyGhost(function (object $object) use ($constructor): void {
69+
return $class->newLazyGhost(function (object $object) use ($constructor, $chain): void {
6870
/** @psalm-suppress DirectConstructorCall For lazy ghosts we have to call the constructor directly */
69-
$object->__construct(...$this->buildClassConstructorParameters($constructor));
71+
$object->__construct(...$this->buildClassConstructorParameters($constructor, $chain));
7072
});
7173
} else {
72-
return $class->newInstanceArgs($this->buildClassConstructorParameters($constructor));
74+
return $class->newInstanceArgs($this->buildClassConstructorParameters($constructor, $chain));
7375
}
7476
}
7577

76-
private function buildClassConstructorParameters(\ReflectionMethod $constructor): array {
77-
return array_map(function (ReflectionParameter $parameter) {
78+
/**
79+
* @param list<class-string> $chain
80+
*/
81+
private function buildClassConstructorParameters(\ReflectionMethod $constructor, array $chain): array {
82+
return array_map(function (ReflectionParameter $parameter) use ($chain) {
7883
$parameterType = $parameter->getType();
7984

8085
$resolveName = $parameter->getName();
@@ -87,7 +92,7 @@ private function buildClassConstructorParameters(\ReflectionMethod $constructor)
8792
try {
8893
$builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType)
8994
&& $parameterType->isBuiltin();
90-
return $this->query($resolveName, !$builtIn);
95+
return $this->query($resolveName, !$builtIn, $chain);
9196
} catch (ContainerExceptionInterface $e) {
9297
// Service not found, use the default value when available
9398
if ($parameter->isDefaultValueAvailable()) {
@@ -97,7 +102,7 @@ private function buildClassConstructorParameters(\ReflectionMethod $constructor)
97102
if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) {
98103
$resolveName = $parameter->getName();
99104
try {
100-
return $this->query($resolveName);
105+
return $this->query($resolveName, chain: $chain);
101106
} catch (ContainerExceptionInterface $e2) {
102107
// Pass null if typed and nullable
103108
if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) {
@@ -114,12 +119,16 @@ private function buildClassConstructorParameters(\ReflectionMethod $constructor)
114119
}, $constructor->getParameters());
115120
}
116121

117-
public function resolve($name) {
122+
/**
123+
* @inheritDoc
124+
* @param list<class-string> $chain
125+
*/
126+
public function resolve($name, array $chain = []) {
118127
$baseMsg = 'Could not resolve ' . $name . '!';
119128
try {
120129
$class = new ReflectionClass($name);
121130
if ($class->isInstantiable()) {
122-
return $this->buildClass($class);
131+
return $this->buildClass($class, $chain);
123132
} else {
124133
throw new QueryException($baseMsg
125134
. ' Class can not be instantiated');
@@ -130,14 +139,22 @@ public function resolve($name) {
130139
}
131140
}
132141

133-
public function query(string $name, bool $autoload = true) {
142+
/**
143+
* @inheritDoc
144+
* @param list<class-string> $chain
145+
*/
146+
public function query(string $name, bool $autoload = true, array $chain = []) {
134147
$name = $this->sanitizeName($name);
135148
if (isset($this->container[$name])) {
136149
return $this->container[$name];
137150
}
138151

139152
if ($autoload) {
140-
$object = $this->resolve($name);
153+
if (in_array($name, $chain, true)) {
154+
throw new RuntimeException('Tried to query ' . $name . ', but it is already in the chain: ' . implode(', ', $chain));
155+
}
156+
157+
$object = $this->resolve($name, array_merge($chain, [$name]));
141158
$this->registerService($name, function () use ($object) {
142159
return $object;
143160
});

‎lib/private/ServerContainer.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,28 +111,29 @@ public function has($id, bool $noRecursion = false): bool {
111111
/**
112112
* @template T
113113
* @param class-string<T>|string $name
114+
* @param list<class-string> $chain
114115
* @return T|mixed
115116
* @psalm-template S as class-string<T>|string
116117
* @psalm-param S $name
117118
* @psalm-return (S is class-string<T> ? T : mixed)
118119
* @throws QueryException
119120
* @deprecated 20.0.0 use \Psr\Container\ContainerInterface::get
120121
*/
121-
public function query(string $name, bool $autoload = true) {
122+
public function query(string $name, bool $autoload = true, array $chain = []) {
122123
$name = $this->sanitizeName($name);
123124

124125
if (str_starts_with($name, 'OCA\\')) {
125126
// Skip server container query for app namespace classes
126127
try {
127-
return parent::query($name, false);
128+
return parent::query($name, false, $chain);
128129
} catch (QueryException $e) {
129130
// Continue with general autoloading then
130131
}
131132
// In case the service starts with OCA\ we try to find the service in
132133
// the apps container first.
133134
if (($appContainer = $this->getAppContainerForService($name)) !== null) {
134135
try {
135-
return $appContainer->queryNoFallback($name);
136+
return $appContainer->queryNoFallback($name, $chain);
136137
} catch (QueryException $e) {
137138
// Didn't find the service or the respective app container
138139
// In this case the service won't be part of the core container,
@@ -144,14 +145,14 @@ public function query(string $name, bool $autoload = true) {
144145
$segments = explode('\\', $name);
145146
try {
146147
$appContainer = $this->getAppContainer(strtolower($segments[1]), $segments[1]);
147-
return $appContainer->queryNoFallback($name);
148+
return $appContainer->queryNoFallback($name, $chain);
148149
} catch (QueryException $e) {
149150
// Didn't find the service or the respective app container,
150151
// ignore it and fall back to the core container.
151152
}
152153
}
153154

154-
return parent::query($name, $autoload);
155+
return parent::query($name, $autoload, $chain);
155156
}
156157

157158
/**

0 commit comments

Comments
 (0)