Fix bug 71572

[flaupretre]

See https://bugs.php.net/bug.php?id=71572

[nikic]

Should be size_t.

[flaupretre]

@nikic When using an invalid string offset, the assignment returns null. I did the same for assignment from an empty string. A quick look at zend_execute.c shows that some other failing assignments also return null. But, returning the unchanged character is an option too. As you know the core much better than me, just tell me which one you prefer.

Edit: after thinking more about it, I now also think that returning the unchanged character is more consistent. Patch modified.

[nikic]

@flaupretre As all the other dimension assignment failure cases return null, this looks fine.

[nikic]

Please add a test case for $str{10} = "" as well. Currently this will still extend the string, and likely additionally leave one uninitialized byte.

[flaupretre]

@nikic So, finally, as both are possible (and I don't have a strong opinion about it), which one is better, in your opinion ?

[nikic]

Just thinking out loud. The options are:

• Return the original RHS. This would be, for a number of reasons, the most useful and sane thing to do, but sadly it's just not the way other assignments work.
• Return the result of interpreting the LHS as a read operation. This would mean returning the original character if the offset is within range or the empty string if the offset is larger than the string length. However, this does not match the null return value when assigning to a negative string offset (it ought to be an empty string instead, if this pattern were followed).
• Return null, following the other non-fatal dimension assignment error cases.
• A mix of these, returning the original LHS character if within range, null otherwise.
• Throw an exception :P

I'd probably go for always returning null.

[flaupretre]

New version. If RHS resolves to an empty string :

• LHS string is never modified (no extension if offset larger than string length)
• assignment returns null.

[hikari-no-yume]

Is this meant to be a new line? Using a tab here is strange.

[hikari-no-yume]

This looks good to me, any objections to merging?

[nikic]

Merged via be607e7. Per Andreas suggestion, I've moved the new comments to the next line.