Everything

php

question regarding type hinting parameters of php functions (array_slice)

php.internals

Dirk Thomas / 4wd media

18 years ago

Hi,

i have tried a current snapshot of PHP 5.3 and have a question regarding
type hinting.

For example when using the function
"array_slice(array $array, int $offset, int $length)"
with a non-integer length parameter, what is the desired behavior?

When calling
"array_slice($array, 0, (float)2);"
the resulting array is EMPTY.
When using the right type
"array_slice($array, 0, (int)2);"
it works as expected.

Shouldn't there be a notice/warning than just a wrong return value?
In my case there is neither a warning nor does it work as expected.
(perhaps i do something wrong?)

Of course i can use int's, but in my opinion either a warning should be
given or the function should gracefully handle the wrong typed parameter.

Is this problem specific to "array_splice" or the same with other
functions? It works with 5.2.x without a problem.

Thank you for any feedback,
Dirk

PS: i posted this to php-general before and someone supposed to better
post it to the iternals list

Marco Kaiser

18 years ago

Hi Dirk,

> When calling
>     "array_slice($array, 0, (float)2);"
>     the resulting array is EMPTY.
> When using the right type
>     "array_slice($array, 0, (int)2);"
>     it works as expected.

i think this should print a warning like other array functions.
But i looked into the src and this looks more like a casting bug inside this
function.

$input = array('a', 'b', 'c', 'd', 'e');
var_dump(array_slice($input, 1, "1"));
var_dump(array_slice($input, 1, 1));
var_dump(array_slice($input, 1, (FLOAT)1));
var_dump(array_slice($input, 1, 1.0));

output:
array(1) {
[0]=>
string(1) "b"
}
array(1) {
[0]=>
string(1) "b"
}
array(0) {
}
array(0) {
}

So i think the float value isnt correct casted as int value here. Maybe
someone else
can proof this.

-- Marco

Moritz Bechler

18 years ago

Hi,

>
>> When calling
>>     "array_slice($array, 0, (float)2);"
>>     the resulting array is EMPTY.
>> When using the right type
>>     "array_slice($array, 0, (int)2);"
>>     it works as expected.
>
> i think this should print a warning like other array functions.
> But i looked into the src and this looks more like a casting bug inside this
> function.

Actually most functions use implicit type conversions for this. So I
wouldn't say this should cause a warning.

>
>
> So i think the float value isnt correct casted as int value here. Maybe
> someone else
> can proof this.

Yes, you are right here I think....

the code is

if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) {
length = Z_LVAL_P(length_param);
} else {
length = num_in;
}

and afaik should be

if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) {
convert_to_long(length_param);
length = Z_LVAL_P(length_param);
} else {
length = num_in;
}


best regards

Moritz Bechler

Stanislav Malyshev

18 years ago

> the code is
>
> if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) {
> 	length = Z_LVAL_P(length_param);
> } else {
> 	length = num_in;
> }
>
> and afaik should be

I think in fact it should just parse it as long and not as zval - what
would be any reason to parse it as zval and then convert to long anyway?

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

Steph

18 years ago

Hi Stas,

I just spent most of two evenings looking at this one - so much for an easy
fix. Read on...

>> the code is
>>
>> if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) {
>> length = Z_LVAL_P(length_param);
>> } else {
>> length = num_in;
>> }
>>
>> and afaik should be
>
> I think in fact it should just parse it as long and not as zval - what
> would be any reason to parse it as zval and then convert to long anyway?

The problem is that this function's always been wrong, so it doesn't really
care what you throw at it - it just does a silent conversion to long if you
get it wrong. If you turn the parameter into a long now, there's a good
chance of breaking a lot of code out there - not least because at present if
you give array_slice() a length of 0, that's what it (sanely or otherwise)
takes it to mean. If you fix it, it will see 0 as meaning 'everything to the
end of the array'. So one of the tests at present is:

var_dump(array_slice($sub_array, 0, 0));

and the expectation is for that to always return:

array(0) {
}

Unfortunately this is far from new behaviour.

As far as the original float issue goes: What's been happening up to now is
that anything fed in as the third argument gets converted to a long, and now
it doesn't. Should be easy enough to sort out you'd think, but when you try
to do:

/* We want all entries from offset to the end if length is not passed or is
null */
if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) {
convert_to_long_ex(&length_param);
length = Z_LVAL_P(length_param);
} else {
length = num_in;
}

all of a sudden the fourth parameter, preserve_keys, doesn't throw
zend_parse_parameter() warnings any more, regardless of the type you give
it. (You tell me...)

There's also the issue of what to do about inappropriate vs appropriate
input types, since we can't rely on zend_parse_parameters() to psychically
know that this particular zval is really a loose long kind of thing... so I
tried:

if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) > IS_DOUBLE) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "integer expected in parameter
3, %s given", zend_zval_type_name(length_param));
}

This actually doesn't seem to harm anything, but I bet Dmitry'd say
otherwise (it probably slows the function right down).

So - not as straightforward as it looks, whatever way you look at it. Maybe
we should just revert to the old, messy but mostly working code for
array_slice()?

- Steph

Steph

18 years ago

> all of a sudden the fourth parameter, preserve_keys, doesn't throw
> zend_parse_parameter() warnings any more, regardless of the type you give
> it. (You tell me...)

... because I re-used the same variable in the test script after it had been
converted to long.

OK, so that mystery's solved. But the rest still stands.

- Steph

Stanislav Malyshev

18 years ago

>> I think in fact it should just parse it as long and not as zval - what
>> would be any reason to parse it as zval and then convert to long anyway?
>
> The problem is that this function's always been wrong, so it doesn't
> really care what you throw at it - it just does a silent conversion to
> long if you get it wrong. If you turn the parameter into a long now,

Right, that's what "l" parameter spec is doing, doesn't it? Why convert
it manually?

> There's also the issue of what to do about inappropriate vs appropriate
> input types, since we can't rely on zend_parse_parameters() to
> psychically know that this particular zval is really a loose long kind
> of thing... so I tried:

What's "loose long"?

> So - not as straightforward as it looks, whatever way you look at it.
> Maybe we should just revert to the old, messy but mostly working code
> for array_slice()?

I still don't understand what prevents one from using new parameter
parsing API and why manual conversions doing the same are needed.

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

Steph

18 years ago

>>> I think in fact it should just parse it as long and not as zval - what
>>> would be any reason to parse it as zval and then convert to long anyway?
>>
>> The problem is that this function's always been wrong, so it doesn't
>> really care what you throw at it - it just does a silent conversion to
>> long if you get it wrong. If you turn the parameter into a long now,
>
> Right, that's what "l" parameter spec is doing, doesn't it? Why convert it
> manually?

Because it doesn't know the difference between NULL and 0 if it's a long.

>> There's also the issue of what to do about inappropriate vs appropriate
>> input types, since we can't rely on zend_parse_parameters() to
>> psychically know that this particular zval is really a loose long kind of
>> thing... so I tried:
>
> What's "loose long"?

One that doesn't break every script that didn't strictly stay with int over
the last 8 years or so.

>> So - not as straightforward as it looks, whatever way you look at it.
>> Maybe we should just revert to the old, messy but mostly working code for
>> array_slice()?
>
> I still don't understand what prevents one from using new parameter
> parsing API and why manual conversions doing the same are needed.

See above. Unless you know a way to make IS_NULL work the same way with a
long as it does with a zval anyways.

- Steph

Stanislav Malyshev

18 years ago

>> Right, that's what "l" parameter spec is doing, doesn't it? Why
>> convert it manually?
>
> Because it doesn't know the difference between NULL and 0 if it's a long.

And why this difference is important in array_slice()? I don't see
anything in manual that says anything about NULLs. Could you explain?

> One that doesn't break every script that didn't strictly stay with int
> over the last 8 years or so.

Errm... I still don't understand. Could you give code example?

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com

Steph

18 years ago

>>> Right, that's what "l" parameter spec is doing, doesn't it? Why convert
>>> it manually?
>>
>> Because it doesn't know the difference between NULL and 0 if it's a long.
>
> And why this difference is important in array_slice()? I don't see
> anything in manual that says anything about NULLs. Could you explain?

There's an IS_NULL check...

if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) { ....

If Z_TYPE_P(length_param) is 0, it's not NULL because it has something in
it. If length_param were of type long and had a value of 0, it would be
NULL.

The manual kind of skirts around it:  "If length is given and is positive,
then the sequence will have that many elements in it. If length is given and
is negative then the sequence will stop that many elements from the end of
the array. If it is omitted, then the sequence will have everything from
offset up until the end of the array ."

>> One that doesn't break every script that didn't strictly stay with int
>> over the last 8 years or so.

Actually that check hasn't been there forever, behaviour's different again
in 5.1* - I didn't realize that. It used to always mean 0 (!) and was
changed in 5.2.3 in response to a bug report:
http://bugs.php.net/bug.php?id=41686.

> Errm... I still don't understand. Could you give code example?

C:\Documents and Settings\Steph\Desktop\PHP versions\5_1_2>php -a
Interactive mode enabled

<?php

$arr = array(1,2,3);
var_dump(array_slice($arr, 0, 0));
array(0) {
}
var_dump(array_slice($arr, 0, null));
array(0) {
}

C:\Documents and Settings\Steph\Desktop\PHP versions\5_2_4>php -a
Interactive mode enabled

<?php

$arr = array(1,2,3);
var_dump(array_slice($arr, 0, 0));
array(0) {
}
var_dump(array_slice($arr, 0, null));
array(3) {
[0]=>
int(1)
[1]=>
int(2)
[2]=>
int(3)
}

I think 0 and NULL should both always have acted like NULL does in 5_2_4
(which would happen if the third parameter was a long), but since 0 has
always acted wrongly it's possible that people are relying on that
behaviour. There's also the side-issue that every type has always been
allowed in that argument, whether it makes sense or not.

- Steph

ps the patch on my bug report is now out of date (again).

Stanislav Malyshev

18 years ago

> There's an IS_NULL check...
>
> if (ZEND_NUM_ARGS() >= 3 && Z_TYPE_P(length_param) != IS_NULL) { ....
>
> If Z_TYPE_P(length_param) is 0, it's not NULL because it has something
> in it. If length_param were of type long and had a value of 0, it would
> be NULL.

Ouch... Now I see. Is there anybody using this trick, provided it never
was documented? This is *so* wrong.

> The manual kind of skirts around it:  "If length is given and is
> positive, then the sequence will have that many elements in it. If
> length is given and is negative then the sequence will stop that many
> elements from the end of the array. If it is omitted, then the sequence
> will have everything from offset up until the end of the array ."

Meaning, basically, -1 is the same as omitted and the same as null?

--
Stanislav Malyshev, Zend Software Architect
stas@zend.com   http://www.zend.com/
(408)253-8829   MSN: stas@zend.com