3

I am trying to select records where a column is in a suggested list if values using the IN clause. I have tried the query below but the system is not working.

    $db = JFactory::getDbo();
    $query = $db->getQuery(true); 
    $query
        ->select(array('type', 'name', 'id', 'created_by', 'amount', 'created', 'cat_id', 'status'))
        ->from($db->quoteName('#__zoo_item'))
        ->where($db->quoteName('created_by') .' = '.$db->quote(10))
        ->where($db->quoteName('status') .' IN '.$db->quote(1,2,4))//THIS IS MY PROBLEM.
        ->group($db->quoteName('created_by'))
        ->order('id DESC')
        ->setLimit(1);
    $db->setQuery($query);
    $loan = $db->loadObject();

The problematic clause is this

->where($db->quoteName('status') .' IN '.$db->quote(1,2,4))

I will be glad if someone can help, thank you.

6

First, the trouble with

->where($db->quoteName('status') .' IN '.$db->quote(1,2,4))

is not only that a syntax error is generated on the query, but also that $db->quote() is treating 1 as the $text parameter and 2,4 as the expected bool value for the $escape parameter. (I code on PhpStorm and this fact is spelled out)

Unfortunately for your project, while the solution that you have posted removes the syntax fault in your query, the logic fault remains. Effectively, the 2 and 4 values have been disregarded during all of your queries.

If you call $query->dump(), you will see that your generated query looks like this:

SELECT type,name,id,created_by,amount,created,cat_id,status
FROM `zyxwv_zoo_item`
WHERE `created_by` = '10' AND `status` IN ('1')
GROUP BY `created_by`
ORDER BY id DESC LIMIT 1

This is the same damaged value for IN as in your original code, except it is parenthetically-wrapped now.


Second, a common misconception is that you NEED to quote every possible data point in your Joomla sql queries. This coding practice errs on the side of caution -- and isn't a horrid piece of advice because some Joomla developers are very new and don't know a secure/stable query from an insecure/instable one.

The truth is, if you are hardcoding integers into an array (or more generally, the data is not supplied by an untrustworthy source -- user input or an offsite resource -- and you are not incorporating values with monkeywrenching characters like quotes), then you have sufficient control over the stability of the query and there is no impact on security.

For the query posted above, it is ENTIRELY stable and secure to write it this way:

$query = $db->getQuery(true)
    ->select(['type', 'name', 'id', 'created_by', 'amount', 'created', 'cat_id', 'status'])
    ->from("#__zoo_item")
    ->where(["created_by = 10", "status IN (1,2,4)"])
    ->order("id DESC")
    ->setLimit(1);

There are no MySQL Reserved Keywords, no variables, no quotes in strings, and it is assumed that the table prefix #__ is stable to begin with, so the query can be written with far less code-bloat, improved efficiency, and still be just as secure and stable.

*note, there were no AGGREGATE functions in the SELECT clause, so I removed the needless group() clause.


For other researchers...

You could quote-wrap the integers like this:

->where("status IN (" . implode(',', $db->q($array)) . ")")

Although this technique will prove to protect your query from an injection attack, it does not sanitise the input values as integers. This is more of a shortcut than an appropriate solution, because it would allow completely erroneous data types to come into the query.

If you have an untrusted/dynamic array of string values (all of which are assumed to be integers), then you may write your own custom sanitizing process or you could cast each value as an integer.

->where("status IN (" . implode(',', array_map('intval', $array)) . ")")  

or you can use the ArrayHelper method in the core (which effectively does the same thing)...

This is expressed in the Joomla Documentation @ Secure_arrays_of_integers section of https://docs.joomla.org/Secure_coding_guidelines**.

->where($db->quoteName('status') . ' IN (' . implode(',', ArrayHelper::toInteger($array)) . ')')

You can see that the Joomla core is appropriately securing its own queries with toInteger() in files like: administrator/components/com_contact/models/contacts.php

*note, depending on accessibility of your script, you may need to declare:

use \Joomla\Utilities\ArrayHelper;

For anyone who wants to generate a quoted/escaped comma-separated string for IN (because the values are non-integers), then the syntax gets a bit longer:

sub-PHP7.4

->where("fullname IN (" . implode(',', array_map(function($n)use($db){return $db->q($n);}, $array)) . ")")

PHP7.4+

->where("fullname IN (" . implode(',', array_map(fn($n) => $db->q($n), $array)) . ")")

This final snippet will properly single-quote each string and escape the single-quotes within each string (e.g. 'Mark O\'Malley') Here's a similar post for a bit more reading.

0

I have solved it. What I did was to surround the quote() method with a bracket

->where($db->quoteName('status') .' IN ('.$db->quote(1,2,4).')')

This solved the problem. I hope it helps someone.

  • Yes, this removes the syntax error, but the intended query logic is broken -- 2 and 4 are lost. I wish I could track down the 3 upvoters and explain that this is not the solution that they are looking for. – mickmackusa May 26 '18 at 14:56
  • Please take a moment to earn your Disciplined Badge by removing this post. I have explained why your answer is definitely not something that researchers should be implementing. I'll also ask you to accept my answer so that this page is deemed resolved in this community (and will no longer get "bumped" by the system). – mickmackusa Oct 18 '18 at 0:38
  • I don't think deleting this answer will be appropriate. It works although it is not the right way. I decided to leave it there for someone using the same method to correct and learn the right way – David Addoteye Nov 23 '18 at 2:45
  • I fundamentally disagree with your decision -- deleting an incorrect solution because it will confuse/mislead researchers is always a good idea -- but I am powerless to force my will. I wanted to give you the chance to earn a badge by deleting this incorrect answer with 3 upvotes, but if you will not delete then the best thing that I can do for this community is downvote to discourage readers from reading it. – mickmackusa Nov 23 '18 at 2:52
  • I don't know why we have to have this discussion. Kindly take your time to review other question on the main StackOverflow. There are many methods of doing things and there is the right way to do them. My method might not be the appropriate but it is correct. I accepted your answer because it is right not because mine did not work. – David Addoteye Nov 23 '18 at 3:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.