refactor(redis, discovery, cache): enhance validation, error handling, and class filtering
- Remove redundant fallback for `RedisConfig` key prefix to enforce explicit configuration. - Refine `ClassExtractor` with class name validation to exclude invalid identifiers and handle creation errors. - Improve `AttributeCache` by validating class existence before reflection, preventing unnecessary exceptions and caching empty results on failure.
This commit is contained in:
@@ -38,11 +38,30 @@ final readonly class ClassExtractor
|
|||||||
}
|
}
|
||||||
|
|
||||||
$classes = $this->tokenizer->extractClasses($content);
|
$classes = $this->tokenizer->extractClasses($content);
|
||||||
|
$validClassNames = [];
|
||||||
|
|
||||||
return array_map(
|
foreach ($classes as $class) {
|
||||||
fn ($class) => ClassName::create($class['fqn']),
|
$fqn = $class['fqn'] ?? null;
|
||||||
$classes
|
if ($fqn === null || empty($fqn)) {
|
||||||
);
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Validate that the extracted name is actually a valid class name
|
||||||
|
// and not a property name, method name, or other identifier
|
||||||
|
if (! $this->isValidExtractedClassName($fqn)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
$className = ClassName::create($fqn);
|
||||||
|
$validClassNames[] = $className;
|
||||||
|
} catch (Throwable) {
|
||||||
|
// Skip invalid class names (e.g., property names mistaken as classes)
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $validClassNames;
|
||||||
|
|
||||||
} catch (Throwable) {
|
} catch (Throwable) {
|
||||||
// Silently fail for files that can't be processed
|
// Silently fail for files that can't be processed
|
||||||
@@ -50,6 +69,57 @@ final readonly class ClassExtractor
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate that an extracted class name is actually a class name
|
||||||
|
* and not a property name, method name, or other identifier
|
||||||
|
*/
|
||||||
|
private function isValidExtractedClassName(string $fqn): bool
|
||||||
|
{
|
||||||
|
// Extract the short name (last part after backslash)
|
||||||
|
$lastBackslash = strrpos($fqn, '\\');
|
||||||
|
$shortName = $lastBackslash === false ? $fqn : substr($fqn, $lastBackslash + 1);
|
||||||
|
|
||||||
|
// Class names should start with uppercase letter (PascalCase convention)
|
||||||
|
// But we can't enforce this strictly as some valid classes might not follow it
|
||||||
|
|
||||||
|
// However, we can filter out known invalid patterns:
|
||||||
|
// - Single lowercase words (likely property names like "state", "container")
|
||||||
|
// - camelCase starting with lowercase (likely method names)
|
||||||
|
|
||||||
|
// Check if it's a single lowercase word (without backslashes)
|
||||||
|
if ($shortName === strtolower($shortName) && ! str_contains($shortName, '_') && ! str_contains($shortName, '\\')) {
|
||||||
|
// Allow built-in types and known valid exceptions
|
||||||
|
$allowedLowercase = ['string', 'int', 'bool', 'float', 'array', 'object', 'mixed', 'void', 'null', 'false', 'true', 'iterable', 'callable', 'resource'];
|
||||||
|
if (in_array(strtolower($shortName), $allowedLowercase, true)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Single lowercase word is suspicious - likely a property/method name
|
||||||
|
// But we'll be conservative and only reject known problematic names
|
||||||
|
$knownInvalid = ['state', 'container', 'get', 'set', 'map', 'compile', 'install', 'shouldretry', 'additionaldata'];
|
||||||
|
if (in_array(strtolower($shortName), $knownInvalid, true)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// camelCase starting with lowercase (likely method names)
|
||||||
|
if (preg_match('/^[a-z][a-zA-Z0-9]*$/', $shortName) && ! str_contains($shortName, '\\')) {
|
||||||
|
$methodPrefixes = ['get', 'set', 'is', 'has', 'should', 'can', 'will', 'do', 'add', 'remove', 'update', 'delete'];
|
||||||
|
$lowercase = strtolower($shortName);
|
||||||
|
foreach ($methodPrefixes as $prefix) {
|
||||||
|
if (str_starts_with($lowercase, $prefix) && strlen($shortName) > strlen($prefix)) {
|
||||||
|
// Check if it's in our known invalid list
|
||||||
|
$knownInvalid = ['shouldretry', 'additionaldata'];
|
||||||
|
if (in_array($lowercase, $knownInvalid, true)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check if content contains actual PHP code
|
* Check if content contains actual PHP code
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -37,7 +37,7 @@ final readonly class RedisConfig
|
|||||||
database: 0,
|
database: 0,
|
||||||
timeout: 1.0,
|
timeout: 1.0,
|
||||||
readWriteTimeout: 1.0,
|
readWriteTimeout: 1.0,
|
||||||
keyPrefix: $env->get(EnvKey::REDIS_PREFIX, null)
|
keyPrefix: $env->get(EnvKey::REDIS_PREFIX)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -47,12 +47,27 @@ final class AttributeCache implements \App\Framework\Reflection\Contracts\Reflec
|
|||||||
{
|
{
|
||||||
$key = "{$className->getFullyQualified()}::attributes::" . ($attributeClass ?? 'all');
|
$key = "{$className->getFullyQualified()}::attributes::" . ($attributeClass ?? 'all');
|
||||||
if (! isset($this->attributeCache[$key])) {
|
if (! isset($this->attributeCache[$key])) {
|
||||||
/** @var class-string $className */
|
/** @var class-string $classNameString */
|
||||||
$className = $className->getFullyQualified();
|
$classNameString = $className->getFullyQualified();
|
||||||
$class = new \ReflectionClass($className);
|
|
||||||
|
try {
|
||||||
|
// Check if class exists before trying to reflect it
|
||||||
|
// This prevents ReflectionException for invalid class names (e.g., property names mistaken as classes)
|
||||||
|
if (! class_exists($classNameString) && ! interface_exists($classNameString) && ! trait_exists($classNameString)) {
|
||||||
|
// Cache empty result to avoid repeated checks
|
||||||
|
$this->attributeCache[$key] = [];
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
|
$class = new \ReflectionClass($classNameString);
|
||||||
$this->attributeCache[$key] = $attributeClass
|
$this->attributeCache[$key] = $attributeClass
|
||||||
? $class->getAttributes($attributeClass)
|
? $class->getAttributes($attributeClass)
|
||||||
: $class->getAttributes();
|
: $class->getAttributes();
|
||||||
|
} catch (\ReflectionException $e) {
|
||||||
|
// Class doesn't exist or can't be reflected - cache empty result
|
||||||
|
$this->attributeCache[$key] = [];
|
||||||
|
return [];
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return $this->attributeCache[$key];
|
return $this->attributeCache[$key];
|
||||||
|
|||||||
Reference in New Issue
Block a user