Possible bug, but no time to test it

[emanuele]

And I'm not even sure how to test it.

Code Select Expand

while ($row = $smcFunc['db_fetch_assoc']($request))
{
// If it's not within what our view is ignore it...
if (($row['id_msg'] == $row['id_first_msg'] && $context['current_view'] != 'topics') || ($row['id_msg'] != $row['id_first_msg'] && $context['current_view'] != 'replies'))
continue;

$can_add = false;
// If we're approving this is simple.
if ($curAction == 'approve' && ($any_array == array(0) || in_array($row['id_board'], $any_array)))
{
$can_add = true;
}
// Delete requires more permission checks...
elseif ($curAction == 'delete')
{
// Own post is easy!
if ($row['id_member'] == $user_info['id'] && ($delete_own_boards == array(0) || in_array($row['id_board'], $delete_own_boards)))
$can_add = true;
// Is it a reply to their own topic?
elseif ($row['id_member'] == $row['id_member_started'] && $row['id_msg'] != $row['id_first_msg'] && ($delete_own_replies == array(0) || in_array($row['id_board'], $delete_own_replies)))
$can_add = true;
// Someone elses?
elseif ($row['id_member'] != $user_info['id'] && ($delete_any_boards == array(0) || in_array($row['id_board'], $delete_any_boards)))
$can_add = true;
}

if ($can_add)
$anItem = $context['current_view'] == 'topics' ? $row['id_topic'] : $row['id_msg'];
$toAction[] = $anItem;

// All clear. What have we got now, what, what?
$details[$anItem] = array();
$details[$anItem]["subject"] = $row['subject'];
$details[$anItem]["topic"] = $row['id_topic'];
$details[$anItem]["member"] = ($context['current_view'] == 'topics') ? $row['id_member_started'] : $row['id_member'];
$details[$anItem]["board"] = $row['id_board'];
}
$smcFunc['db_free_result']($request);


So, if $can_add is false, $anItem is not defined.
Additionally, starting from the 2nd loop, if $can_add is false, the value of $anItem remains the one of the previous loop overwriting the details.

Am I reading it wrong?

[Shambles]

I only see one loop...

[emanuele]

I mean starting from the second round of the loop.

[MrPhil]

Unless $anItem was set to something before this snippet of code, it certainly looks to me like it will be undefined if $can_add is false. Also, that it will keep its old value in later loops whenever $can_add goes false.

[Arantor]

Yup, definitely should more stuff attached to $can_add's test there.

[emanuele]

Looking at the code I think a couple of brackets would be enough (from the if up to the end) and should not break anything.