Moodle
  1. Moodle
  2. MDL-10090

Infinite Loop in load_user_capability

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.8.1
    • Fix Version/s: None
    • Component/s: Unknown
    • Labels:
      None
    • Environment:
      IIS 6.0, PHP 4.4.4, IMAP Authentication System, Windows Server 2003
    • Rank:
      1809

      Description

      Updated to the newest version of moodle today, logged in, then recieved the following page.

      -------------------------------------------------------------------------------------------------------------------------------------------

      <div id="page">

      <!-- END OF HEADER -->
      <div id="content">
      <br /><div class="box errorbox errorboxcontent">Query failed in load_user_capability.</div><div class="continuebutton"><div class="singlebutton"><form action="http://moodle.paston.ac.uk/" method="post"><div><input type="submit" value="Continue" /></div></form></div></div>

      </div> <!-- end div containerContent -->

      <!-- START OF FOOTER -->
      <div id="footer">

      <p class="helplink"><br /><div class="box errorbox errorboxcontent">Query failed in load_user_capability.</div><div class="continuebutton"><div class="singlebutton"><form action="http://moodle.paston.ac.uk/" method="post"><div><input type="submit" value="Continue" /></div></form></div></div>

      </div> <!-- end div containerContent -->
      <!-- START OF FOOTER -->
      <div id="footer">

      <p class="helplink"><br /><div class="box errorbox errorboxcontent">Query failed in load_user_capability.</div><div class="continuebutton"><div class="singlebutton"><form action="http://moodle.paston.ac.uk/" method="post"><div><input type="submit" value="Continue" /></div></form></div></div>

      </div> <!-- end div containerContent -->
      <!-- START OF FOOTER -->

      --------------------------------------------------------------------------------------------------------------------

      As you can see, the code has gotten into an inifnite loop, the code continues to flow in until I hit stop on my browser window. the first line on the page says "PHP has encountered a Stack overflow", but I do not see how this is possible, I have assigned 128Mb of memory to the script, so it shouldn't overflow perticularly easily.

      PHPINFO() output can be seen at http://moodle.paston.ac.uk/meh.php

      I have managed to get the source of the error down to this line.

      if (!$rs = get_recordset_sql($SQL1))

      { error("Query failed in load_user_capability."); }

      I am completely unable to find the root of the problem other than pinning down that line of code.

      I hope you can help me, my whole moodle system is down at the moment, and we need access to it to do coursework over the next month.

        Activity

        Hide
        Matthew Davidson added a comment - - edited

        This is a major issue for us.....the entire website becomes unresponsive if one person comes upon this error.

        This seems to be the call that causes the hangup. In user/view.php

        if ($USER->id != $user->id && empty($USER->realuser) && has_capability('moodle/user:loginas', $coursecontext) &&
        ! has_capability('moodle/site:doanything', $coursecontext, $user->id, false)) {

        it loads the viewed user's entire capabilities list, then finds out if that person is a moodle/site;doanything capable user. Seems like lifting a house off the ground to get a couch out.

        Show
        Matthew Davidson added a comment - - edited This is a major issue for us.....the entire website becomes unresponsive if one person comes upon this error. This seems to be the call that causes the hangup. In user/view.php if ($USER->id != $user->id && empty($USER->realuser) && has_capability('moodle/user:loginas', $coursecontext) && ! has_capability('moodle/site:doanything', $coursecontext, $user->id, false)) { it loads the viewed user's entire capabilities list, then finds out if that person is a moodle/site;doanything capable user. Seems like lifting a house off the ground to get a couch out.
        Hide
        Martyn J Compton added a comment -

        It does seem a little OTT...

        I eventually found out what the problem was on our end, It was to do with this piece of code. We didnt have enough memory to run the whole query when we had a heavy load on the server, then, as you said, the entire site became unresponsive...

        We restarted the server, then the swarm of browsers caused the same error, a memory upgrade fixed the problem.

        Show
        Martyn J Compton added a comment - It does seem a little OTT... I eventually found out what the problem was on our end, It was to do with this piece of code. We didnt have enough memory to run the whole query when we had a heavy load on the server, then, as you said, the entire site became unresponsive... We restarted the server, then the swarm of browsers caused the same error, a memory upgrade fixed the problem.
        Hide
        Ryan Smith added a comment - - edited

        I work with Matthew that posted above about this issue. How much memory are you running? We have 2 GB, and only had 3 or 4 users online when this was happening. We had around 1.5 GB of memory free. BTW, we're running Server 2003, Apache 2.2.4, MySQL 5.0.41, and PHP 5.2.3.

        Matthew edited the offending code out, so admins on our site can no longer "Login-as" for the time being.

        Show
        Ryan Smith added a comment - - edited I work with Matthew that posted above about this issue. How much memory are you running? We have 2 GB, and only had 3 or 4 users online when this was happening. We had around 1.5 GB of memory free. BTW, we're running Server 2003, Apache 2.2.4, MySQL 5.0.41, and PHP 5.2.3. Matthew edited the offending code out, so admins on our site can no longer "Login-as" for the time being.
        Hide
        Matthew Davidson added a comment - - edited

        Maybe a function is needed that just returns whether another user has a certain permission is a given context. Maybe that function already exists but if not...maybe this one will work.

        function user_has_capability($capability, $context, $userid)
        {
        global $CFG;
        $result = false;
        $sitecontext = get_context_instance(CONTEXT_SYSTEM);

        if($context->instanceid == SITEID) $context = $sitecontext;

        $SQL = 'select * from '.$CFG->prefix.'role_assignments ra, '.$CFG->prefix.'role r where ra.userid='.$userid.' and ra.contextid='.$context->id.' and ra.roleid = r.id';
        if ($roles = get_records_sql($SQL))
        {
        foreach ($roles as $userrole)
        {
        $siteadmin = false;
        if($capability != 'moodle/site:doanything')

        { if(user_has_capability('moodle/site:doanything',$sitecontext,$userid)) $result = true; if(user_has_capability('moodle/site:doanything',$context,$userid)) $result = true; if($result) return true; }

        $reg_role = get_record('role_capabilities', 'roleid' ,$userrole->roleid, 'capability', $capability, 'contextid', $sitecontext->id);

        if($reg_role->permission == 1)

        { $override = get_record('role_capabilities', 'roleid' ,$userrole->roleid, 'capability', $capability, 'contextid', $context->id); if(!$override) $result = true; elseif($override->permission != 1) $result = false; else $result = true; }

        }
        }
        elseif(user_has_capability('moodle/site:doanything',$sitecontext,$userid)) $result = true;
        else $result = false;

        return $result;
        }

        I placed that function in the accesslib.php file and then
        I changed my user/view.php to say this

        if ($USER->id != $user->id && empty($USER->realuser) && has_capability('moodle/user:loginas', $coursecontext) &&
        ! user_has_capability('moodle/site:doanything', $coursecontext, $user->id)) {

        Show
        Matthew Davidson added a comment - - edited Maybe a function is needed that just returns whether another user has a certain permission is a given context. Maybe that function already exists but if not...maybe this one will work. function user_has_capability($capability, $context, $userid) { global $CFG; $result = false; $sitecontext = get_context_instance(CONTEXT_SYSTEM); if($context->instanceid == SITEID) $context = $sitecontext; $SQL = 'select * from '.$CFG->prefix.'role_assignments ra, '.$CFG->prefix.'role r where ra.userid='.$userid.' and ra.contextid='.$context->id.' and ra.roleid = r.id'; if ($roles = get_records_sql($SQL)) { foreach ($roles as $userrole) { $siteadmin = false; if($capability != 'moodle/site:doanything') { if(user_has_capability('moodle/site:doanything',$sitecontext,$userid)) $result = true; if(user_has_capability('moodle/site:doanything',$context,$userid)) $result = true; if($result) return true; } $reg_role = get_record('role_capabilities', 'roleid' ,$userrole->roleid, 'capability', $capability, 'contextid', $sitecontext->id); if($reg_role->permission == 1) { $override = get_record('role_capabilities', 'roleid' ,$userrole->roleid, 'capability', $capability, 'contextid', $context->id); if(!$override) $result = true; elseif($override->permission != 1) $result = false; else $result = true; } } } elseif(user_has_capability('moodle/site:doanything',$sitecontext,$userid)) $result = true; else $result = false; return $result; } I placed that function in the accesslib.php file and then I changed my user/view.php to say this if ($USER->id != $user->id && empty($USER->realuser) && has_capability('moodle/user:loginas', $coursecontext) && ! user_has_capability('moodle/site:doanything', $coursecontext, $user->id)) {
        Hide
        Martyn J Compton added a comment -

        We were unfortunately being stupid and only running 512 Mb of RAM on a relatively old machine, we had about 40-50 clients all connect in a matter of about 2 seconds... kinda heavy workload for such an old server.

        Show
        Martyn J Compton added a comment - We were unfortunately being stupid and only running 512 Mb of RAM on a relatively old machine, we had about 40-50 clients all connect in a matter of about 2 seconds... kinda heavy workload for such an old server.
        Hide
        Matthew Davidson added a comment -

        Sorry to edit this so many times.....it is correctly returning true or false to a users capability in a certain context...taking overrides into consideration. Please consider this as a possible solution to the load_capabilities problem.

        Show
        Matthew Davidson added a comment - Sorry to edit this so many times.....it is correctly returning true or false to a users capability in a certain context...taking overrides into consideration. Please consider this as a possible solution to the load_capabilities problem.
        Hide
        Yu Zhang added a comment -

        Hi Matthew,

        Thanks for the code, I think user_has_capability() is ignoring certain things.

        1) There is no check for prohibit which is supposed to override allows at lower contexts.
        2) Permissions are calculated by summing, e.g. if a user has 2 roles in a course, 1 role with capability set to allow, another set to prevent. They should cancel out.
        3) Roles assigned at higher level context is not taken into consideration, e.g. the student could have a role assigned at course category context level which has moodle/site:doanything.

        These points may not be very relevant for this particular case, since we are only checking for moodle/site:doanything. However checking other user's capability is used fairly often, e.g. forum cron checks each user capabilities before sending out emails. has_capability() already loads only the relevant capability, and searches only the relevant contexts for that user when calling load_user_capability().

        Cheers,

        Yu

        Show
        Yu Zhang added a comment - Hi Matthew, Thanks for the code, I think user_has_capability() is ignoring certain things. 1) There is no check for prohibit which is supposed to override allows at lower contexts. 2) Permissions are calculated by summing, e.g. if a user has 2 roles in a course, 1 role with capability set to allow, another set to prevent. They should cancel out. 3) Roles assigned at higher level context is not taken into consideration, e.g. the student could have a role assigned at course category context level which has moodle/site:doanything. These points may not be very relevant for this particular case, since we are only checking for moodle/site:doanything. However checking other user's capability is used fairly often, e.g. forum cron checks each user capabilities before sending out emails. has_capability() already loads only the relevant capability, and searches only the relevant contexts for that user when calling load_user_capability(). Cheers, Yu
        Hide
        Matthew Davidson added a comment -

        Ok, thanks for the help...I'm going to continue to develope the user_has_capability() function further, but I have some serious questions about point #2.

        This doesn't make much sense to me. Shouldn't moodle have a global setting like (default RESTRICTIVENESS to restrict or allow) meaning, if a user has 2 roles...either allow trumps prevent or vice versa depending on the global RESTRICTIVENESS setting and prohibit would trump all. I don't understand why I would want neither of the permissions, which is what you get when you sum. I would think that any set permission would trump a "not set" permission.

        Maybe you can help clear this up on how summing the permissions is a good theoretical way of doing roles. Thanks for your help.

        Matthew

        Show
        Matthew Davidson added a comment - Ok, thanks for the help...I'm going to continue to develope the user_has_capability() function further, but I have some serious questions about point #2. This doesn't make much sense to me. Shouldn't moodle have a global setting like (default RESTRICTIVENESS to restrict or allow) meaning, if a user has 2 roles...either allow trumps prevent or vice versa depending on the global RESTRICTIVENESS setting and prohibit would trump all. I don't understand why I would want neither of the permissions, which is what you get when you sum. I would think that any set permission would trump a "not set" permission. Maybe you can help clear this up on how summing the permissions is a good theoretical way of doing roles. Thanks for your help. Matthew
        Hide
        Yu Zhang added a comment - - edited

        Hi Matthew

        Summing is quite a loose term to use, when we resolve capabilities, we take into consideration of many things.

        For example, to find the capability of a user at course level, we first have to find the context parents of the course context.

        (top)-----------------------------------------------------> (bottom)
        System -> Course Cat A -> Course Cat B -> Course context

        First we check if prohibit is set at any parent categories, if it is set, then the user does not have that capabilities. We then check, from bottom up, the resolved capabilities at each level. There are a few rules,

        1) Overrides at a context level X will take precedence over capabilities from roles assigned at the same context
        e.g. normal student role allows user to post in forums, however, in teacher forums this is done by using an override to disallow users with student role to post in that particular forum instance.

        2) Roles assigned at lower context level will take precedence over roles assigned at higher context
        e.g. user with student role in course A has no right to delete other user's forum posts, but he is given a forum moderator role in a forum activity which would then allow him to delete posts.

        3) Same level roles and overrides are summed, if they cancel out, we go up in context level to find permission.
        e.g. student could not send bulk message to other users in a course, but the same user has another role called "the messenger" which allows bulk messaging. The capabilities are then summed as 0 (not set for student) + 1 (allow for the messenger), which then allows the student to send bulk messaging in this course.

        The advantage of summing is that it allows multiple roles in the same context to work nicely together. Please note that normally, if you don't want someone to be able to do something, you can simply set the capability to not set (value 0). You only use prevent (-1) when you really mean it. A high level override of prohibit should be used if you don't want the user to be able to do something in all lower contexts, e.g. a user with naughty student role assign at system context with forum:post set to prohibit will not be able to post in any forums. In most cases, summing would grant capabilities in stead of taking them away.

        For a full implementation you can read the comments in accesslib.php, particularly the load_user_capability() and has_capability() function. I think the current implementation of the mechanism to load other user's capability is complete, and we have made many improvements over the past few months, if there is anything you can think of that would help to speed this up, please let us know!

        Cheers,

        Yu

        Show
        Yu Zhang added a comment - - edited Hi Matthew Summing is quite a loose term to use, when we resolve capabilities, we take into consideration of many things. For example, to find the capability of a user at course level, we first have to find the context parents of the course context. (top)-----------------------------------------------------> (bottom) System -> Course Cat A -> Course Cat B -> Course context First we check if prohibit is set at any parent categories, if it is set, then the user does not have that capabilities. We then check, from bottom up, the resolved capabilities at each level. There are a few rules, 1) Overrides at a context level X will take precedence over capabilities from roles assigned at the same context e.g. normal student role allows user to post in forums, however, in teacher forums this is done by using an override to disallow users with student role to post in that particular forum instance. 2) Roles assigned at lower context level will take precedence over roles assigned at higher context e.g. user with student role in course A has no right to delete other user's forum posts, but he is given a forum moderator role in a forum activity which would then allow him to delete posts. 3) Same level roles and overrides are summed, if they cancel out, we go up in context level to find permission. e.g. student could not send bulk message to other users in a course, but the same user has another role called "the messenger" which allows bulk messaging. The capabilities are then summed as 0 (not set for student) + 1 (allow for the messenger), which then allows the student to send bulk messaging in this course. The advantage of summing is that it allows multiple roles in the same context to work nicely together. Please note that normally, if you don't want someone to be able to do something, you can simply set the capability to not set (value 0). You only use prevent (-1) when you really mean it. A high level override of prohibit should be used if you don't want the user to be able to do something in all lower contexts, e.g. a user with naughty student role assign at system context with forum:post set to prohibit will not be able to post in any forums. In most cases, summing would grant capabilities in stead of taking them away. For a full implementation you can read the comments in accesslib.php, particularly the load_user_capability() and has_capability() function. I think the current implementation of the mechanism to load other user's capability is complete, and we have made many improvements over the past few months, if there is anything you can think of that would help to speed this up, please let us know! Cheers, Yu
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this issue.

        We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported.

        If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed.

        Michael d;

        lqjjLKA0p6

        Show
        Michael de Raadt added a comment - Thanks for reporting this issue. We have detected that this issue has been inactive for over a year has been recorded as affecting versions that are no longer supported. If you believe that this issue is still relevant to current versions (2.1 and beyond), please comment on the issue. Issues left inactive for a further month will be closed. Michael d; lqjjLKA0p6
        Hide
        Michael de Raadt added a comment -

        I'm closing this issue as it has become inactive and does not appear to affect a current supported version. If you are encountering this problem or one similar, please launch a new issue.

        Show
        Michael de Raadt added a comment - I'm closing this issue as it has become inactive and does not appear to affect a current supported version. If you are encountering this problem or one similar, please launch a new issue.

          People

          • Votes:
            2 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: