Moodle
  1. Moodle
  2. MDL-27899

Moodle device selection causes error if $USER global is null

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      Temporarily edit your config.php to create a wwwrootmismatch error (you'll need two valid urls pointing for the same Moodle instance). If you don't get an error then the fix works.

      Show
      Temporarily edit your config.php to create a wwwrootmismatch error (you'll need two valid urls pointing for the same Moodle instance). If you don't get an error then the fix works.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull Master Branch:
      wip-MDL-27899-master
    • Rank:
      17803

      Description

      If Moodle displays a screen when the $USER global is null, this causes an error because this scenario is not checked for in the code. Only time this happens AFAIK is when a wwwrootmismatch error occurs, but presumably there are other errors.

      Debug info: Argument 1 passed to check_user_preferences_loaded() must be an instance of stdClass, null given, called in /fs1/www_root/ajf425/github/lib/moodlelib.php on line 1553 and defined
      Stack trace:

      line 359 of /lib/setuplib.php: coding_exception thrown
      line 1327 of /lib/moodlelib.php: call to default_error_handler()
      line 1553 of /lib/moodlelib.php: call to check_user_preferences_loaded()
      line 7764 of /lib/moodlelib.php: call to get_user_preferences()
      line 534 of /lib/pagelib.php: call to get_user_device_type()
      line 615 of /lib/pagelib.php: call to moodle_page->magic_get_devicetypeinuse()
      line 1335 of /lib/pagelib.php: call to moodle_page->__get()
      line 1257 of /lib/pagelib.php: call to moodle_page->resolve_theme()
      line 1244 of /lib/setuplib.php: call to moodle_page->initialise_theme_and_output()
      line 2495 of /lib/weblib.php: call to bootstrap_renderer->__call()
      line 2495 of /lib/weblib.php: call to bootstrap_renderer->redirect_message()
      line 676 of /lib/setuplib.php: call to redirect()
      line 656 of /lib/setup.php: call to initialise_fullme()
      line 29 of /config.php: call to require_once()
      line 31 of /index.php: call to require_once()

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Hi Anthony,

          I've just marked this needs more work, not because your solution doesn't work but because I suspect that this problem will likely arise again in future code, and that we may need to fix the ordering of things in setup.
          A solution to this would be to move the call to initialise_fullme within setuplib.php down a couple of lines so that it is after $USER is established.
          The reason being that that call to redirect for wwwrootmismatch is likely to lead to further errors like this occurring. That call to redirect with a message is always going to trigger the initialisation of the core page/theme stuff.

          I've added Eloy as a watcher here as that would of course be a serious change, so Eloy please can you let us know your thoughts.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Anthony, I've just marked this needs more work, not because your solution doesn't work but because I suspect that this problem will likely arise again in future code, and that we may need to fix the ordering of things in setup. A solution to this would be to move the call to initialise_fullme within setuplib.php down a couple of lines so that it is after $USER is established. The reason being that that call to redirect for wwwrootmismatch is likely to lead to further errors like this occurring. That call to redirect with a message is always going to trigger the initialisation of the core page/theme stuff. I've added Eloy as a watcher here as that would of course be a serious change, so Eloy please can you let us know your thoughts. Cheers Sam
          Hide
          Michael de Raadt added a comment -

          Hi, Anthony.

          I've put this on the Stable backlog, but feel free to work on it whenever you wish.

          Michael;

          Show
          Michael de Raadt added a comment - Hi, Anthony. I've put this on the Stable backlog, but feel free to work on it whenever you wish. Michael;
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          Anthony - thanks for spotting this and the patch you produced. I've just been talking to Eloy about this issue as we think it may be a better idea for us to solve it at a lower level.
          As such I've just put forward a patch for integration that involves moves the initialise_fullme() call down below the establishment of $USER.
          I've tested this by attempting to reproduce the problem, and I've also tested a fresh installation, and an upgrade.. all went fine.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, Anthony - thanks for spotting this and the patch you produced. I've just been talking to Eloy about this issue as we think it may be a better idea for us to solve it at a lower level. As such I've just put forward a patch for integration that involves moves the initialise_fullme() call down below the establishment of $USER. I've tested this by attempting to reproduce the problem, and I've also tested a fresh installation, and an upgrade.. all went fine. Cheers Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          tested by forcing the wwwrootmismatch error and also tried install / upgrade. All went ok. Passing.

          Show
          Eloy Lafuente (stronk7) added a comment - tested by forcing the wwwrootmismatch error and also tried install / upgrade. All went ok. Passing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U.P.S.T.R.E.A.M

          Show
          Eloy Lafuente (stronk7) added a comment - U.P.S.T.R.E.A.M
          Hide
          Petr Škoda added a comment - - edited

          oh, this was a very wrong workaround, this can not be after session start - we absolutely must not mess with cookies when the www root is wrong! We simply need to fix the missing $USER, reverting. In any case I need this to be available very very early in order to get the multitenant thing working...

          Show
          Petr Škoda added a comment - - edited oh, this was a very wrong workaround, this can not be after session start - we absolutely must not mess with cookies when the www root is wrong! We simply need to fix the missing $USER, reverting. In any case I need this to be available very very early in order to get the multitenant thing working...

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: