A few answers...
$s = "string"; isset($s['offset']) returns false
This is pretty critical, as it's the only way to detect this situation, and ensure that array tests do not return positive results for strings. It also catches an obvious, but previously hidden and probably serious bugs in the PHP code..
$s = "string"; $s[' 2 ']; and $s['2foo'] both emit errors, and return false from isset() - the code pretty much uses is_numeric() internally. - I'm pretty sure these would be un-intentional bugs, so behaving as such seems consistent.
I do not mind either way if $s['offset'] returns the first char or an empty string, however it seemed a little inconstant to return false on isset(), yet actually return a string. (although technically an empty string does kind of indicate it is 'isset') - it at least leaves the engine in a consistent state. Perhaps that change can wait's until 5.5.. thoughts...
As for dropping the syntax for Strings eventually.. It would be nice, but I'm not sure it is feasible anymore unfortunately.
Regards
Alan
On Monday, December 05, 2011 01:28 AM, Laruence wrote:
Hi:
I have submit a new patch based on the origin patch, which only
trigger notice when string offset cast occurred.
thanks
On Sun, Dec 4, 2011 at 10:25 PM, Laruence<
[email protected]> wrote:
+1.
thanks.
On Sun, Dec 4, 2011 at 10:05 PM, Ferenc Kovacs<
[email protected]> wrote:
On Sat, Dec 3, 2011 at 5:08 PM, Alan Knowles<
[email protected]> wrote:
I've had a look at making string offsets of strings a bit saner.
At present with the fix for array dereferencing : ?search=hello and a
test like isset($_GET['search']['name']) results in true, which is has
potential security problems and is very confusing for any programmer
finding and working out why something like that may be failing.
To solve this quite a few people agreed that not allowing non-numeric
string offsets on strings would be the smart way to go, the change is going
to break BC, so the idea is to at least not break it too badly...
This patch is a start.
https://bugs.php.net/patch-**display.php?bug_id=60362&**
patch=first_effort_to_fix_**this&revision=latest<
https://bugs.php.net/patch-display.php?bug_id=60362&patch=first_effort_to_fix_this&revision=latest>
It's been quite a while since I hacked on the engine, so the patch only
works reasonably well.. (see the FIXME on the tests at the bottom of the
patch.)
The patch changes the following:
* $s = "string"; $s['offset'] -- produces a warning (and returns an
empty string)
* $s = "string"; $s['1'] -- works as before..
* $s = "string"; $s[true] $s[false] $s[0.1] -- give a notice (cast it to
an int if you want to get rid of the notice) - however work as before.
* changes the warning on invalid indexes to say "Uninitialized or
invalid" rather than just "Uninitialized"
* fixes most of the related tests
I think that those changes are pretty much in line with the discussion that
we had.
Thanks for fixing this!
--
Ferenc Kovács
@Tyr43l -
http://tyrael.hu
--
Laruence Xinchen Hui
http://www.laruence.com/