Bug #63217 [Asn->Ana]: Constant numeric strings become integers when used as ArrayAccess offset

From: Date: Fri, 26 May 2017 15:31:13 +0000
Subject: Bug #63217 [Asn->Ana]: Constant numeric strings become integers when used as ArrayAccess offset
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-209271@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=63217&edit=1

 ID:                 63217
 Updated by:         ajf@php.net
 Reported by:        kmsheng at pixnet dot tw
 Summary:            Constant numeric strings become integers when used
                     as ArrayAccess offset
-Status:             Assigned
+Status:             Analyzed
 Type:               Bug
 Package:            Arrays related
 Operating System:   freebsd
 PHP Version:        5.4Git-2012-10-04 (Git)
-Assigned To:        ajf
+Assigned To:        
 Block user comment: N
 Private report:     N

 New Comment:

Removing myself from being assigned to this. I could fix it, but I lost interest in doing so for the
most part.


Previous Comments:
------------------------------------------------------------------------
[2016-04-04 04:20:48] whatchildisthis at gmail dot com

Examples using expressions:

<?php
$test["1" . "0"]; // int(10) expect string(2) "10"
$test[1 + 0 . "0"]; // int(10) expect string(2) "10"
$test[<<<_
10
_
]; // int(10) expect string (2) "10"
$test[(string) "10"]; // only one that worked, string(2) "10"
?>

------------------------------------------------------------------------
[2015-11-24 17:23:15] ajf@php.net

I updated the title to better describe the bug.

------------------------------------------------------------------------
[2015-11-24 15:56:53] ajf@php.net

To clarify why this happens, it wasn't due to wanting to make ArrayAccess like arrays. Rather,
PHP has an optimisation for array indexing with a constant (a literal like "123", 123,
true, false, etc, not the const/define() kind) string (e.g. $_POST["password"]) where it
will check at compile-time if it is numeric and replace it then if so (so $foobar["123"]
becomes $foobar[123], but $foobar["bar"] stays the same). This means we don't have to
run the numeric string check every time that line of code is executed. Arrays in PHP consider
$foo["1"] and $foo[1] to be the same, so otherwise we'd need to check when we run the
code if the string is a number.

The problem is that $foobar in this case might actually be an object with ArrayAccess and not an
array, and so what apparently would be a transparent optimisation ends up causing this bug.

The solution is to remove this optimisation. This won't necessarily cause a performance hit,
because there's other ways we could avoid doing the numeric string check at runtime.

------------------------------------------------------------------------
[2015-11-24 15:10:48] levim@php.net

Changing this back to a bug status. Here is an example where I hope it is clear it is a bug (on
3v4l: https://3v4l.org/XGtVh):

<?php
class Dictionary implements ArrayAccess {
	function offsetExists($offset) {}
	function offsetGet($offset) {}
	function offsetUnset($offset) {}

	function offsetSet($offset, $value) {
		if (!is_string($offset)) {
			throw new InvalidArgumentException();
		}
	}
}

try {
    $Dictionary = new Dictionary();
    $Dictionary["12"] = 0xDEADBEEF;
    echo "No Exception for \"12\"\n";
} catch (InvalidArgumentException $e) {
    echo "Caught Exception for \"12\"\n";
}

try {
    $str = "12";
    $Dictionary[$str] = 0xDEADBEEF;
    echo "No Exception for \$variable = \"12\"\n";
} catch (InvalidArgumentException $e) {
    echo "Caught Exception for \$variable = \"12\"\n";
}

?>

------------------------------------------------------------------------
[2015-11-24 05:22:51] rudolf dot theunissen at gmail dot com

It makes sense for array keys to be coerced to intergers, ie. 1, 1.0, and "1" will attempt
to access the same index. However, this does not make sense for ArrayAccess. The argument for
consistency is poor, because float and object keys are handled differently already (floats stay
floats and objects don't raise warnings).  One example use case is SplObjectStorage, which
makes use of object keys via ArrayAccess.

With the introduction of strict types and scalar typehints, it makes sense for the implementation to
be responsible for handling various key types.

ArrayAccess should only provide array syntax, not array behaviour.

Test script:
https://3v4l.org/0BMYh
---------------
<?php

class Test implements ArrayAccess
{
    public function offsetGet($offset){
        return gettype($offset);
    }
    
    public function offsetSet($offset, $value){}
    public function offsetUnset($offset){}
    public function offsetExists($offset){}
}

$a = array('a', 'b', 'c');
$o = new Test();
$s = '1';

echo $o[1],   "\n"; // 'integer'
echo $o['1'], "\n"; // 'integer' !?
echo $o[1.0], "\n"; // 'double'
echo $o[$s],  "\n"; // 'string'
echo $o[$o],  "\n"; // 'object'

echo $a[1],   "\n"; // 'integer'
echo $a['1'], "\n"; // 'integer'
echo $a[1.0], "\n"; // 'integer'
echo $a[$s],  "\n"; // 'integer'
echo $a[$o],  "\n"; // Warning: Illegal offset type

Output:
---------------
integer
integer
double
string
object
b
b
b
b

Warning: Illegal offset type in...

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=63217


--
Edit this bug report at https://bugs.php.net/bug.php?id=63217&edit=1


Thread (7 messages)

« previous php.bugs (#209271) next »