429793 - Bug in get_permissions causes all pages to break after cache clear

Status: RESOLVED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Jeremy Orem [:oremj]]

When this function is called:

tiki-setup_base.php Line 461:

        $allperms = $userlib->get_permissions(0, -1, 'permName_desc', '', '');

lib/userslib.php Line 1702:

        if ($type) { <- $type is '' 
            $mid = ' where `type`= ? ';
            $values[] = $type;
        } else { <- runs this code
            // Mozilla opt: this index is not getting used 
            // (and it should be; otherwise the order by uses filesort
            $mid = ' force index (permName) '; <- this sets up $mid
        }


            if ($mid) { <- $mid is set above or true
                $mid .= " and `permName` > ''"; <- $mid appends this
            } else { 
                $mid .= " where `permName` > ''";
            }

Code ends up generating this query:

select * from `users_permissions` force index (permName) and `permName` > '' order by `permName` desc;

Which of course is invalid and ends up displaying an error on the front page.

Comment 1

[Jeremy Orem [:oremj]]

Running svn blame it looks like the bug was introduced in revision 12313:

 12313 [e-mail]     } else {
 12313 [e-mail]             // Mozilla opt: this index is not getting used 
 12313 [e-mail]             // (and it should be; otherwise the order by uses filesort
 12313 [e-mail]             $mid = ' force index (permName) ';
 12313 [e-mail]         }

Comment 2

[Wil Clouser [:clouserw]]

Attachment: fix problem

Sounds like the problem is we're putting stuff in $mid.  Instead of overloading $mid, this patch adds a $bonus to the query.

Comment 3

[Jeremy Orem [:oremj]]

Comment on attachment 316547 [details] [diff] [review]
fix problem

Looks like that would fix the problem.

Comment 4

[Reed Loden [:reed]]

Comment on attachment 316547 [details] [diff] [review]
fix problem

>--- userslib.php	2008-04-16 12:45:39.000000000 -0700
>+++ userslib2.php	2008-04-18 23:21:55.000000000 -0700
>@@ -1699,13 +1699,14 @@
> 	$values = array();
> 	$sort_mode = $this->convert_sortmode($sort_mode);
> 	$mid = '';
>+    $bonus = '';

Spaces vs. tabs.

Comment 5

[Wil Clouser [:clouserw]]

r12400.  Hopefully that variable isn't the straw that breaks this bloated camel's back.

Comment 6

[Michael Morgan [:morgamic]]

Thanks guys -- will figure out why this wasn't caught earlier.

Comment 7

[Louis-Philippe Huberdeau]

Seems to be sumo-only from the discussion.

Comment 8

[David Tenser [:djst]]

morgamic: anything here that should be upstreamed?