Skip to content

feat(DI): Abort querying if infinite loop is detected #56218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 13, 2025
Merged

feat(DI): Abort querying if infinite loop is detected #56218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions lib/private/AppFramework/DependencyInjection/DIContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,21 +310,29 @@ public function has($id): bool {
return false;
}

public function query(string $name, bool $autoload = true) {
/**
* @inheritDoc
* @param list<class-string> $chain
*/
public function query(string $name, bool $autoload = true, array $chain = []) {
if ($name === 'AppName' || $name === 'appName') {
return $this->appName;
}

$isServerClass = str_starts_with($name, 'OCP\\') || str_starts_with($name, 'OC\\');
if ($isServerClass && !$this->has($name)) {
return $this->getServer()->query($name, $autoload);
/** @var ServerContainer $server */
$server = $this->getServer();
return $server->query($name, $autoload, $chain);
}

try {
return $this->queryNoFallback($name);
return $this->queryNoFallback($name, $chain);
} catch (QueryException $firstException) {
try {
return $this->getServer()->query($name, $autoload);
/** @var ServerContainer $server */
$server = $this->getServer();
return $server->query($name, $autoload, $chain);
} catch (QueryException $secondException) {
if ($firstException->getCode() === 1) {
throw $secondException;
Expand All @@ -339,23 +347,23 @@ public function query(string $name, bool $autoload = true) {
* @return mixed
* @throws QueryException if the query could not be resolved
*/
public function queryNoFallback($name) {
public function queryNoFallback($name, array $chain) {
$name = $this->sanitizeName($name);

if ($this->offsetExists($name)) {
return parent::query($name);
return parent::query($name, chain: $chain);
} elseif ($this->appName === 'settings' && str_starts_with($name, 'OC\\Settings\\')) {
return parent::query($name);
return parent::query($name, chain: $chain);
} elseif ($this->appName === 'core' && str_starts_with($name, 'OC\\Core\\')) {
return parent::query($name);
return parent::query($name, chain: $chain);
} elseif (str_starts_with($name, \OC\AppFramework\App::buildAppNamespace($this->appName) . '\\')) {
return parent::query($name);
return parent::query($name, chain: $chain);
} elseif (
str_starts_with($name, 'OC\\AppFramework\\Services\\')
|| str_starts_with($name, 'OC\\AppFramework\\Middleware\\')
) {
/* AppFramework services are scoped to the application */
return parent::query($name);
return parent::query($name, chain: $chain);
}

throw new QueryException('Could not resolve ' . $name . '!'
Expand Down
41 changes: 29 additions & 12 deletions lib/private/AppFramework/Utility/SimpleContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use ReflectionException;
use ReflectionNamedType;
use ReflectionParameter;
use RuntimeException;
use function class_exists;

/**
Expand Down Expand Up @@ -52,10 +53,11 @@

/**
* @param ReflectionClass $class the class to instantiate
* @param list<class-string> $chain
* @return object the created class
* @suppress PhanUndeclaredClassInstanceof
*/
private function buildClass(ReflectionClass $class): object {
private function buildClass(ReflectionClass $class, array $chain): object {
$constructor = $class->getConstructor();
if ($constructor === null) {
/* No constructor, return a instance directly */
Expand All @@ -64,17 +66,20 @@
if (PHP_VERSION_ID >= 80400 && self::$useLazyObjects && !$class->isInternal()) {
/* For PHP>=8.4, use a lazy ghost to delay constructor and dependency resolving */
/** @psalm-suppress UndefinedMethod */
return $class->newLazyGhost(function (object $object) use ($constructor): void {
return $class->newLazyGhost(function (object $object) use ($constructor, $chain): void {
/** @psalm-suppress DirectConstructorCall For lazy ghosts we have to call the constructor directly */
$object->__construct(...$this->buildClassConstructorParameters($constructor));
$object->__construct(...$this->buildClassConstructorParameters($constructor, $chain));
});
} else {
return $class->newInstanceArgs($this->buildClassConstructorParameters($constructor));
return $class->newInstanceArgs($this->buildClassConstructorParameters($constructor, $chain));
}
}

private function buildClassConstructorParameters(\ReflectionMethod $constructor): array {
return array_map(function (ReflectionParameter $parameter) {
/**
* @param list<class-string> $chain
*/
private function buildClassConstructorParameters(\ReflectionMethod $constructor, array $chain): array {
return array_map(function (ReflectionParameter $parameter) use ($chain) {
$parameterType = $parameter->getType();

$resolveName = $parameter->getName();
Expand All @@ -87,7 +92,7 @@
try {
$builtIn = $parameterType !== null && ($parameterType instanceof ReflectionNamedType)
&& $parameterType->isBuiltin();
return $this->query($resolveName, !$builtIn);
return $this->query($resolveName, !$builtIn, $chain);
} catch (ContainerExceptionInterface $e) {
// Service not found, use the default value when available
if ($parameter->isDefaultValueAvailable()) {
Expand All @@ -97,7 +102,7 @@
if ($parameterType !== null && ($parameterType instanceof ReflectionNamedType) && !$parameterType->isBuiltin()) {
$resolveName = $parameter->getName();
try {
return $this->query($resolveName);
return $this->query($resolveName, chain: $chain);
} catch (ContainerExceptionInterface $e2) {
// Pass null if typed and nullable
if ($parameter->allowsNull() && ($parameterType instanceof ReflectionNamedType)) {
Expand All @@ -114,12 +119,16 @@
}, $constructor->getParameters());
}

public function resolve($name) {
/**
* @inheritDoc
* @param list<class-string> $chain
*/
public function resolve($name, array $chain = []) {
$baseMsg = 'Could not resolve ' . $name . '!';
try {
$class = new ReflectionClass($name);

Check failure on line 129 in lib/private/AppFramework/Utility/SimpleContainer.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable 

lib/private/AppFramework/Utility/SimpleContainer.php:129:33: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
if ($class->isInstantiable()) {
return $this->buildClass($class);
return $this->buildClass($class, $chain);
} else {
throw new QueryException($baseMsg
. ' Class can not be instantiated');
Expand All @@ -130,14 +139,22 @@
}
}

public function query(string $name, bool $autoload = true) {
/**
* @inheritDoc
* @param list<class-string> $chain
*/
public function query(string $name, bool $autoload = true, array $chain = []) {
$name = $this->sanitizeName($name);
if (isset($this->container[$name])) {
return $this->container[$name];
}

if ($autoload) {
$object = $this->resolve($name);
if (in_array($name, $chain, true)) {
throw new RuntimeException('Tried to query ' . $name . ', but it is already in the chain: ' . implode(', ', $chain));
}

$object = $this->resolve($name, array_merge($chain, [$name]));
$this->registerService($name, function () use ($object) {
return $object;
});
Expand Down
11 changes: 6 additions & 5 deletions lib/private/ServerContainer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
if (!isset($this->hasNoAppContainer[$namespace])) {
$applicationClassName = 'OCA\\' . $sensitiveNamespace . '\\AppInfo\\Application';
if (class_exists($applicationClassName)) {
$app = new $applicationClassName();

Check failure on line 89 in lib/private/ServerContainer.php

View workflow job for this annotation

GitHub Actions / static-code-analysis-security

TaintedCallable 

lib/private/ServerContainer.php:89:17: TaintedCallable: Detected tainted text (see https://psalm.dev/243)
if (isset($this->appContainers[$namespace])) {
$this->appContainers[$namespace]->offsetSet($applicationClassName, $app);
return $this->appContainers[$namespace];
Expand All @@ -111,28 +111,29 @@
/**
* @template T
* @param class-string<T>|string $name
* @param list<class-string> $chain
* @return T|mixed
* @psalm-template S as class-string<T>|string
* @psalm-param S $name
* @psalm-return (S is class-string<T> ? T : mixed)
* @throws QueryException
* @deprecated 20.0.0 use \Psr\Container\ContainerInterface::get
*/
public function query(string $name, bool $autoload = true) {
public function query(string $name, bool $autoload = true, array $chain = []) {
$name = $this->sanitizeName($name);

if (str_starts_with($name, 'OCA\\')) {
// Skip server container query for app namespace classes
try {
return parent::query($name, false);
return parent::query($name, false, $chain);
} catch (QueryException $e) {
// Continue with general autoloading then
}
// In case the service starts with OCA\ we try to find the service in
// the apps container first.
if (($appContainer = $this->getAppContainerForService($name)) !== null) {
try {
return $appContainer->queryNoFallback($name);
return $appContainer->queryNoFallback($name, $chain);
} catch (QueryException $e) {
// Didn't find the service or the respective app container
// In this case the service won't be part of the core container,
Expand All @@ -144,14 +145,14 @@
$segments = explode('\\', $name);
try {
$appContainer = $this->getAppContainer(strtolower($segments[1]), $segments[1]);
return $appContainer->queryNoFallback($name);
return $appContainer->queryNoFallback($name, $chain);
} catch (QueryException $e) {
// Didn't find the service or the respective app container,
// ignore it and fall back to the core container.
}
}

return parent::query($name, $autoload);
return parent::query($name, $autoload, $chain);
}

/**
Expand Down
Loading