Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-40191

Switching role to student when viewing a hidden resource produces 'Coding error detected...' error

    Details

    • Testing Instructions:
      Hide
      1. Create course with several activities
      2. Make some activities unavailable: completely hidden, with conditional access, with time restrictions, with group restrictions
      3. Keep the page open so you can copy the URLs
      4. Switch role to student and make sure you can access any of the unavailable activities by typing the URL, make sure the activity name appears in the naviagation
      5. Login as a student and make sure you can not access any of the unavailable activities by typing the URL, make sure there is no navigation in both standard and clean theme
      Show
      Create course with several activities Make some activities unavailable: completely hidden, with conditional access, with time restrictions, with group restrictions Keep the page open so you can copy the URLs Switch role to student and make sure you can access any of the unavailable activities by typing the URL, make sure the activity name appears in the naviagation Login as a student and make sure you can not access any of the unavailable activities by typing the URL, make sure there is no navigation in both standard and clean theme
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      wip-MDL-40191-master

      Description

      To reproduce (as Admin, Manager or Teacher)

      1. Create a page resource and put some stuff in it (anything)
      2. Set 'visible' to 'Hide'
      3. Save and display the resource
      4. In the Administration block, select 'Switch role to...' and pick 'Student' (or any role that should not see this resource)

      Result is 'Coding error detected, it must be fixed by programmer' error. With debugging on trace is:

      Debug info: Argument 3 passed to global_navigation::load_activity()
      must be an instance of navigation_node, boolean given, called in
      [dirroot]/lib/navigationlib.php on line 1204 and defined
      Error code: codingerror
      Stack trace:
       
      line 406 of /lib/setuplib.php: coding_exception thrown
      line 2018 of /lib/navigationlib.php: call to default_error_handler()
      line 1204 of /lib/navigationlib.php: call to global_navigation->load_activity()
      line 2936 of /lib/navigationlib.php: call to global_navigation->initialise()
      line 766 of /lib/pagelib.php: call to navbar->has_items()
      line 32 of /theme/wset25/layout/general.php: call to moodle_page->has_navbar()
      line 847 of /lib/outputrenderers.php: call to include()
      line 777 of /lib/outputrenderers.php: call to
      core_renderer->render_page_layout()
      line ? of unknownfile: call to core_renderer->header()
      line 1479 of /lib/setuplib.php: call to call_user_func_array()
      line 79 of /mod/page/view.php: call to bootstrap_renderer->__call()
      line 79 of /mod/page/view.php: call to bootstrap_renderer->header()
      

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              howardsmiller Howard Miller added a comment -

              Quick analysis...

              Essentially, line 1204 in lib/navigationlib.php calls

              $this->load_activity($cm, $course, $activity);

              $activity is generated by this line...

              $activity = $coursenode->find(....)

              The find function is capable of return 'false' if nothing is found and this condition is not checked before calling load_activity(..). A check needs to be added that does something sensible in this case.

              Show
              howardsmiller Howard Miller added a comment - Quick analysis... Essentially, line 1204 in lib/navigationlib.php calls $this->load_activity($cm, $course, $activity); $activity is generated by this line... $activity = $coursenode->find(....) The find function is capable of return 'false' if nothing is found and this condition is not checked before calling load_activity(..). A check needs to be added that does something sensible in this case.
              Hide
              salvetore Michael de Raadt added a comment -

              Thanks for reporting that, Howard.

              Show
              salvetore Michael de Raadt added a comment - Thanks for reporting that, Howard.
              Hide
              jfbelisle Jean-Francois Belisle added a comment - - edited

              Exactly same problem here:

              When taking the student role in an assignment

              Debug info: Argument 3 passed to global_navigation::load_activity() must be an instance of navigation_node, boolean given, called in [dirroot]/lib/navigationlib.php on line 1205 and defined
              Error code: codingerror
              Stack trace:
              line 406 of /lib/setuplib.php: coding_exception thrown
              line 2019 of /lib/navigationlib.php: call to default_error_handler()
              line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity()
              line 2937 of /lib/navigationlib.php: call to global_navigation->initialise()
              line 783 of /lib/pagelib.php: call to navbar->has_items()
              line 6 of /theme/sherbrooke/layout/general.php: call to moodle_page->has_navbar()
              line 847 of /lib/outputrenderers.php: call to include()
              line 777 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
              line 216 of /mod/assign/renderer.php: call to core_renderer->header()
              line 215 of /lib/outputrenderers.php: call to mod_assign_renderer->render_assign_header()
              line 3559 of /mod/assign/locallib.php: call to plugin_renderer_base->render()
              line 494 of /mod/assign/locallib.php: call to assign->view_submission_page()
              line 53 of /mod/assign/view.php: call to assign->view()
              

              Show
              jfbelisle Jean-Francois Belisle added a comment - - edited Exactly same problem here: When taking the student role in an assignment Debug info: Argument 3 passed to global_navigation::load_activity() must be an instance of navigation_node, boolean given, called in [dirroot]/lib/navigationlib.php on line 1205 and defined Error code: codingerror Stack trace: line 406 of /lib/setuplib.php: coding_exception thrown line 2019 of /lib/navigationlib.php: call to default_error_handler() line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity() line 2937 of /lib/navigationlib.php: call to global_navigation->initialise() line 783 of /lib/pagelib.php: call to navbar->has_items() line 6 of /theme/sherbrooke/layout/general.php: call to moodle_page->has_navbar() line 847 of /lib/outputrenderers.php: call to include() line 777 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 216 of /mod/assign/renderer.php: call to core_renderer->header() line 215 of /lib/outputrenderers.php: call to mod_assign_renderer->render_assign_header() line 3559 of /mod/assign/locallib.php: call to plugin_renderer_base->render() line 494 of /mod/assign/locallib.php: call to assign->view_submission_page() line 53 of /mod/assign/view.php: call to assign->view()
              Hide
              jfbelisle Jean-Francois Belisle added a comment -

              Hi,

              I added a quick and dirty workaround to prevent at least the error message for our teachers

              at line 1204 of /lib/navigationlib.php

              if ($activity){
                  // Finally load the cm specific navigaton information
                  $this->load_activity($cm, $course, $activity);
                  // Check if we have an active ndoe
                      if (!$activity->contains_active_node() && !$activity->search_for_active_node()) {
                           // And make the activity node active.
                           $activity->make_active();
                      }
              }else{
                  echo "<H1 style='color:red'>****Avec le rôle d'étudiant, vous ne devriez pas être en mesure de voir cette activité car elle est masquée!****</h1>";
              }

              Show
              jfbelisle Jean-Francois Belisle added a comment - Hi, I added a quick and dirty workaround to prevent at least the error message for our teachers at line 1204 of /lib/navigationlib.php if ($activity){ // Finally load the cm specific navigaton information $this->load_activity($cm, $course, $activity); // Check if we have an active ndoe if (!$activity->contains_active_node() && !$activity->search_for_active_node()) { // And make the activity node active. $activity->make_active(); } }else{ echo "<H1 style='color:red'>****Avec le rôle d'étudiant, vous ne devriez pas être en mesure de voir cette activité car elle est masquée!****</h1>"; }
              Hide
              mohamed.alsharaf Mohamed Alsharaf added a comment -

              I found this bug not a problem with the resource module or the navigation class, but a bug exists within the required_login()

              The function "required_login" must prevent an access to hidden activity. The code for that is in the function but it is not always executed, which result to the end of the function without an exception to prevent the user from viewing the page.

              Then the navigation class attempts to generate the page nav but the course module is inaccessible (hidden activity), which result to the navigation exception.

              You can find my fix for this issue in the following branches:

              2.5 Branch: wip-MDL-40191-MOODLE_25_STABLE
              https://github.com/satrun77/moodle/compare/MOODLE_25_STABLE...wip-MDL-40191-MOODLE_25_STABLE

              2.4 Branch: wip-MDL-40191-MOODLE_24_STABLE
              https://github.com/satrun77/moodle/compare/MOODLE_24_STABLE...wip-MDL-40191-MOODLE_24_STABLE

              master Branch: wip-MDL-40191-master
              https://github.com/satrun77/moodle/compare/master...wip-MDL-40191-master

              Show
              mohamed.alsharaf Mohamed Alsharaf added a comment - I found this bug not a problem with the resource module or the navigation class, but a bug exists within the required_login() The function "required_login" must prevent an access to hidden activity. The code for that is in the function but it is not always executed, which result to the end of the function without an exception to prevent the user from viewing the page. Then the navigation class attempts to generate the page nav but the course module is inaccessible (hidden activity), which result to the navigation exception. You can find my fix for this issue in the following branches: 2.5 Branch: wip- MDL-40191 -MOODLE_25_STABLE https://github.com/satrun77/moodle/compare/MOODLE_25_STABLE...wip-MDL-40191-MOODLE_25_STABLE 2.4 Branch: wip- MDL-40191 -MOODLE_24_STABLE https://github.com/satrun77/moodle/compare/MOODLE_24_STABLE...wip-MDL-40191-MOODLE_24_STABLE master Branch: wip- MDL-40191 -master https://github.com/satrun77/moodle/compare/master...wip-MDL-40191-master
              Hide
              rbon Ray Bon added a comment - - edited

              I have tracked this down to a check made in lib/moodlelib.php
              There is a check if current user is a site admin. If user is an admin, it returns. This occurs before any processing takes place for hidden resource(s) etc.
              The following code change skips admin check if admin is acting in a different role.

              sesskey();

              // Do not bother admins with any formalities
              // But if admin is acting as another role, continue as that role
              if (!is_role_switched($course->id) and is_siteadmin()) {
              //set accesstime or the user will appear offline which messes up messaging
              user_accesstime_log($course->id);
              return;

              Show
              rbon Ray Bon added a comment - - edited I have tracked this down to a check made in lib/moodlelib.php There is a check if current user is a site admin. If user is an admin, it returns. This occurs before any processing takes place for hidden resource(s) etc. The following code change skips admin check if admin is acting in a different role. sesskey(); // Do not bother admins with any formalities // But if admin is acting as another role, continue as that role if (!is_role_switched($course->id) and is_siteadmin()) { //set accesstime or the user will appear offline which messes up messaging user_accesstime_log($course->id); return;
              Hide
              mwebster Mark van Hoek added a comment - - edited

              We've reproduced this fatal error below in Moodle 2.4.6+ (Build: 20131018) when logged in as a real student role trying to view a hidden quiz:
              The url is mod/quiz/view.php?id=8227
              Activity completion settings are used:
              -Completion tracking = Show activity as complete when conditions are met
              -checked Student must receive a grade to complete this activity
              -Expect complete on: 13 Sep 2013 checked Enable

              Our particular error was:
              Coding error detected, it must be fixed by a programmer: PHP catchable fatal error Debug: Argument 3 passed to global_navigation::load_activity() must be an instance of navigation_node, boolean given, called in [dirroot]/lib/navigationlib.php on line 1205 and defined\nError code: codingerror\n* line 406 of /lib/setuplib.php: coding_exception thrown\n* line 2020 of /lib/navigationlib.php: call to default_error_handler()\n* line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity()\n* line 3018 of /lib/navigationlib.php: call to global_navigation->initialise()\n* line 766 of /lib/pagelib.php: call to navbar->has_items()\n* line 21 of /theme/uvic/layout/general.php: call to moodle_page->has_navbar()\n* line 810 of /lib/outputrenderers.php: call to include()\n* line 740 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()\n* line 686 of /lib/outputrenderers.php: call to core_renderer->header()\n* line ? of unknownfile: call to core_renderer->redirect_message()\n* line 1460 of /lib/setuplib.php: call to call_user_func_array()\n* line 2536 of /lib/weblib.php: call to bootstrap_renderer->__call()\n* line 2536 of /lib/weblib.php: call to bootstrap_renderer->redirect_message()\n* line 3094 of /lib/moodlelib.php: call to redirect()\n* line 56 of /mod/quiz/view.php: call to require_login()\n

              Show
              mwebster Mark van Hoek added a comment - - edited We've reproduced this fatal error below in Moodle 2.4.6+ (Build: 20131018) when logged in as a real student role trying to view a hidden quiz: The url is mod/quiz/view.php?id=8227 Activity completion settings are used: -Completion tracking = Show activity as complete when conditions are met -checked Student must receive a grade to complete this activity -Expect complete on: 13 Sep 2013 checked Enable Our particular error was: Coding error detected, it must be fixed by a programmer: PHP catchable fatal error Debug: Argument 3 passed to global_navigation::load_activity() must be an instance of navigation_node, boolean given, called in [dirroot] /lib/navigationlib.php on line 1205 and defined\nError code: codingerror\n* line 406 of /lib/setuplib.php: coding_exception thrown\n* line 2020 of /lib/navigationlib.php: call to default_error_handler()\n* line 1205 of /lib/navigationlib.php: call to global_navigation->load_activity()\n* line 3018 of /lib/navigationlib.php: call to global_navigation->initialise()\n* line 766 of /lib/pagelib.php: call to navbar->has_items()\n* line 21 of /theme/uvic/layout/general.php: call to moodle_page->has_navbar()\n* line 810 of /lib/outputrenderers.php: call to include()\n* line 740 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()\n* line 686 of /lib/outputrenderers.php: call to core_renderer->header()\n* line ? of unknownfile: call to core_renderer->redirect_message()\n* line 1460 of /lib/setuplib.php: call to call_user_func_array()\n* line 2536 of /lib/weblib.php: call to bootstrap_renderer->__call()\n* line 2536 of /lib/weblib.php: call to bootstrap_renderer->redirect_message()\n* line 3094 of /lib/moodlelib.php: call to redirect()\n* line 56 of /mod/quiz/view.php: call to require_login()\n
              Hide
              hedgesg Gareth Hedges added a comment -

              We are seeing this error instead of the 'sorry this page is currently hidden' when a student is linked directly to a hidden/restricted activity or resource. It happens both as an actual student and if you switch your role to student. The error is a little disconcerting to students and is causing them to panic a little!

              Version: 2.4.4+ (Build: 20130530)

              Show
              hedgesg Gareth Hedges added a comment - We are seeing this error instead of the 'sorry this page is currently hidden' when a student is linked directly to a hidden/restricted activity or resource. It happens both as an actual student and if you switch your role to student. The error is a little disconcerting to students and is causing them to panic a little! Version: 2.4.4+ (Build: 20130530)
              Hide
              marina Marina Glancy added a comment -

              In 2.6 & 2.7 there is already a commit from David Mudrak that solves this issue. It was integrated as part of another issue. In my branches I backported it to 2.4 and 2.5.

              Also I've added commit that always creates navigation item for module even when it is not visible that will not be displayed unless requested directly.
              Ready for peer review. Maybe Sam Hemelryk can look at it?

              Show
              marina Marina Glancy added a comment - In 2.6 & 2.7 there is already a commit from David Mudrak that solves this issue. It was integrated as part of another issue. In my branches I backported it to 2.4 and 2.5. Also I've added commit that always creates navigation item for module even when it is not visible that will not be displayed unless requested directly. Ready for peer review. Maybe Sam Hemelryk can look at it?
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Marina looks spot on - I've pushed this to integration review now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Marina looks spot on - I've pushed this to integration review now.
              Hide
              marina Marina Glancy added a comment -

              Thanks Sam

              Show
              marina Marina Glancy added a comment - Thanks Sam
              Hide
              poltawski Dan Poltawski added a comment -

              Integrated to master, 26, 25 and 24 - thanks Marina

              Show
              poltawski Dan Poltawski added a comment - Integrated to master, 26, 25 and 24 - thanks Marina
              Hide
              salvetore Michael de Raadt added a comment -

              I've created 2.6 instances ready for testing. I should be able to start testing of this issue after lunch.

              Show
              salvetore Michael de Raadt added a comment - I've created 2.6 instances ready for testing. I should be able to start testing of this issue after lunch.
              Hide
              salvetore Michael de Raadt added a comment -

              I tested this in 2.4, 2.5, 2.6 and master.

              I found one problem. In 2.5, when I attempted to access an activity hidden on the course page while I was switched to view as a student, I was given the message "Sorry, this activity is currently hidden". In other versions, the activity appeared under these circumstances.

              Show
              salvetore Michael de Raadt added a comment - I tested this in 2.4, 2.5, 2.6 and master. I found one problem. In 2.5, when I attempted to access an activity hidden on the course page while I was switched to view as a student, I was given the message "Sorry, this activity is currently hidden". In other versions, the activity appeared under these circumstances.
              Hide
              salvetore Michael de Raadt added a comment -

              Actually, this may be due to the mix of activities I used in different versions. It looks like Forums misbehave across versions. This may be more due to activities than versions. I'll try each activity and report back.

              Show
              salvetore Michael de Raadt added a comment - Actually, this may be due to the mix of activities I used in different versions. It looks like Forums misbehave across versions. This may be more due to activities than versions. I'll try each activity and report back.
              Hide
              marina Marina Glancy added a comment -

              Michael, I looked in the code and found that some activity types (feedback, forum, glossary and survey) have an explicit check for module visibility AFTER calling require_login() which probably remains since old times because require_login() is supposed to check it. We should exclude those activity from "completely hidden" test.

              Show
              marina Marina Glancy added a comment - Michael, I looked in the code and found that some activity types (feedback, forum, glossary and survey) have an explicit check for module visibility AFTER calling require_login() which probably remains since old times because require_login() is supposed to check it. We should exclude those activity from "completely hidden" test.
              Hide
              salvetore Michael de Raadt added a comment -

              Most activities behave as expected.

              • Feedback, Forum, Glossary and Survey show "Sorry, this activity is currently hidden".
              • The Folder resource shows no content.
              • The Lesson module throws errors.
              Show
              salvetore Michael de Raadt added a comment - Most activities behave as expected. Feedback, Forum, Glossary and Survey show "Sorry, this activity is currently hidden". The Folder resource shows no content. The Lesson module throws errors.
              Hide
              marina Marina Glancy added a comment -

              I suppose I was the one who introduced this problem with folder in https://github.com/moodle/moodle/commit/38199c24
              For some reason I thought it'd be cool to add the capability check in the renderer
              It does not happen in 2.4, only 2.5 and above.

              Regarding lesson, I was able to reproduce it too:

              Error: could not find lesson_timer records
               
              More information about this error
              Debug info:
              Error code: cannotfindtimer
              Stack trace:
               
                  line 476 of /lib/setuplib.php: moodle_exception thrown
                  line 1216 of /mod/lesson/locallib.php: call to print_error()
                  line 301 of /mod/lesson/view.php: call to lesson->update_timer()

              Show
              marina Marina Glancy added a comment - I suppose I was the one who introduced this problem with folder in https://github.com/moodle/moodle/commit/38199c24 For some reason I thought it'd be cool to add the capability check in the renderer It does not happen in 2.4, only 2.5 and above. Regarding lesson, I was able to reproduce it too: Error: could not find lesson_timer records   More information about this error Debug info: Error code: cannotfindtimer Stack trace:   line 476 of /lib/setuplib.php: moodle_exception thrown line 1216 of /mod/lesson/locallib.php: call to print_error() line 301 of /mod/lesson/view.php: call to lesson->update_timer()
              Hide
              marina Marina Glancy added a comment -

              I have a fix for folder:

              git checkout master
              git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-master
               
              git checkout MOODLE_26_STABLE
              git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-m26
               
              git checkout MOODLE_25_STABLE
              git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-m25

              https://github.com/marinaglancy/moodle/compare/moodle:MOODLE_26_STABLE...wip-MDL-40191-2-m26

              Additional testing instructions:

              1. Create course with two folders, one displayed on separate page, one on course page
              2. Make folders unavailable for students (hidden or conditionally unavailable)
              3. Open the first folder
              4. Switch role to student, make sure you can see the folder contents
              5. Go to course page, make sure you can not see the inline folder
              6. Login as student
              7. Make sure you can not see either of the folders, even when entering URL directly
              Show
              marina Marina Glancy added a comment - I have a fix for folder: git checkout master git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-master   git checkout MOODLE_26_STABLE git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-m26   git checkout MOODLE_25_STABLE git pull git://github.com/marinaglancy/moodle.git wip-MDL-40191-2-m25 https://github.com/marinaglancy/moodle/compare/moodle:MOODLE_26_STABLE...wip-MDL-40191-2-m26 Additional testing instructions: Create course with two folders, one displayed on separate page, one on course page Make folders unavailable for students (hidden or conditionally unavailable) Open the first folder Switch role to student, make sure you can see the folder contents Go to course page, make sure you can not see the inline folder Login as student Make sure you can not see either of the folders, even when entering URL directly
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks Marina - i've pulled those fixes into master, 26 and 25

              Show
              poltawski Dan Poltawski added a comment - Thanks Marina - i've pulled those fixes into master, 26 and 25
              Hide
              salvetore Michael de Raadt added a comment - - edited

              The folder module works nicely now.

              Show
              salvetore Michael de Raadt added a comment - - edited The folder module works nicely now.
              Hide
              marina Marina Glancy added a comment -

              I created an issue MDL-43217 about hidden modules but I still don't think we should resolve it in stable versions

              Show
              marina Marina Glancy added a comment - I created an issue MDL-43217 about hidden modules but I still don't think we should resolve it in stable versions
              Hide
              poltawski Dan Poltawski added a comment -

              Can we pass this?

              Show
              poltawski Dan Poltawski added a comment - Can we pass this?
              Hide
              salvetore Michael de Raadt added a comment -

              Test result: Passed

              With the showing of Forum, Feedback, Glossary and Survey being handled in MDL-43217, and with the Folder viewing fixed, I think we can pass this issue. Certainly the original problem has been resolved.

              Show
              salvetore Michael de Raadt added a comment - Test result: Passed With the showing of Forum, Feedback, Glossary and Survey being handled in MDL-43217 , and with the Folder viewing fixed, I think we can pass this issue. Certainly the original problem has been resolved.
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks for your contributions, this change is now upstream!

              “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

              Show
              poltawski Dan Poltawski added a comment - Thanks for your contributions, this change is now upstream! “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    13/Jan/14