PHP function not executing correctly?

Question

I'm building a small PHP forum that allows users to create posts and comment on them. I have a specific function that's supposed to display the number of replies on a post, that for some reason is only displaying zero. I also have a similar function that counts views and it's working perfectly, so I'm not sure what the issue is.

Here is the function code for the replies:

  function countReplies($cid, $scid, $tid, $mysqli){
$select = mysqli_query($mysqli, "SELECT category_id, subcategory_id, topic_id FROM replies WHERE ".$cid." = category_id AND
                                ".$scid." = subcategory_id AND ".$tid." = topic_id");
return mysqli_num_rows($select);
}

And here is how I called it:

<div class = "content">

disptopics($_GET['cid'], $_GET['scid'], $mysqli);
countReplies($_GET['cid'], $_GET['scid'], $_GET['tid'], $mysqli);

?>
</div>

I'm getting the following error:

Notice: Undefined index: tid in C:\xampp\htdocs(A)Book 2.0\Bootstrap\topics.php on line 56

Warning: mysqli_num_rows() expects parameter 1 to be mysqli_result, boolean given in C:\xampp\htdocs(A)Book 2.0\Bootstrap\content_function.php on line 110

I'm pretty sure I defined variable tid, so any insight into this would be greatly appreciated.

It looks like you reversed your column names and the values you are looking for i.e. category_id = $cid. Also you should sanatize your query. — jdubu423, Oct 14 '18 at 00:20
What do you mean? Sorry, it's late and I've been looking at this for a while. — CaptainAmerica16, Oct 14 '18 at 00:22

score 2 · Answer 1 · edited Oct 14 '18 at 00:57

The error says otherwise, but the fact that says "Undefined index" means that is trying to lookup the index tid in an array, likely $_GET, not the variable $tid itself.

When this happens you get an invalid SQL sentence since you are effectively passing nothing to the condition, resulting in a syntax error:

"SELECT category_id, subcategory_id, topic_id FROM replies 
 WHERE $cid = category_id AND $scid = subcategory_id AND = topic_id"

Note the subcategory_id AND = topic_id.

To avoid that, you should always quote your parameters (this will work because of variable interpolation):

"SELECT category_id, subcategory_id, topic_id FROM replies 
 WHERE '$cid' = category_id AND '$scid' = subcategory_id AND '$tid' = topic_id"

You'll still get wrong results, but avoid other cascading errors.

However, you should check if all your required parameters are present before calling the function. And while you are at it, you also should Learn how to prevent SQL injection.

I tried to apply the changes, but now I'm getting this: Recoverable fatal error: Object of class mysqli could not be converted to string in C:\xampp\htdocs\(A)Book 2.0\Bootstrap\content_function.php on line 109 — CaptainAmerica16, Oct 14 '18 at 01:10
@CaptainAmerica16 It's trying to use `$mysqli` as a string for whatever reason, as if you swaped the arguments order or quoted the variable. Do you have a stray comma or something ? — msg, Oct 14 '18 at 03:40

score 1 · Answer 2 · answered Oct 14 '18 at 00:25

Your error message says that $_GET['tid'] has no value:

Notice: Undefined index: tid in C:\xampp\htdocs(A)Book 2.0\Bootstrap\topics.php on line 56

Another tips to your script:

1) Never use external variables directly to your SQL query. At MySQLi, always use real_escape_string method to filter external SQL injection;

2) It's easier to declare $mysqli as global variable, so, you doesn't need to pass it as parameter;

3) You can use COUNT(*) MySQL method to count the entries and avoid large data transfer and manipulation.

You can rewrite your code as:

function countReplies ( $cid, $scid, $tid)
{
  global $mysqli;

  if ( ! $select = $mysqli->query ( "SELECT COUNT(*) AS `Count` FROM `replies` WHERE `category_id` = '" . $mysqli->real_escape_string ( $cid) . "' AND `subcategory_id` = '" . $mysqli->real_escape_string ( $scid) . "' AND `topic_id` = '" . $mysqli->real_escape_string ( $tid) . "'"))
  {
    return false;
  }
  return $select->fetch_assoc ()["Count"];
}

This code is secure, and will return false if there's any error, or the number of replies found.

score 1 · Answer 3 · answered Oct 14 '18 at 00:29

You current query:

$select = mysqli_query($mysqli, "SELECT category_id, subcategory_id, topic_id FROM replies WHERE ".$cid." = category_id AND
                            ".$scid." = subcategory_id AND ".$tid." = topic_id");

It should be:

$select = mysqli_query($mysqli, "SELECT category_id, subcategory_id, topic_id FROM 
replies WHERE category_id = ".$cid." AND subcategory_id =

".$scid." = AND topic_id = ".$tid);

Lastly, look into sanitizing your query.
https://www.shift8web.ca/2015/06/securing-your-mysql-queries-from-sql-injection-in-php/

No, it shouldn't. It should be parameterized. Otherwise there is danger of SQL injection. — Jay Blanchard, Oct 14 '18 at 00:54

score 0 · Answer 4 · answered Oct 14 '18 at 00:26

The problem is from $_GET['tid'] are you sure it is set?

mysqli_num_rows() is returning that error because if there is fail on the query it returns false.

Check the $_GET['tid'] that has something acceptable first. And i always suggest to try and catch code or atleast confirm the result especially if it is from user input!

PHP function not executing correctly?

4 Answers4