Moodle
  1. Moodle
  2. MDL-34182

Invalid JSON output on filepicker when repository plugins output invalid content

    Details

    • Testing Instructions:
      Hide

      With debugging disabled:

      • Open the profile editing page
      • Open the file picker for account images
      • Select each of the repositories, search for and select an image using each one
        • Confirm no errors were shown at any stage
      • Expand the site administration menu (2.6 and master only
        • Confirm no errors were displayed*
      • Edit repositories/wikimedia/wikimedia.php and theme/splash/settings.php
        • Add a character before the opening php tags
      • Repeat the tests confirming a complete lack of errors
      • Expand the site administration menu (2.6 and master only
        • Confirm no errors were displayed*

      Set debug to debug_developer

      • repeat the tests
        • Confirm: An ajaxException should be shown for plugins which have any invalid output associated with them
      Show
      With debugging disabled: Open the profile editing page Open the file picker for account images Select each of the repositories, search for and select an image using each one Confirm no errors were shown at any stage Expand the site administration menu (2.6 and master only Confirm no errors were displayed* Edit repositories/wikimedia/wikimedia.php and theme/splash/settings.php Add a character before the opening php tags Repeat the tests confirming a complete lack of errors Expand the site administration menu (2.6 and master only Confirm no errors were displayed* Set debug to debug_developer repeat the tests Confirm: An ajaxException should be shown for plugins which have any invalid output associated with them
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull 2.6 Branch:
    • Pull Master Branch:
      MDL-34182-master
    • Story Points (Obsolete):
      8
    • Sprint:
      FRONTEND Sprint 7

      Description

      I have just installed MOODLE V2.3 and have tried to upload and restore my backup files from MOODLE V1.9.18. When I select a Small backup file it run ok from the file picker but when I select a larger one say 25Megs using the File Picker then an Error Dialogue Box comes up informing me of the Invalid JSON Script.

      I turned on the Debugger and found the following script message;
      ----------------------------------------------------------------------------------------
      Invalid JSON string

      Fatal error: Allowed memory size of 100663296 bytes exhausted (tried to allocate 85462399 bytes) in /home/souther9/public_html/lib/filelib.php on line 3066
      ----------------------------------------------------------------------------------------

      When I checked the code it displays the following so I am not sure where to go to next;
      _________________________________________________________________________________________________________________
      // create curl instance
      $curl = curl_init($url);
      $options['url'] = $url;
      $this->apply_opt($curl, $options);
      if ($this->cache && $ret = $this->cache->get($this->options))

      { return $ret; } else {
      $ret = curl_exec($curl);
      if ($this->cache) { $this->cache->set($this->options, $ret); }
      }

      $this->info = curl_getinfo($curl);
      $this->error = curl_error($curl);

      if ($this->debug){ echo '<h1>Return Data</h1>'; var_dump($ret); echo '<h1>Info</h1>'; var_dump($this->info); echo '<h1>Error</h1>'; var_dump($this->error); }

      curl_close($curl);

      if (empty($this->error)){ return $ret; }

      else

      { return $this->error; // exception is not ajax friendly //throw new moodle_exception($this->error, 'curl'); }

      }
      ___________________________________________________________________________________

      Can anyone help me get this sorted out please I have tried the forums and looked through some of the other issues but to no help found.

      Thanks

      Jim Kersting

        Gliffy Diagrams

        1. config.php
          31 kB
          Rafa Gutierrez

          Issue Links

            Activity

            Hide
            Marina Glancy added a comment -

            Jim,
            can you tell how exactly do you select a file from filepicker? Do you upload it or select from some repository?

            Show
            Marina Glancy added a comment - Jim, can you tell how exactly do you select a file from filepicker? Do you upload it or select from some repository?
            Hide
            James Kersting added a comment -

            Sorry! Yes I upload it using either Dropbox connection or Upload a File link.

            Now I get another dialogue box telling me that, "The uploaded file may exceed max_post_size directive in php.ini". How do I allocate this one as the system server will not add it for me so I will have to write it in with some scripting.

            Show
            James Kersting added a comment - Sorry! Yes I upload it using either Dropbox connection or Upload a File link. Now I get another dialogue box telling me that, "The uploaded file may exceed max_post_size directive in php.ini". How do I allocate this one as the system server will not add it for me so I will have to write it in with some scripting.
            Hide
            Marina Glancy added a comment -

            James, one of the suggestions is that you can install the repository filesystem. You can upload a file to your filesystem using ftp. When you pick it in filepicker it is just copied within your filesystem, so both max_post_size and curl will not eat up your memory.

            There is a capability 'moodle/course:ignorefilesizelimits' (always on for admin), that removes the file size limit for you. But it makes sense only for picking files from repositories, not for upload.

            Show
            Marina Glancy added a comment - James, one of the suggestions is that you can install the repository filesystem. You can upload a file to your filesystem using ftp. When you pick it in filepicker it is just copied within your filesystem, so both max_post_size and curl will not eat up your memory. There is a capability 'moodle/course:ignorefilesizelimits' (always on for admin), that removes the file size limit for you. But it makes sense only for picking files from repositories, not for upload.
            Hide
            James Kersting added a comment -

            Oh ok I will give it a try and see what happens.

            Thank you for your response.

            Show
            James Kersting added a comment - Oh ok I will give it a try and see what happens. Thank you for your response.
            Hide
            Helen Foster added a comment -

            Linking to MDL-25190 as both issues appear very similar, or perhaps identical? If anyone can confirm they're identical then we can close the newest issue.

            Show
            Helen Foster added a comment - Linking to MDL-25190 as both issues appear very similar, or perhaps identical? If anyone can confirm they're identical then we can close the newest issue.
            Hide
            Didier Raboud added a comment -

            I can confirm this is happenning on a 2.3 instance here too. The "invalid JSON String" error happens at the same time as a "PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 1387 bytes) in …/lib/dml/pgsql_native_moodle_database.php on line 720, referer: …/course/modedit.php?update=…&return=0&sr=0" So I guess the PHP script behind just stops and breaks the JS output.

            Show
            Didier Raboud added a comment - I can confirm this is happenning on a 2.3 instance here too. The "invalid JSON String" error happens at the same time as a "PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 1387 bytes) in …/lib/dml/pgsql_native_moodle_database.php on line 720, referer: …/course/modedit.php?update=…&return=0&sr=0" So I guess the PHP script behind just stops and breaks the JS output.
            Hide
            Nancy K Hoke added a comment -

            I hope this is correct place to add this. We upgraded to Moodle 2.3 this fall and I am preparing a guide for the faculty to use the file picker for their Spring Term courses. I have logged on in Faculty Role - went to a course and selected File from Resources. Clicked on Add for the File Picker - and Server Files. I see all the files for the current course - but when I click on Server Files - to see the other courses I am seeing the Invalid JSON string error message. Our version is Moodle 2.3.2+ on Linux.

            Show
            Nancy K Hoke added a comment - I hope this is correct place to add this. We upgraded to Moodle 2.3 this fall and I am preparing a guide for the faculty to use the file picker for their Spring Term courses. I have logged on in Faculty Role - went to a course and selected File from Resources. Clicked on Add for the File Picker - and Server Files. I see all the files for the current course - but when I click on Server Files - to see the other courses I am seeing the Invalid JSON string error message. Our version is Moodle 2.3.2+ on Linux.
            Hide
            Luciano Silva added a comment -

            This problem keeps the same on Moodle 2.3.4 build 20130228. We had to apply the patch. Could you incorporate this patch to the release?

            Show
            Luciano Silva added a comment - This problem keeps the same on Moodle 2.3.4 build 20130228. We had to apply the patch. Could you incorporate this patch to the release?
            Hide
            Marina Glancy added a comment -

            Luciano, I would love to. Can you attach the patch to the issue and/or put it in the comment?

            Show
            Marina Glancy added a comment - Luciano, I would love to. Can you attach the patch to the issue and/or put it in the comment?
            Hide
            Rajesh Taneja added a comment -

            I am taking this as I worked on MDL-25190, will look at this in current sprint as this seems to be broken functionality.

            Show
            Rajesh Taneja added a comment - I am taking this as I worked on MDL-25190 , will look at this in current sprint as this seems to be broken functionality.
            Hide
            Rajesh Taneja added a comment - - edited

            Hello Luciano,

            Can you please confirm few things for me.

            1. Do you have $CFG->sslproxy=true; and/or $CFG->loginhttps = '1'; in your config
            2. If above, are they defined after require_once(dirname(_FILE_) . '/lib/setup.php'); ?
            3. Does your $CFG->wwwroot has 'https://MOODLESITE' or 'http://MOODLESITE'

            I managed to reproduce this problem, and it happens only if these configurations are not set after setup.php is included and wwwroot is http not https.

            If above is not the case then please add more information about your setup and error msg, as with above information it's not easy to detect the cause of problem.

            Show
            Rajesh Taneja added a comment - - edited Hello Luciano, Can you please confirm few things for me. Do you have $CFG->sslproxy=true; and/or $CFG->loginhttps = '1'; in your config If above, are they defined after require_once(dirname(_ FILE _) . '/lib/setup.php'); ? Does your $CFG->wwwroot has 'https://MOODLESITE' or 'http://MOODLESITE' I managed to reproduce this problem, and it happens only if these configurations are not set after setup.php is included and wwwroot is http not https. If above is not the case then please add more information about your setup and error msg, as with above information it's not easy to detect the cause of problem.
            Hide
            Rajesh Taneja added a comment -

            Hello Nancy,

            Can you please describe faculty role. Is it teacher or manager?
            Also, is it system level or role or course level role?

            Show
            Rajesh Taneja added a comment - Hello Nancy, Can you please describe faculty role. Is it teacher or manager? Also, is it system level or role or course level role?
            Hide
            Rajesh Taneja added a comment -

            Hello Luciano,

            Referring to your comment can you please provide more information about your patch.

            It will be helpful if you can provide some debug information, or share config settings (make sure to remove site related information) to get to this problem.

            Show
            Rajesh Taneja added a comment - Hello Luciano, Referring to your comment can you please provide more information about your patch. It will be helpful if you can provide some debug information, or share config settings (make sure to remove site related information) to get to this problem.
            Hide
            Rajesh Taneja added a comment -

            As I am not confident enough to solve this issue, removing myself as assignee so that someone else can work on it.

            Show
            Rajesh Taneja added a comment - As I am not confident enough to solve this issue, removing myself as assignee so that someone else can work on it.
            Hide
            Rafa Gutierrez added a comment -

            bug fix 'invalid json string'

            diff --git a/repository/draftfiles_ajax.php b/repository/draftfiles_ajax.php
            index e50544a..6d07e8f 100644
            — a/repository/draftfiles_ajax.php
            +++ b/repository/draftfiles_ajax.php
            @@ 49,10 +49,14 @@ echo $OUTPUT>header(); // send headers
            //NOTE TO ALL DEVELOPERS: this script must deal only with draft area of current user, it has to use only file_storage and no file_browser!!
            //

            +// bug fix.- cadena json no valida
            +ob_start();
            switch ($action) {
            case 'dir':
            $data = new stdClass();
            file_get_drafarea_folders($draftid, $filepath, $data);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($data);
            die;

            @@ -65,6 +69,8 @@ switch ($action) {
            $data->filesize = $info['filesize'];
            $data->tree = new stdClass();
            file_get_drafarea_folders($draftid, '/', $data->tree);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($data);
            die;

            @@ -76,6 +82,8 @@ switch ($action) {
            $fs->create_directory($user_context->id, 'user', 'draft', $draftid, file_correct_filepath(file_correct_filepath($filepath).$newdirname));
            $return = new stdClass();
            $return->filepath = $filepath;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($return);
            die;

            @@ -95,16 +103,24 @@ switch ($action) {
            }
            $stored_file->delete();
            $return->filepath = $parent_path;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($return);
            } else {
            if($result = $stored_file->delete())

            { $return->filepath = $parent_path; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); }

            else

            { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); }

            }
            } else

            { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); }
            die;
            @@ -118,6 +134,8 @@ switch ($action) {
            file_reset_sortorder($user_context->id, 'user', 'draft', $draftid);
            // set main file
            $return = file_set_sortorder($user_context->id, 'user', 'draft', $draftid, $filepath, $filename, 1);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($return);
            die;

            @@ -170,8 +188,12 @@ switch ($action) {
            if ($newfile = $zipper->archive_to_storage(array($filepath => $file), $user_context->id, 'user', 'draft', $draftid, $parent_path, $zipfile, $USER->id)) { $return = new stdClass(); $return->filepath = $parent_path; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); } else { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); }

            die;
            @@ -201,6 +223,8 @@ switch ($action)

            { $return = new stdClass(); $return->fileurl = moodle_url::make_draftfile_url($newdraftitemid, '/', $filename)->out(); $return->filepath = $filepath; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); }

            else {
            echo json_encode(false);
            @@ -266,6 +290,8 @@ switch ($action)

            { echo json_encode(false); }

            else

            { $return = array('filename' => $filename, 'filepath' => $filepath, 'original' => $file->get_reference_details()); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode((object)$return); }

            die;
            @@ -294,6 +320,8 @@ switch ($action) {
            }
            }
            }
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode((object)$return);
            }
            die;
            diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php
            index d2b1dec..3859852 100644
            — a/repository/repository_ajax.php
            +++ b/repository/repository_ajax.php
            @@ -119,6 +119,9 @@ switch ($action)

            { break; }

            +// bug fix.- cadena json no valida
            +ob_start();
            +
            // These actions all occur on the currently active repository instance
            switch ($action) {
            case 'sign':
            @@ -127,6 +130,8 @@ switch ($action) {
            if ($repo->check_login())

            { $listing = repository::prepare_listing($repo->get_listing($req_path, $page)); $listing['repo_id'] = $repo_id; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($listing); break; }

            else {
            @@ -135,23 +140,31 @@ switch ($action) {
            case 'login':
            $listing = $repo->print_login();
            $listing['repo_id'] = $repo_id;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($listing);
            break;
            case 'logout':
            $logout = $repo->logout();
            $logout['repo_id'] = $repo_id;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($logout);
            break;
            case 'searchform':
            $search_form['repo_id'] = $repo_id;
            $search_form['form'] = $repo->print_search();
            $search_form['allowcaching'] = true;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($search_form);
            break;
            case 'search':
            $search_result = repository::prepare_listing($repo->search($search_text, (int)$page));
            $search_result['repo_id'] = $repo_id;
            $search_result['issearchresult'] = true;
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($search_result);
            break;
            case 'download':
            @@ -190,6 +203,8 @@ switch ($action)

            { $info['file'] = $saveas_filename; $info['type'] = 'link'; $info['url'] = $link; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($info); die; }

            else {
            @@ -281,6 +296,8 @@ switch ($action)

            { // You can cache reository file in this callback // or complete other tasks. $repo->cache_file_by_reference($reference, $storedfile); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($event); die; }

            else if ($repo->has_moodle_files()) {
            @@ -291,6 +308,8 @@ switch ($action) {
            //

            {@link repository::copy_to_area()}

            .
            $fileinfo = $repo->copy_to_area($reference, $record, $maxbytes, $areamaxbytes);

            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($fileinfo);
            die;
            } else {
            @@ -316,12 +335,16 @@ switch ($action)

            { $info['e'] = get_string('error', 'moodle'); }

            }
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($info);
            die;
            }
            break;
            case 'upload':
            $result = $repo->upload($saveas_filename, $maxbytes);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($result);
            break;

            @@ -334,6 +357,8 @@ switch ($action) {
            $newfilename = required_param('newfilename', PARAM_FILE);

            $info = repository::overwrite_existing_draftfile($itemid, $filepath, $filename, $newfilepath, $newfilename);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode($info);
            break;

            @@ -341,6 +366,8 @@ switch ($action) {
            // delete tmp file
            $newfilepath = required_param('newfilepath', PARAM_PATH);
            $newfilename = required_param('newfilename', PARAM_FILE);
            + // bug fix.- cadena json no valida
            + ob_end_clean();
            echo json_encode(repository::delete_tempfile_from_draft($itemid, $newfilepath, $newfilename));

            break;

            Show
            Rafa Gutierrez added a comment - bug fix 'invalid json string' diff --git a/repository/draftfiles_ajax.php b/repository/draftfiles_ajax.php index e50544a..6d07e8f 100644 — a/repository/draftfiles_ajax.php +++ b/repository/draftfiles_ajax.php @@ 49,10 +49,14 @@ echo $OUTPUT >header(); // send headers //NOTE TO ALL DEVELOPERS: this script must deal only with draft area of current user, it has to use only file_storage and no file_browser!! // +// bug fix.- cadena json no valida +ob_start(); switch ($action) { case 'dir': $data = new stdClass(); file_get_drafarea_folders($draftid, $filepath, $data); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($data); die; @@ -65,6 +69,8 @@ switch ($action) { $data->filesize = $info ['filesize'] ; $data->tree = new stdClass(); file_get_drafarea_folders($draftid, '/', $data->tree); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($data); die; @@ -76,6 +82,8 @@ switch ($action) { $fs->create_directory($user_context->id, 'user', 'draft', $draftid, file_correct_filepath(file_correct_filepath($filepath).$newdirname)); $return = new stdClass(); $return->filepath = $filepath; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); die; @@ -95,16 +103,24 @@ switch ($action) { } $stored_file->delete(); $return->filepath = $parent_path; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); } else { if($result = $stored_file->delete()) { $return->filepath = $parent_path; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); } else { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); } } } else { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); } die; @@ -118,6 +134,8 @@ switch ($action) { file_reset_sortorder($user_context->id, 'user', 'draft', $draftid); // set main file $return = file_set_sortorder($user_context->id, 'user', 'draft', $draftid, $filepath, $filename, 1); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); die; @@ -170,8 +188,12 @@ switch ($action) { if ($newfile = $zipper->archive_to_storage(array($filepath => $file), $user_context->id, 'user', 'draft', $draftid, $parent_path, $zipfile, $USER->id)) { $return = new stdClass(); $return->filepath = $parent_path; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); } else { + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(false); } die; @@ -201,6 +223,8 @@ switch ($action) { $return = new stdClass(); $return->fileurl = moodle_url::make_draftfile_url($newdraftitemid, '/', $filename)->out(); $return->filepath = $filepath; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($return); } else { echo json_encode(false); @@ -266,6 +290,8 @@ switch ($action) { echo json_encode(false); } else { $return = array('filename' => $filename, 'filepath' => $filepath, 'original' => $file->get_reference_details()); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode((object)$return); } die; @@ -294,6 +320,8 @@ switch ($action) { } } } + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode((object)$return); } die; diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php index d2b1dec..3859852 100644 — a/repository/repository_ajax.php +++ b/repository/repository_ajax.php @@ -119,6 +119,9 @@ switch ($action) { break; } +// bug fix.- cadena json no valida +ob_start(); + // These actions all occur on the currently active repository instance switch ($action) { case 'sign': @@ -127,6 +130,8 @@ switch ($action) { if ($repo->check_login()) { $listing = repository::prepare_listing($repo->get_listing($req_path, $page)); $listing['repo_id'] = $repo_id; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($listing); break; } else { @@ -135,23 +140,31 @@ switch ($action) { case 'login': $listing = $repo->print_login(); $listing ['repo_id'] = $repo_id; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($listing); break; case 'logout': $logout = $repo->logout(); $logout ['repo_id'] = $repo_id; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($logout); break; case 'searchform': $search_form ['repo_id'] = $repo_id; $search_form ['form'] = $repo->print_search(); $search_form ['allowcaching'] = true; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($search_form); break; case 'search': $search_result = repository::prepare_listing($repo->search($search_text, (int)$page)); $search_result ['repo_id'] = $repo_id; $search_result ['issearchresult'] = true; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($search_result); break; case 'download': @@ -190,6 +203,8 @@ switch ($action) { $info['file'] = $saveas_filename; $info['type'] = 'link'; $info['url'] = $link; + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($info); die; } else { @@ -281,6 +296,8 @@ switch ($action) { // You can cache reository file in this callback // or complete other tasks. $repo->cache_file_by_reference($reference, $storedfile); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($event); die; } else if ($repo->has_moodle_files()) { @@ -291,6 +308,8 @@ switch ($action) { // {@link repository::copy_to_area()} . $fileinfo = $repo->copy_to_area($reference, $record, $maxbytes, $areamaxbytes); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($fileinfo); die; } else { @@ -316,12 +335,16 @@ switch ($action) { $info['e'] = get_string('error', 'moodle'); } } + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($info); die; } break; case 'upload': $result = $repo->upload($saveas_filename, $maxbytes); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($result); break; @@ -334,6 +357,8 @@ switch ($action) { $newfilename = required_param('newfilename', PARAM_FILE); $info = repository::overwrite_existing_draftfile($itemid, $filepath, $filename, $newfilepath, $newfilename); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode($info); break; @@ -341,6 +366,8 @@ switch ($action) { // delete tmp file $newfilepath = required_param('newfilepath', PARAM_PATH); $newfilename = required_param('newfilename', PARAM_FILE); + // bug fix.- cadena json no valida + ob_end_clean(); echo json_encode(repository::delete_tempfile_from_draft($itemid, $newfilepath, $newfilename)); break;
            Hide
            Petr Skoda added a comment -

            instead we should put define('NO_DEBUG_DISPLAY', true); to all ajax scripts or make it default to that in lib/setup.php

            Show
            Petr Skoda added a comment - instead we should put define('NO_DEBUG_DISPLAY', true); to all ajax scripts or make it default to that in lib/setup.php
            Hide
            Rafa Gutierrez added a comment - - edited

            Errors "invalid json string" keep popping up if we only put the solution you are proposing (NO_DEBUG_DISPLAY).

            The problem is due to a side effect of some previous unexpected output. Any mistake in the previous proccesing (it could be an unexpected condition/situation/data the program does not handle properly) causes an invalid JSON string.

            We think this script should be more solid, and be more isolated to external mistakes, and that could be achieved dropping previous outputs before outputing JSON string.

            Show
            Rafa Gutierrez added a comment - - edited Errors "invalid json string" keep popping up if we only put the solution you are proposing (NO_DEBUG_DISPLAY). The problem is due to a side effect of some previous unexpected output. Any mistake in the previous proccesing (it could be an unexpected condition/situation/data the program does not handle properly) causes an invalid JSON string. We think this script should be more solid, and be more isolated to external mistakes, and that could be achieved dropping previous outputs before outputing JSON string.
            Hide
            Marina Glancy added a comment -

            The patch above from Rafa looks interesting. But instead of "eating" the previous output I would return it in json response in some 'otheroutput' variable.
            Inside the script (filepicker/filemanager) we need to check this var and display a message something like "Unexpected output detected: ...." and continue doing what it would be doing otherwise.

            Because if there is this output it means that something went wrong and it needs to be fixed.

            BTW when marking it as a blocker you need to provide the exact replication steps on any of supported versions (2.4, 2.5 or master).

            Show
            Marina Glancy added a comment - The patch above from Rafa looks interesting. But instead of "eating" the previous output I would return it in json response in some 'otheroutput' variable. Inside the script (filepicker/filemanager) we need to check this var and display a message something like "Unexpected output detected: ...." and continue doing what it would be doing otherwise. Because if there is this output it means that something went wrong and it needs to be fixed. BTW when marking it as a blocker you need to provide the exact replication steps on any of supported versions (2.4, 2.5 or master).
            Hide
            Kris Stokking added a comment -

            Marina, one of the biggest challenges with this ticket is reproducing it. It might be easier to do so if we could make code changes to force an error or debug message which would never be integrated. Would that work?

            Show
            Kris Stokking added a comment - Marina, one of the biggest challenges with this ticket is reproducing it. It might be easier to do so if we could make code changes to force an error or debug message which would never be integrated. Would that work?
            Hide
            Rafa Gutierrez added a comment -

            include this file (attachment) to force the error

            Show
            Rafa Gutierrez added a comment - include this file (attachment) to force the error
            Hide
            Andrew Nicols added a comment -

            I'm not sure on the best approach to this issue, so I'll start by summarising what seem to be the issue(s):

            1. The configuration file that Rafa Gutierrez has attached includes a unicode character (U+feff - a Zero-width non-breaking space) before the php open tag. Since this is before our output starts, we have no way of capturing it, short of running ob_start() before config.php is included on any AJAX script. We'd also have to know when to stop collecting, and when to throw the appropriate errors. I don't see this as a realistic solution to this particular issue and I think it would likely cause more issues than it solves in the long-run.

              <feff><?php
              

            2. Several users have reported that they're seeing out-of-memory issues. When php throws an out-of-memory notice, it dies. There is no way that we can capture the output and return it in a JSON object instead because the script ceases to process at this time. As I understand it, the only solution to this issue would be to increase the memory allocation, though others may have alternative suggestions on a better fix (it's still early and I haven't had my coffee yet)
            3. Issues with some Moodle plugin (most likely repository plugins, but not necessarily just these). As Marina says, Rafaels patch shows some promise, but we should try and warn appropriate users rather than entirely throwing away the content because these kinds of whitespace issues can cause all sorts of other quirky issues. I'm linking this issue to MDL-42989 which was caused by a similar issue. In that case we captured the output, and if the debug setting was DEBUG_DEVELOPER, we threw an exception; otherwise we swallowed it. I'm not particularly keen on this solution because it is less performant, and masks developer issues making them harder to debug, but this may well be the 'best' solution in this case.
            Show
            Andrew Nicols added a comment - I'm not sure on the best approach to this issue, so I'll start by summarising what seem to be the issue(s): The configuration file that Rafa Gutierrez has attached includes a unicode character (U+feff - a Zero-width non-breaking space) before the php open tag. Since this is before our output starts, we have no way of capturing it, short of running ob_start() before config.php is included on any AJAX script. We'd also have to know when to stop collecting, and when to throw the appropriate errors. I don't see this as a realistic solution to this particular issue and I think it would likely cause more issues than it solves in the long-run. <feff><?php Several users have reported that they're seeing out-of-memory issues. When php throws an out-of-memory notice, it dies. There is no way that we can capture the output and return it in a JSON object instead because the script ceases to process at this time. As I understand it, the only solution to this issue would be to increase the memory allocation, though others may have alternative suggestions on a better fix (it's still early and I haven't had my coffee yet) Issues with some Moodle plugin (most likely repository plugins, but not necessarily just these). As Marina says, Rafaels patch shows some promise, but we should try and warn appropriate users rather than entirely throwing away the content because these kinds of whitespace issues can cause all sorts of other quirky issues. I'm linking this issue to MDL-42989 which was caused by a similar issue. In that case we captured the output, and if the debug setting was DEBUG_DEVELOPER, we threw an exception; otherwise we swallowed it. I'm not particularly keen on this solution because it is less performant, and masks developer issues making them harder to debug, but this may well be the 'best' solution in this case.
            Hide
            Andrew Nicols added a comment -

            Just a note to say that MDL-43325 will block this issue.

            Show
            Andrew Nicols added a comment - Just a note to say that MDL-43325 will block this issue.
            Hide
            Andrew Nicols added a comment -

            I've written this using a function to do the testing because it's used in multiple place through the filepicker, and since 2.6 in other files. It also allows us to unit tests it (supplied).

            I've rewritten the one use we had of this same test to now use the same function too.

            As I understand it, we should not need to modify draftfiles_ajax.php because it does not load external content, but I may be wrong. Perhaps Marina Glancy or Frédéric Massart could indicate whether I'm correct or not..?

            Show
            Andrew Nicols added a comment - I've written this using a function to do the testing because it's used in multiple place through the filepicker, and since 2.6 in other files. It also allows us to unit tests it (supplied). I've rewritten the one use we had of this same test to now use the same function too. As I understand it, we should not need to modify draftfiles_ajax.php because it does not load external content, but I may be wrong. Perhaps Marina Glancy or Frédéric Massart could indicate whether I'm correct or not..?
            Hide
            Frédéric Massart added a comment -

            AFAIK draftfiles_ajax is the ajax file called from the File Manager, so for the files already present in the area. It does not load content, it interacts with those existing files.

            Show
            Frédéric Massart added a comment - AFAIK draftfiles_ajax is the ajax file called from the File Manager, so for the files already present in the area. It does not load content, it interacts with those existing files.
            Hide
            Jason Fowler added a comment -

            Looks great Andrew, submitting for integration.

            Show
            Jason Fowler added a comment - Looks great Andrew, submitting for integration.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Hi guys,

            this is just a message to share with you that I'm going to perform a test of the automated pre-checker against all the current issues awaiting integration (16 issues).

            So, soon, you'll get some extra comments in this issue with some information from the pre-checker. Note it's not final, but just an experiment and there are lots of things to improve, from the message itself to various false positives in the checkers. So take any report with caution, it's not 100% accurate yet.

            Please, feel free to comment any idea/objection @ MDLSITE-2662. I'll be collecting everything there.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Hi guys, this is just a message to share with you that I'm going to perform a test of the automated pre-checker against all the current issues awaiting integration (16 issues). So, soon, you'll get some extra comments in this issue with some information from the pre-checker. Note it's not final, but just an experiment and there are lots of things to improve, from the message itself to various false positives in the checkers. So take any report with caution, it's not 100% accurate yet. Please, feel free to comment any idea/objection @ MDLSITE-2662 . I'll be collecting everything there. TIA and ciao
            Hide
            CiBoT added a comment -

            Results for MDL-34182

            Show
            CiBoT added a comment - Results for MDL-34182 Branch MDL-34182 -24 to be integrated into upstream MOODLE_24_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/662 Execution status: 2 Error: The MDL-34182 -24 branch at git://github.com/andrewnicols/moodle.git is very old. Please rebase against current MOODLE_24_STABLE. Branch MDL-34182 -25 to be integrated into upstream MOODLE_25_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/663 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/663/artifact/work/smurf.html Branch MDL-34182 -26 to be integrated into upstream MOODLE_26_STABLE Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/664 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/664/artifact/work/smurf.html Branch MDL-34182 -master to be integrated into upstream master Executed job http://ci.stronk7.com/job/Precheck%20remote%20branch/665 Execution status: 0 Details: http://ci.stronk7.com/job/Precheck%20remote%20branch/665/artifact/work/smurf.html
            Hide
            Sam Hemelryk added a comment -

            Thanks Andrew - this has been integrated now.

            Show
            Sam Hemelryk added a comment - Thanks Andrew - this has been integrated now.
            Hide
            Sam Hemelryk added a comment -

            Thanks Andrew - tested and passed.

            Show
            Sam Hemelryk added a comment - Thanks Andrew - tested and passed.
            Hide
            Sam Hemelryk added a comment -

            Ah - noting I had to make an additional commit during integration, in a couple of places we had used Y.use('moodle-core-notification-ajaxException') - the capital E there causes bugs.

            Show
            Sam Hemelryk added a comment - Ah - noting I had to make an additional commit during integration, in a couple of places we had used Y.use('moodle-core-notification-ajaxException') - the capital E there causes bugs.
            Hide
            Andrew Nicols added a comment -

            My apologies - tab completion fail.

            Show
            Andrew Nicols added a comment - My apologies - tab completion fail.
            Hide
            Sam Hemelryk added a comment -

            Thank you, your code has landed just in time for 2013.
            Merry Christmas and may your 2014 be even better than 2013.

            Kind regards with much holiday spirit
            Sam

            Show
            Sam Hemelryk added a comment - Thank you, your code has landed just in time for 2013. Merry Christmas and may your 2014 be even better than 2013. Kind regards with much holiday spirit Sam

              People

              • Votes:
                6 Vote for this issue
                Watchers:
                15 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile