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 2.6 Branch:
      wip-MDL-40191-m26
    • 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

          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: