CHtml::value() in 1.1.13: replace is_string with is_scalar #1861

Closed
MonkeyMaster opened this Issue Dec 16, 2012 · 24 comments

7 participants

CHtml::value() in 1.1.13 RC brokes existing code. I use this function with assoc arrays and now get call_user_func() errors when $attribute is int.
Please replace is_string() with is_scalar()

Member
mdomba commented Dec 19, 2012

Related PR #1399

@resurtm can you take a look at this issue?

Contributor
resurtm commented Dec 19, 2012

Sorry for late replying.

@mdomba, yes i will analyze today all related past changes and issue itself.

@resurtm resurtm added a commit to resurtm/yii that referenced this issue Dec 19, 2012
@resurtm resurtm Fixes #1861. CHtml::value() didn't worked properly with array-typed $…
…model parameter and integer-typed $attribute parameter.
321e812
@resurtm resurtm referenced this issue Dec 19, 2012
Merged

Fixes #1861 #1868

Member
mdomba commented Dec 19, 2012

@MonkeyMaster please test @resurtm PR and let us know if it works for your case.

Contributor
resurtm commented Dec 19, 2012

And if there are no assertions for your use case let me know. I'll add them.

@resurtm PR fixed the issue

Contributor
resurtm commented Dec 20, 2012

@MonkeyMaster, great! Thanks for bug report! :-)

@mdomba mdomba was assigned Dec 20, 2012
@mdomba mdomba closed this in 321e812 Dec 20, 2012
Member
mdomba commented Dec 20, 2012

Merged... Thanks to both for reporting and fixing it...

New code brings more serious issue:
If a model has property named as some php function (for example, sort) CHtml::value() now tries to call that function!

CHtml::value($model, 'sort') results in Parameter 1 to sort() expected to be a reference, value given

@mdomba mdomba reopened this Dec 21, 2012
Contributor
resurtm commented Dec 21, 2012

Will fix it soon.

UPD: okay i've found a good solution. Preparing it.

vitalg commented Dec 21, 2012

Have the same problem:
Parameter 1 to key() expected to be a reference, value given
PHP version was 5.3.2

May be take a look this PHP bug:
https://bugs.php.net/bug.php?id=54910

In PHP 5.3.6 we don't have this problem.

Contributor
resurtm commented Dec 21, 2012

@vitalg, i think if #1877 would be merged this bug won't be related anymore. Since #1877 removes is_callable call at all.

Contributor
resurtm commented Dec 21, 2012

@vitalg, @MonkeyMaster, could you please test my new PR? :-)

Member
mdomba commented Dec 21, 2012

I merged the #1877 PR, please test it and let us know if it works for you so we can close this issue...

psihius commented Dec 21, 2012

@mdomba I work with @vitalg , we updated our PHP on DEV server from 5.3.2 to 5.3.6 and the issue disappeared before you merged the fix. Updating Yii and testing the fix you merged.

psihius commented Dec 21, 2012

@mdomba fix works and didn't break anything under PHP 5.3.6
PHP 5.3.2 can't be tested, because we don't have it installed anywhere anymore

Member
mdomba commented Dec 21, 2012

great, thanks for testing

Owner
samdark commented Dec 21, 2012

Closing this one?

Member
mdomba commented Dec 21, 2012

@MonkeyMaster issue did reopen this issue, so I was waiting for his confirmation...

@resurtm resurtm referenced this issue Dec 21, 2012
Merged

Enhances #1861 #1879

@mdomba mdomba closed this Dec 22, 2012

@resurtm Could you also add a check for null value (is_scalar($attribute) || $attribute === null)?
Now if $attribute happens to be null CHtml::value() results in
call_user_func() expects parameter 1 to be a valid callback, no array or string given

Contributor
resurtm commented Dec 23, 2012

@MonkeyMaster, thanks, PR created: #1882.

Owner

In what circumstance would $attribute be null?

Since CHtml::value() works with assoc arrays I used it widely than for getting model attributes - it's a convenient way to get a value by key with no php errors (till 1.1.13 rc).

Owner

I don't quite get it. Could you provide an example? IMO, it's a usage error by providing a null $attribute.

It's not exactly CHtml::value() is intended for but it was working

<?php
$models = $model->findAll(array('index' => 'id'));
foreach ($models as $model) {
    $model->root_r = CHtml::value($models, $model->root);
    $model->parent_r = CHtml::value($models, $model->parent);
}

As a replacement for

<?php
$model->root_r = isset($models[$model->root]) ? $models[$model->root] : null;
$model->parent_r = isset($models[$model->parent]) ? $models[$model->parent] : null;

Anyway without null check one could get strange error related to call_user_func()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment