Re: Re: [PHP-DEV] Fixing string offsets of strings.

I think I got what you where after - not to clear on the $s['offset'] should result in empty or 'o' This is the latest relivant patch https://bugs.php.net/patch-display.php?bug_id=60362&patch=fix_disabling_bad_string_offsets&revision=latest In that patch $s['offset'] would return an empty string to change that behaviour so it returns first character as before, just remove the lines in zend_execute, that munge the offset to -1 if (invalid_string) { dim->value.lval = -1; } As I said before, isset() should not show a warning, and should return false (yes it's a BC, but I do not think it's significant) Once check I need to do if the -1 munge is removed, is to see if isset($array['a_string']['test']) return true or false.. (as I'm not sure if the vm handles the array dereferencing on the secondary array or the execute calls do it.) Regards Alan --- On 18/Dec/2011, Stas Malyshev wrote: > Hi! > > I think the idea behind this patch is good. I'll do final checks and > apply it tomorrow if no objections heard until then. > > Some notes: > > > $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.. > > This change concerns me a bit since it's a behavior change and provided > that most of the people aren't actually aware that $s['offset'] means > $s[0] it may lead to subtle code breakage. Then again, doing > $s['offset'] leads to code breakage right now - I just encountered > another bug caused by this in production code mere days ago, code was > checking that $s['offset'] is set without actually checking that $s is > an array, and it happened to be not... > > > $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. > > Agreed, I think is_numeric is the right way. > > > 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 > > Here I am conflicted. The right thing would be to return empty but it > may cause some breakage. Then again, doing what we do now causes > breakage right now too (not new in 5.4, same breakage happening in 5.3 > too) so I'm inclined to go with false here unless somebody gives me a > use case where it's useful for anything. > > > 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. > > We tried to move to {} syntax but that didn't work, and probably isn't > possible anymore without massive breakage. > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227