Re: 5.4 regression: non-existent sub-sub keys now have values

On 11/24/2011 02:58 PM, Ferenc Kovacs wrote:

On Thu, Nov 24, 2011 at 9:12 PM, Larry Garfield<larry@garfieldtech.com>wrote:

On 11/23/2011 12:13 PM, Lester Caine wrote:

Richard Quadling wrote:

I agree with Daniel on this. Just looking for any test relating to isset() to see what tests will now fail.

So it's not just me :) I am seeing this break real world projects and can't see a easy way to fix the break. There is nothing really wrong with the current code except that the sub keys have yet to be populated.

This is going to be a huge problem for Drupal. Drupal uses deep associative array structures a lot, by design. That means we isset() or empty() on arrays a lot. I haven't had a chance to test it yet, but I see this change breaking, um, A LOT. And as Daniel noted, the fix is to turn one line of very readable code into 8 lines of hard to read code.

Did you managed to read the whole thread? I'm asking because there were lot of confusion about the actual impact of this problem, and Lester particularly seemed confused.

To be fair, no, I hadn't read the whole thread by the time I sent that. (One of the challenges of long fast-moving threads is they're hard to keep up with.)

"There is nothing really wrong with the current code except that the sub keys have yet to be populated." that isn't true, if the array or some sub elements are empty("yet to be populated"), you won't bump into this change. See my previous mail for the exact details. so the above mentioned one liner: "if (isset($arr['package']['attribs']['version'])) {" what could go wrong: $arr is not an array, but a string -> it would fatal on 5.3(Fatal: cannot use string offset as an array), but it would work with 5.4 $arr['package'] is not an array but a string -> false on 5.3, true on 5.4 (this is the worst case) $arr['package']['attribs'] is not an array but a string -> true on both 5.3 and 5.4 (so you are already screwed)

So to clarify, what Drupal does on a regular basis is something like: if (isset($foo['bar']['baz']['beep'])) { // Assume that bar, baz, and beep all exist, and that beep has a value } // or sometimes if (!empty($foo['bar']['baz']['beep'])) { // Assume that bar, baz, and beep all exist, // and that beep has a meaningful value } Generally $foo, bar, and baz will all be arrays, and if they're not it means someone else had a bug somewhere. Of course, Drupal module developers never have bugs in their code that accidentally puts a string where they should have put an array, no, not at all. :-) (Generally when that happens we already hit a "first argument to foreach() must be an array" error.) Currently we don't use ArrayAccess anywhere aside from inheriting it from PDO. If that doesn't change, then I rescind my previous panic attack. --Larry Garfield