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

Filepicker "Invalid JSON String" error due to bug in how accepted_types is handled

    Details

    • Testing Instructions:
      Hide

      Not sure it can be reproduced anywhere in core at the moment but it surely can break 3rd party code

      To test

      1. hack /user/files.php (or any other filemanager options) and set
        'accepted_types' => array('.jpg', 'web_image')
      2. Open page with this filemanager
      3. Try to pick files from repositories, make sure no errors occur
      Show
      Not sure it can be reproduced anywhere in core at the moment but it surely can break 3rd party code To test hack /user/files.php (or any other filemanager options) and set 'accepted_types' => array('.jpg', 'web_image') Open page with this filemanager Try to pick files from repositories, make sure no errors occur
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      MDL-40258_master

      Description

      I discovered a bug in the way that the "accepted_types" option of the file picker is handled. When passing in several different extensions and mime-types in the array file_get_grouptype() in lib/filelib.php returns an numerically indexed array of extensions. Those are passed in js_init_call(), which json_encodes it and includes in the page on output.

      When the array has consecutive indices starting at 0 (e.g. array(0 => 'jpg', 1 => 'png', 2 => 'svg') it gets encoded as an Array in JavaScript (e.g. ['jpg', 'png', 'svg']) and when the filepicker code turns that into the accepted_types parameter for the ajax call to the repository it works great there is no error.

      When the array has non-consecutive indices (e.g. array(0 => 'jpg', 1 => 'png', 9 => 'svg') it gets encoded as an JavaScript Object instead (e.g.

      {0: 'jpg', 1: 'png', 9: 'svg'}

      which in turn gets turned into the string "[object Object]" when the filepicker code trys to turn it into the accepted_types parameter for the ajax call to the repository.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            marina Marina Glancy added a comment -

            Thanks a lot Sam, submitting this for integration

            TO INTEGRATORS: Please cherry-pick in all versions starting from 2.3

            Show
            marina Marina Glancy added a comment - Thanks a lot Sam, submitting this for integration TO INTEGRATORS: Please cherry-pick in all versions starting from 2.3
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            sbc24 Sam Chaffee added a comment -

            Thanks for the quick response and testing instructions, Marina. Dan, I've rebased my branch. Let me know if there is anything else I can do.

            Show
            sbc24 Sam Chaffee added a comment - Thanks for the quick response and testing instructions, Marina. Dan, I've rebased my branch. Let me know if there is anything else I can do.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated (23, 24, 25 & master), thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (23, 24, 25 & master), thanks!
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys, appears to work perfectly.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys, appears to work perfectly.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Thanks for giving me joys and smiles
            Thanks for sharing my trouble's pile

            Thanks for wipeing the tears of my eye
            Thanks for showing me the glad view of sky

            Thanks for lending me your shoulders to lean
            Thanks for giving my words a proper mean

            Thanks for telling me the value of life
            Thanks for showing me the rules to survive

            Thanks for lending me the sympathetic ears
            Thanks for showing how much you care

            From all this what I mean in the end
            Is thanks for being my special friend.

            – Seema Chowdhury

            Sent upstream so... closing, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for giving me joys and smiles Thanks for sharing my trouble's pile Thanks for wipeing the tears of my eye Thanks for showing me the glad view of sky Thanks for lending me your shoulders to lean Thanks for giving my words a proper mean Thanks for telling me the value of life Thanks for showing me the rules to survive Thanks for lending me the sympathetic ears Thanks for showing how much you care From all this what I mean in the end Is thanks for being my special friend. – Seema Chowdhury Sent upstream so... closing, thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13