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

Block Navigation Javascript iterates out-of-bounds exception on Array

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9.5, 3.0.3
    • Fix Version/s: 2.9.6, 3.0.4
    • Component/s: Navigation
    • Labels:
    • Testing Instructions:
      Hide
      Without the patch
      1. Go to Site administration/Appearance/Additional HTML
      2. Add for example this to "Within HEAD":

        <style>
        .hugeicon { width:100px !important; height: 100px !important; }
        </style>
        <script type="text/javascript">
        Array.prototype.messingwithmoodle = 'hugeicon';
        </script>

        This will add a javascript snippet to every page that extends the Array type with a dummy function "messingwithmoodle". As you will see this function will add the class "hugeicon" to the icon image in front of the Siteadministration settings icons.

      3. Go to the frontpage. Verify that the Site administration menu is not extended yet. Go ahead and click to expand it. It should now show hugely inflated versions of the settings icons in the Site administration menu.
      With the patch
      1. Repeat step 3 from above, ensure you do not see the huge icons.
      Show
      Without the patch Go to Site administration/Appearance/Additional HTML Add for example this to "Within HEAD": <style> .hugeicon { width:100px !important; height: 100px !important; } </style> <script type="text/javascript"> Array.prototype.messingwithmoodle = 'hugeicon'; </script> This will add a javascript snippet to every page that extends the Array type with a dummy function "messingwithmoodle". As you will see this function will add the class "hugeicon" to the icon image in front of the Siteadministration settings icons. Go to the frontpage. Verify that the Site administration menu is not extended yet. Go ahead and click to expand it. It should now show hugely inflated versions of the settings icons in the Site administration menu. With the patch Repeat step 3 from above, ensure you do not see the huge icons.
    • Affected Branches:
      MOODLE_29_STABLE, MOODLE_30_STABLE
    • Fixed Branches:
      MOODLE_29_STABLE, MOODLE_30_STABLE
    • Pull from Repository:

      Description

      Throughout moodle-block_navigation-navigation.js for-in constructs are used to iterate over 'simple' Arrays. The problem is that if other code has extended the Array type (i.e. Array.prototype.somefunction) the loop will also try to iterate over these additional methods as if they were properties. This results in exceptions and a YUI popup window with "System Error" when clicking to expand "Site administration"

      I found an unresolved issue here https://tracker.moodle.org/browse/MDL-37388 and some background info on why using for-in to iterate over an Array is often not a good idea here http://stackoverflow.com/questions/500504/why-is-using-for-in-with-array-iteration-such-a-bad-idea.

      The problem in my case pops up because I am using an external reporting tool (jchartfx.com) that actually extends the Array prototype.

      There are various easy fixes possible. It would probably be best (at least in the cases I have seen in this code) to replace the for-in loop by using a simple for-loop with an enumerator. There are some cases where you would want to use a for-in loop but you'd still have to make sure you use only the properties that were intended, e.g. by checking hasOwnProperty

        Attachments

          Issue Links

            Activity

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  9/May/16