Skip to content

Commit e793e58

Browse files
gr8manmichalsn
andauthored
fix: env() TypeError for non-string $_SERVER values + esc() fixes (#10305)
* 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 * Apply suggestion from @michalsn Co-authored-by: Michal Sniatala <michal@sniatala.pl> * Apply suggestion from @michalsn Co-authored-by: Michal Sniatala <michal@sniatala.pl> * Update system/Common.php Co-authored-by: Michal Sniatala <michal@sniatala.pl> * style: cs-fix * test: add tests for esc() encoding changes and update changelog * Update tests/system/CommonFunctionsTest.php Co-authored-by: Michal Sniatala <michal@sniatala.pl> * docs: move env/esc fix changelog entry to v4.7.4.rst * docs: split Common changelog entry --------- Co-authored-by: Michal Sniatala <michal@sniatala.pl>
1 parent 32eeb5d commit e793e58

3 files changed

Lines changed: 69 additions & 8 deletions

File tree

system/Common.php

Lines changed: 14 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,10 @@ 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+
471+
return $data;
464472
}
465473

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

471479
$method = $context === 'attr' ? 'escapeHtmlAttr' : 'escape' . ucfirst($context);
472480

473-
static $escaper;
474-
if (! $escaper) {
475-
$escaper = new Escaper($encoding);
476-
}
481+
static $escapers = [];
482+
$cacheKey = strtolower($encoding ?? 'utf-8');
477483

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

482-
$data = $escaper->{$method}($data);
488+
$data = $escapers[$cacheKey]->{$method}($data);
483489
}
484490

485491
return $data;

tests/system/CommonFunctionsTest.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use Config\Services;
4343
use Config\Session as SessionConfig;
4444
use Exception;
45+
use InvalidArgumentException;
4546
use Kint;
4647
use PHPUnit\Framework\Attributes\BackupGlobals;
4748
use PHPUnit\Framework\Attributes\DataProvider;
@@ -131,6 +132,42 @@ public function testEnvBooleans(): void
131132
$this->assertNull(env('p4'));
132133
}
133134

135+
#[DataProvider('provideEnvReturnsCorrectTypesWithoutTypeError')]
136+
public function testEnvReturnsCorrectTypesWithoutTypeError(string $source, mixed $value): void
137+
{
138+
$key = 'ci_test_var';
139+
140+
if ($source === 'SERVER' || $source === 'BOTH') {
141+
service('superglobals')->setServer($key, $value);
142+
}
143+
144+
if ($source === 'ENV' || $source === 'BOTH') {
145+
$_ENV[$key] = $value;
146+
}
147+
148+
$this->assertSame($value, env($key));
149+
}
150+
151+
/**
152+
* @return iterable<string, array{string, mixed}>
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,22 @@ public function testEscapeRecursiveArrayRaw(): void
276313
$this->assertSame($data, esc($data, 'raw'));
277314
}
278315

316+
public function testEscapeArrayPropagatesEncoding(): void
317+
{
318+
$this->expectException(InvalidArgumentException::class);
319+
// If encoding is not propagated, it would not instantiate the Escaper with the invalid encoding and wouldn't throw.
320+
esc(['test'], 'html', 'invalid-encoding');
321+
}
322+
323+
public function testEscapeWithChangingArrayEncoding(): void
324+
{
325+
$data = [hex2bin('E9')];
326+
327+
$this->assertSame(['&#xE9;'], esc($data, 'attr', 'iso-8859-1'));
328+
$this->assertSame(['&#x0439;'], esc($data, 'attr', 'windows-1251'));
329+
$this->assertSame(['&#xE9;'], esc($data, 'attr', 'iso-8859-1'));
330+
}
331+
279332
#[PreserveGlobalState(false)]
280333
#[RunInSeparateProcess]
281334
#[WithoutErrorHandler]

user_guide_src/source/changelogs/v4.7.4.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ Bugs Fixed
3232

3333
- **API:** Fixed a bug in Transformers where the root request's ``fields`` and ``include`` query parameters leaked into nested transformers created inside ``include*()`` methods, causing incorrect field filtering, unexpected includes, or infinite recursion.
3434
- **Commands:** Fixed a bug where ``make:model --return entity`` did not preserve sub-namespaces when generating the related Entity class.
35+
- **Common:** Fixed a bug in ``env()`` where a ``TypeError`` could be thrown when non-string values were passed.
36+
- **Common:** Fixed ``esc()`` to propagate encoding correctly and prevent reference leaks.
3537
- **Commands:** Fixed a bug where ``spark lang:find`` treated translation keys already provided by the framework or another namespace (such as ``Errors.*`` in ``system/Language``) as new, listing them under ``--show-new`` and writing untranslated placeholders into ``app/Language`` that overrode the existing translations.
3638
- **Database:** Fixed a bug where ``updateBatch()`` could be called after Query Builder ``where()`` conditions, even though it's not supported. In this situation, now the ``DatabaseException`` is thrown.
3739
- **Filters:** Fixed a bug in ``InvalidChars`` filter where invalid UTF-8 or control characters in array keys were not checked.

0 commit comments

Comments
 (0)