Moodle
  1. Moodle
  2. MDL-40191

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

    Details

    • Rank:
      50938

      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()
      

        Issue Links

          Activity

          Hide
          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
          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
          Michael de Raadt added a comment -

          Thanks for reporting that, Howard.

          Show
          Michael de Raadt added a comment - Thanks for reporting that, Howard.
          Hide
          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
          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
          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
          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 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 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
          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
          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
          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
          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
          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
          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 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 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
          Sam Hemelryk added a comment -

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

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

          Thanks Sam

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

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

          Show
          Dan Poltawski added a comment - Integrated to master, 26, 25 and 24 - thanks Marina
          Hide
          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
          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
          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
          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
          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
          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 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 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
          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
          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 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 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 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 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
          Dan Poltawski added a comment -

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

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

          The folder module works nicely now.

          Show
          Michael de Raadt added a comment - - edited The folder module works nicely now.
          Hide
          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 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
          Dan Poltawski added a comment -

          Can we pass this?

          Show
          Dan Poltawski added a comment - Can we pass this?
          Hide
          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
          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
          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
          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:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: