Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 3e67099

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #56488 [VarExporter] Fix exporting default values involving global constants (kylekatarnls)
This PR was merged into the 6.4 branch. Discussion ---------- [VarExporter] Fix exporting default values involving global constants | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | License | MIT Hello, we hit a case when a service works fine when injected normally but stop working when injected lazily. The reason was that the default value of a parameter was a global constant that was used without explicitly specifying `\` nor importing it. It ends up being incorrectly transformer in the proxy class as `\MyNamespace\GLOBAL_CONSTANT` instead of `\GLOBAL_CONSTANT`. We worked around that by adding the `\` in this service which is anyway rather a good move to be explicit. However I believe the `ReflectionCaster` should support this case because: - You might want to use a class from a third-party you don't control the code as a lazy service in Symfony and they might be legit not to use the FQCN since it's a feature provided by PHP to work. - So you can be confident that passing an injection from non-lazy to lazy is not going to cause problem. - It minimize surprises. We were confident that mocking this service in our tests was fine and so we didn't catch it early, it would be relatively tedious if we start to assume the proxy class can be incompatible where the normal class is and so that we would have to test all non-mocked of them in all possible situations. Commits ------- 5d33407 [VarExporter] Fix exporting default values involving global constants
2 parents f94a57d + 5d33407 commit 3e67099
Copy full SHA for 3e67099

File tree

Expand file treeCollapse file tree

2 files changed

+28
-7
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+28
-7
lines changed

‎src/Symfony/Component/VarExporter/ProxyHelper.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/VarExporter/ProxyHelper.php
+23-7Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -219,12 +219,14 @@ public static function exportSignature(\ReflectionFunctionAbstract $function, bo
219219
$args = '';
220220
$param = null;
221221
$parameters = [];
222+
$namespace = $function instanceof \ReflectionMethod ? $function->class : $function->getNamespaceName().'\\';
223+
$namespace = substr($namespace, 0, strrpos($namespace, '\\') ?: 0);
222224
foreach ($function->getParameters() as $param) {
223225
$parameters[] = ($param->getAttributes(\SensitiveParameter::class) ? '#[\SensitiveParameter] ' : '')
224226
.($withParameterTypes && $param->hasType() ? self::exportType($param).' ' : '')
225227
.($param->isPassedByReference() ? '&' : '')
226228
.($param->isVariadic() ? '...' : '').'$'.$param->name
227-
.($param->isOptional() && !$param->isVariadic() ? ' = '.self::exportDefault($param) : '');
229+
.($param->isOptional() && !$param->isVariadic() ? ' = '.self::exportDefault($param, $namespace) : '');
228230
if ($param->isPassedByReference()) {
229231
$byRefIndex = 1 + $param->getPosition();
230232
}
@@ -333,7 +335,7 @@ private static function exportPropertyScopes(string $parent): string
333335
return $propertyScopes;
334336
}
335337

336-
private static function exportDefault(\ReflectionParameter $param): string
338+
private static function exportDefault(\ReflectionParameter $param, $namespace): string
337339
{
338340
$default = rtrim(substr(explode('$'.$param->name.' = ', (string) $param, 2)[1] ?? '', 0, -2));
339341

@@ -347,26 +349,40 @@ private static function exportDefault(\ReflectionParameter $param): string
347349
$regexp = "/(\"(?:[^\"\\\\]*+(?:\\\\.)*+)*+\"|'(?:[^'\\\\]*+(?:\\\\.)*+)*+')/";
348350
$parts = preg_split($regexp, $default, -1, \PREG_SPLIT_DELIM_CAPTURE | \PREG_SPLIT_NO_EMPTY);
349351

350-
$regexp = '/([\[\( ]|^)([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z0-9_\x7f-\xff]++)*+)(?!: )/';
352+
$regexp = '/([\[\( ]|^)([a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z0-9_\x7f-\xff]++)*+)(\(?)(?!: )/';
351353
$callback = (false !== strpbrk($default, "\\:('") && $class = $param->getDeclaringClass())
352354
? fn ($m) => $m[1].match ($m[2]) {
353355
'new', 'false', 'true', 'null' => $m[2],
354356
'NULL' => 'null',
355357
'self' => '\\'.$class->name,
356358
'namespace\\parent',
357359
'parent' => ($parent = $class->getParentClass()) ? '\\'.$parent->name : 'parent',
358-
default => '\\'.$m[2],
359-
}
360+
default => self::exportSymbol($m[2], '(' !== $m[3], $namespace),
361+
}.$m[3]
360362
: fn ($m) => $m[1].match ($m[2]) {
361363
'new', 'false', 'true', 'null', 'self', 'parent' => $m[2],
362364
'NULL' => 'null',
363-
default => '\\'.$m[2],
364-
};
365+
default => self::exportSymbol($m[2], '(' !== $m[3], $namespace),
366+
}.$m[3];
365367

366368
return implode('', array_map(fn ($part) => match ($part[0]) {
367369
'"' => $part, // for internal classes only
368370
"'" => false !== strpbrk($part, "\\\0\r\n") ? '"'.substr(str_replace(['$', "\0", "\r", "\n"], ['\$', '\0', '\r', '\n'], $part), 1, -1).'"' : $part,
369371
default => preg_replace_callback($regexp, $callback, $part),
370372
}, $parts));
371373
}
374+
375+
private static function exportSymbol(string $symbol, bool $mightBeRootConst, string $namespace): string
376+
{
377+
if (!$mightBeRootConst
378+
|| false === ($ns = strrpos($symbol, '\\'))
379+
|| substr($symbol, 0, $ns) !== $namespace
380+
|| \defined($symbol)
381+
|| !\defined(substr($symbol, $ns + 1))
382+
) {
383+
return '\\'.$symbol;
384+
}
385+
386+
return '\\'.substr($symbol, $ns + 1);
387+
}
372388
}

‎src/Symfony/Component/VarExporter/Tests/ProxyHelperTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/VarExporter/Tests/ProxyHelperTest.php
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public static function provideExportSignature()
3737
$expected = str_replace(['.', ' . . . ', '\'$a\', \'$a\n\', "\$a\n"'], [' . ', '...', '\'$a\', "\$a\\\n", "\$a\n"'], $expected);
3838
$expected = str_replace('Bar', '\\'.Bar::class, $expected);
3939
$expected = str_replace('self', '\\'.TestForProxyHelper::class, $expected);
40+
$expected = str_replace('= [namespace\\M_PI, new M_PI]', '= [\M_PI, new \Symfony\Component\VarExporter\Tests\M_PI()]', $expected);
4041

4142
yield [$expected, $method];
4243
}
@@ -237,6 +238,10 @@ public static function foo8()
237238
public function foo9($a = self::BOB, $b = ['$a', '$a\n', "\$a\n"], $c = ['$a', '$a\n', "\$a\n", new \stdClass()])
238239
{
239240
}
241+
242+
public function foo10($a = [namespace\M_PI, new M_PI()])
243+
{
244+
}
240245
}
241246

242247
interface TestForProxyHelperInterface1

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.