Skip to content

Commit 53a365d

Browse files
committed
fix: env() TypeError for non-string $_SERVER values + esc() fixes
- env(): guard non-string values (int argc, array argv in CLI) before strtolower() to prevent TypeError under declare(strict_types=1) - esc(): propagate $encoding in recursive array calls (was ignored before), add early return after array processing, replace single static $escaper with static $escapers[] cache keyed by encoding - tests: data-provider test for env() non-string types, three tests for esc() foreach reference leak
1 parent e7ee5b0 commit 53a365d

2 files changed

Lines changed: 86 additions & 8 deletions

File tree

system/Common.php

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,12 @@ function env(string $key, $default = null)
416416
return $default;
417417
}
418418

419+
// Non-string values (e.g. $_SERVER['argc'] is int, $_SERVER['argv'] is array in CLI)
420+
// must be returned as-is to avoid TypeError from strtolower().
421+
if (! is_string($value)) {
422+
return $value;
423+
}
424+
419425
// Handle any boolean values
420426
return match (strtolower($value)) {
421427
'true' => true,
@@ -459,8 +465,11 @@ function esc($data, string $context = 'html', ?string $encoding = null)
459465

460466
if (is_array($data)) {
461467
foreach ($data as &$value) {
462-
$value = esc($value, $context);
468+
$value = esc($value, $context, $encoding);
463469
}
470+
unset($value); // Prevent reference leak: &$value would remain bound to last element
471+
472+
return $data;
464473
}
465474

466475
if (is_string($data)) {
@@ -470,16 +479,14 @@ function esc($data, string $context = 'html', ?string $encoding = null)
470479

471480
$method = $context === 'attr' ? 'escapeHtmlAttr' : 'escape' . ucfirst($context);
472481

473-
static $escaper;
474-
if (! $escaper) {
475-
$escaper = new Escaper($encoding);
476-
}
482+
static $escapers = [];
483+
$cacheKey = $encoding ?? 'default';
477484

478-
if ($encoding !== null && $escaper->getEncoding() !== $encoding) {
479-
$escaper = new Escaper($encoding);
485+
if (! isset($escapers[$cacheKey])) {
486+
$escapers[$cacheKey] = new Escaper($encoding);
480487
}
481488

482-
$data = $escaper->{$method}($data);
489+
$data = $escapers[$cacheKey]->{$method}($data);
483490
}
484491

485492
return $data;

tests/system/CommonFunctionsTest.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,43 @@ public function testEnvBooleans(): void
131131
$this->assertNull(env('p4'));
132132
}
133133

134+
#[DataProvider('provideEnvReturnsCorrectTypesWithoutTypeError')]
135+
public function testEnvReturnsCorrectTypesWithoutTypeError(string $source, mixed $value): void
136+
{
137+
$key = 'ci_test_var';
138+
139+
if ($source === 'SERVER' || $source === 'BOTH') {
140+
$_SERVER[$key] = $value;
141+
}
142+
143+
if ($source === 'ENV' || $source === 'BOTH') {
144+
$_ENV[$key] = $value;
145+
}
146+
147+
try {
148+
$this->assertSame($value, env($key));
149+
} finally {
150+
unset($_SERVER[$key], $_ENV[$key]);
151+
}
152+
}
153+
154+
public static function provideEnvReturnsCorrectTypesWithoutTypeError(): iterable
155+
{
156+
yield 'integer from SERVER' => ['SERVER', 2];
157+
158+
yield 'array from SERVER' => ['SERVER', ['spark', 'migrate']];
159+
160+
yield 'int 1 is not true' => ['SERVER', 1];
161+
162+
yield 'int 0 is not false' => ['SERVER', 0];
163+
164+
yield 'float from SERVER' => ['SERVER', 3.14];
165+
166+
yield 'integer from ENV' => ['ENV', 42];
167+
168+
yield 'CLI simulation BOTH' => ['BOTH', 3];
169+
}
170+
134171
private function createRouteCollection(): RouteCollection
135172
{
136173
return new RouteCollection(Services::locator(), new Modules(), new Routing());
@@ -276,6 +313,40 @@ public function testEscapeRecursiveArrayRaw(): void
276313
$this->assertSame($data, esc($data, 'raw'));
277314
}
278315

316+
public function testEscapeArrayDoesNotLeakForeachReference(): void
317+
{
318+
$data = ['first' => '<b>bold</b>', 'last' => '<i>italic</i>'];
319+
320+
$escaped = esc($data);
321+
322+
$value = 'CORRUPTED';
323+
324+
$this->assertSame('&lt;b&gt;bold&lt;/b&gt;', $escaped['first']);
325+
$this->assertSame('&lt;i&gt;italic&lt;/i&gt;', $escaped['last']);
326+
}
327+
328+
public function testEscapeArrayLastElementNotMutatedAfterCall(): void
329+
{
330+
$data = ['x' => '<script>', 'y' => '<style>'];
331+
332+
$escaped = esc($data);
333+
334+
$this->assertSame('&lt;script&gt;', $escaped['x']);
335+
$this->assertSame('&lt;style&gt;', $escaped['y']);
336+
$this->assertCount(2, $escaped);
337+
}
338+
339+
public function testEscapeArrayReferenceIsCleanedUpOnSingleElement(): void
340+
{
341+
$data = ['only' => '<div>'];
342+
343+
$escaped = esc($data);
344+
345+
$value = 'OVERWRITE_ATTEMPT';
346+
347+
$this->assertSame('&lt;div&gt;', $escaped['only']);
348+
}
349+
279350
#[PreserveGlobalState(false)]
280351
#[RunInSeparateProcess]
281352
#[WithoutErrorHandler]

0 commit comments

Comments
 (0)