Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer in $Model->cacheQueries causes exception #2508

Closed
jtv4k opened this issue Dec 19, 2013 · 3 comments
Closed

Integer in $Model->cacheQueries causes exception #2508

jtv4k opened this issue Dec 19, 2013 · 3 comments

Comments

@jtv4k
Copy link

jtv4k commented Dec 19, 2013

$Model->cacheQueries is a boolean, but it is common for setting values of 1 or 0 for true and false.

Using an integer causes an exception in DboSource.php::_execute (line 458 in 2.4):

if (!$query->execute($params)) {

$params is expected to be an array. The value $param is sanitized in DboSource::fetchAll (line 655) by doing an explicit check for boolean values:

if (is_bool($params)) {
    $options['cache'] = $params;
    $params = array();
}

Since integers do not register as booleans, the $params parameter is left as the unsanitized value of $Model->cacheQueries (in this case an integer). See DboSource::read (line 1076 in 2.4):

$resultSet = $this->fetchAll($query, $model->cacheQueries);
@markstory
Copy link
Member

I don't see why explicitly checking for booleans is a bad thing. You should use the proper types where possible. Is there a reason why you cannot use proper booleans?

markstory added a commit that referenced this issue Dec 19, 2013
@jtv4k
Copy link
Author

jtv4k commented Dec 19, 2013

Once I tracked it down I used the proper booleans. However, since the use of $model->cacheQueries is deep in the read method, I had to step through the entire read method and back trace from the exception. It was a head scratcher as to why $params had a numeric value until I noticed the is_bool check and figured out that $model->cacheQueries gets passed. It's a PDOException, which wouldn't ever lead me to believe I set $model->cacheQueries incorrectly.

Maybe an exception should be thrown in DboSource::execute if params isn't an array?

@markstory
Copy link
Member

Maybe, but the parameters and properties are documented as booleans. If you have a change you think would be helpful propose it as a pull request. I am going to c!ose this as using the framework improperly will cause exceptions, and I don't think adding more squishy typing is a good solution.

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

No branches or pull requests

3 participants