Skip to content

Commit 77c93c2

Browse files
committed
src: deprecated bracket offsets throw on unsupported types
The deprecation notice encoded string offsets but stringified objects after the encode guard, so a Stringable offset's text reached 'notify' output raw. Now getRawValue() unwraps Smart* offsets so they display like plain keys, and any other object or array throws before the notice prints - they were never valid offsets, the lookup itself only takes int|string.
1 parent aebd797 commit 77c93c2

3 files changed

Lines changed: 88 additions & 1 deletion

File tree

src/Deprecations.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,13 @@ public static function triggerArrayAccessDeprecation(mixed $key, string $operati
564564
{
565565
// SECURITY: the key can be user input (e.g. $arr[$_GET['sort']]) and 'notify' mode echoes
566566
// the message into the page, so encode it. $key is display-only from here on; the actual
567-
// data access already happened with the original key.
567+
// data access already happened with the original key. Unwrap Smart* offsets so they
568+
// display like plain keys; everything else (Stringables, arrays) was never a valid
569+
// offset, so throw before any of it can reach the page.
570+
$key = self::getRawValue($key); // throws on unsupported objects
571+
if (!is_scalar($key) && $key !== null) {
572+
throw new InvalidArgumentException("Unsupported array offset type: " . get_debug_type($key));
573+
}
568574
if (is_string($key)) {
569575
$key = self::htmlEncode($key);
570576
}

tests/Unit/ReadAccessTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,47 @@ public function testOffsetGetMissingKeyWarnsAfterTheDeprecationNotice(string $cl
389389
$this->assertCount(1, $deprecations);
390390
}
391391

392+
#[DataProvider('modeProvider')]
393+
public function testOffsetGetRejectsUnsupportedOffsetTypesBeforeAnyOutput(string $class): void
394+
{
395+
// The notice echoes the offset in 'notify' mode, so unsupported types
396+
// (whose text the library doesn't control) must throw before printing
397+
$sa = $class::new(['name' => 'Bob']);
398+
$stringable = new class {
399+
public function __toString(): string
400+
{
401+
return '<img src=x onerror=alert(1)>';
402+
}
403+
};
404+
405+
foreach ([$stringable, ['name']] as $badOffset) {
406+
[$caught, $output] = $this->captureOutput(function () use ($sa, $badOffset) {
407+
try {
408+
return [$sa[$badOffset], null];
409+
} catch (InvalidArgumentException $e) {
410+
return [null, $e];
411+
}
412+
});
413+
[, $e] = $caught;
414+
$this->assertInstanceOf(InvalidArgumentException::class, $e);
415+
$this->assertStringContainsString('Unsupported', $e->getMessage());
416+
$this->assertStringNotContainsString('onerror', $e->getMessage());
417+
$this->assertSame('', $output, 'nothing printed before the throw');
418+
}
419+
420+
// SmartNull's own ArrayAccess methods dispatch through the same rules
421+
[$caught, $output] = $this->captureOutput(function () use ($sa, $stringable) {
422+
try {
423+
return [$sa->missing[$stringable], null];
424+
} catch (InvalidArgumentException $e) {
425+
return [null, $e];
426+
}
427+
});
428+
[, $e] = $caught;
429+
$this->assertInstanceOf(InvalidArgumentException::class, $e);
430+
$this->assertStringNotContainsString('onerror', $output, 'missing-key warning may print, but never the offset text');
431+
}
432+
392433
//endregion
393434
//region __isset / offsetExists
394435

tests/Unit/WriteAccessTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,46 @@ public function testUnsetArraySyntaxRemovesKeyAndNotifiesDeprecation(string $cla
247247
$this->assertCount(1, $deprecations);
248248
}
249249

250+
#[DataProvider('modeProvider')]
251+
public function testOffsetSetAndUnsetRejectUnsupportedOffsetTypesBeforeAnyOutput(string $class): void
252+
{
253+
// The notice echoes the offset in 'notify' mode, so unsupported types
254+
// (whose text the library doesn't control) must throw before printing
255+
$sa = $class::new(['name' => 'Bob']);
256+
$stringable = new class {
257+
public function __toString(): string
258+
{
259+
return '<img src=x onerror=alert(1)>';
260+
}
261+
};
262+
263+
[$e, $output] = $this->captureOutput(function () use ($sa, $stringable) {
264+
try {
265+
$sa[$stringable] = 'x';
266+
} catch (InvalidArgumentException $e) {
267+
return $e;
268+
}
269+
return null;
270+
});
271+
$this->assertInstanceOf(InvalidArgumentException::class, $e, 'set must throw');
272+
$this->assertStringNotContainsString('onerror', $e->getMessage());
273+
$this->assertSame('', $output, 'nothing printed before the set throw');
274+
275+
[$e, $output] = $this->captureOutput(function () use ($sa, $stringable) {
276+
try {
277+
unset($sa[$stringable]);
278+
} catch (InvalidArgumentException $e) {
279+
return $e;
280+
}
281+
return null;
282+
});
283+
$this->assertInstanceOf(InvalidArgumentException::class, $e, 'unset must throw');
284+
$this->assertStringNotContainsString('onerror', $e->getMessage());
285+
$this->assertSame('', $output, 'nothing printed before the unset throw');
286+
287+
$this->assertSame(['name' => 'Bob'], $sa->toArray(), 'collection unchanged');
288+
}
289+
250290
//endregion
251291
//region Position metadata on late writes
252292

0 commit comments

Comments
 (0)