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

Problem with lib/javascript.php in some hosting environments

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.8, 2.1.5, 2.2.2
    • Fix Version/s: 2.1.6, 2.2.3
    • Component/s: Libraries
    • Labels:
      None
    • Environment:
      linux; chroot hosting with DIRROOT set to /
    • Testing Instructions:
      Hide

      Test normal sites only:
      1/ reset JS caches
      2/ look for regressions (missing JS)

      I think it is not necessary to test $CFG->dirroot='/' because it may be hard to set up chrooting properly and Moodle would not work probably much anyway.

      Show
      Test normal sites only: 1/ reset JS caches 2/ look for regressions (missing JS) I think it is not necessary to test $CFG->dirroot='/' because it may be hard to set up chrooting properly and Moodle would not work probably much anyway.
    • Workaround:
      Hide

      <pre>
      +++ lib/javascript.php 2012-02-20 20:04:11.000000000 +0100
      @@ -46,7 +46,7 @@
      // does not exist
      continue;
      }

      • if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== 0) {
        + if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== FALSE) { // hackers - not in dirroot continue; }

        </pre>

      Show
      <pre> +++ lib/javascript.php 2012-02-20 20:04:11.000000000 +0100 @@ -46,7 +46,7 @@ // does not exist continue; } if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== 0) { + if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== FALSE) { // hackers - not in dirroot continue; } </pre>
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w12_MDL-31703_m23_rootdirroot

      Description

      In some hosting environments, the DIRROOT=/ (single slash).
      In this case lib/javascript.php does not work correctly. In particular, the check if the requested $jsfile is in $DIRROOT:
      <pre>
      if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== 0)

      { </pre> If we take for example: <pre> /lib/javascript.php?file=%2Flib%2Fjavascript-static.js&rev=281 </pre> in our case, strpos will return 0 (position of first / in $jsfile), and the result will be empty. This breaks entire javascript on page. According to manual: http://php.net/manual/en/function.strrpos.php in case of "the needle not found", strpos returns FALSE, not 0. Therefore !== FALSE, besides fixing this issue, would be also syntactically correct. <pre> +++ lib/javascript.php 2012-02-20 20:04:11.000000000 +0100 @@ -46,7 +46,7 @@ // does not exist continue; }
      • if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== 0) {
        + if (strpos($jsfile, $CFG->dirroot . DIRECTORY_SEPARATOR) !== FALSE) { // hackers - not in dirroot continue; }

        </pre>

        Gliffy Diagrams

          Activity

          Hide
          skodak Petr Skoda added a comment -

          Please note Moodle is not compatible with $CFG->dirroot='/' - installer may not allow it, you need to manually protect dataroot and some parts of Moodle may not work at all.

          I agree it is better to fix this particular test because we may at least show our warnings and errors with Javascript on these unsupported sites.

          Thanks for the report.

          Show
          skodak Petr Skoda added a comment - Please note Moodle is not compatible with $CFG->dirroot='/' - installer may not allow it, you need to manually protect dataroot and some parts of Moodle may not work at all. I agree it is better to fix this particular test because we may at least show our warnings and errors with Javascript on these unsupported sites. Thanks for the report.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Petr - this has been integrated now

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Petr - this has been integrated now
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Works Great
          Thanks Petr, for fixing this and Imie for reporting and provinding workaround.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Works Great Thanks Petr, for fixing this and Imie for reporting and provinding workaround.
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Congratulations are in order, you've made it, or at least your code has!
          It's now part of Moodle and both the git and cvs repositories have been updated.

          This issue is being marked as fixed and closed.

          Thank you.

          Show
          samhemelryk Sam Hemelryk added a comment - Congratulations are in order, you've made it, or at least your code has! It's now part of Moodle and both the git and cvs repositories have been updated. This issue is being marked as fixed and closed. Thank you.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/12