diff --git a/lib/ajax/ajaxlib.php b/lib/ajax/ajaxlib.php index 4c298b54b59..7fe4b034969 100644 --- a/lib/ajax/ajaxlib.php +++ b/lib/ajax/ajaxlib.php @@ -80,3 +80,40 @@ function ajaxenabled(array $browsers = null) { return false; } } + +/** + * Starts capturing output whilst processing an AJAX request. + * + * This should be used in combination with ajax_check_captured_output to + * report any captured output to the user. + * + * @retrun Boolean Returns true on success or false on failure. + */ +function ajax_capture_output() { + // Start capturing output in case of broken plugins. + return ob_start(); +} + +/** + * Check captured output for content. If the site has a debug level of + * debugdeveloper set, and the content is non-empty, then throw a coding + * exception which can be captured by the Y.IO request and displayed to the + * user. + * + * @return Any output that was captured. + */ +function ajax_check_captured_output() { + global $CFG; + + // Retrieve the output - there should be none. + $output = ob_get_contents(); + ob_end_clean(); + + if ($CFG->debug == DEBUG_DEVELOPER && !empty($output)) { + // Only throw an error if the site is in debugdeveloper. + throw new coding_exception('Unexpected output whilst processing AJAX request. ' . + 'This could be caused by trailing whitespace. Output received: ' . + var_export($output, true)); + } + return $output; +} diff --git a/lib/tests/ajaxlib_test.php b/lib/tests/ajaxlib_test.php new file mode 100644 index 00000000000..9baea2ce8fb --- /dev/null +++ b/lib/tests/ajaxlib_test.php @@ -0,0 +1,121 @@ +. + +/** + * Code quality unit tests that are fast enough to run each time. + * + * @package core + * @category phpunit + * @copyright 2013 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU Public License + */ + +defined('MOODLE_INTERNAL') || die(); + +class core_ajaxlib_testcase extends advanced_testcase { + + protected function helper_test_clean_output() { + $this->resetAfterTest(); + + $result = ajax_capture_output(); + + // ob_start should normally return without issue. + $this->assertTrue($result); + + $result = ajax_check_captured_output(); + $this->assertEmpty($result); + } + + protected function helper_test_dirty_output($expectexception = false) { + $this->resetAfterTest(); + + // Keep track of the content we will output. + $content = "Some example content"; + + $result = ajax_capture_output(); + + // ob_start should normally return without issue. + $this->assertTrue($result); + + // Fill the output buffer. + echo $content; + + if ($expectexception) { + $this->setExpectedException('coding_exception'); + ajax_check_captured_output(); + } else { + $result = ajax_check_captured_output(); + $this->assertEquals($result, $content); + } + } + + public function test_output_capture_normal_debug_none() { + global $CFG; + // In normal conditions, and with DEBUG_NONE set, we should not receive any output or throw any exceptions. + $CFG->debug = DEBUG_NONE; + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debug_normal() { + global $CFG; + // In normal conditions, and with DEBUG_NORMAL set, we should not receive any output or throw any exceptions. + $CFG->debug = DEBUG_NORMAL; + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debug_all() { + global $CFG; + // In normal conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions. + $CFG->debug = DEBUG_ALL; + $this->helper_test_clean_output(); + } + + public function test_output_capture_normal_debugdeveloper() { + global $CFG; + // In normal conditions, and with DEBUG_DEVELOPER set, we should not receive any output or throw any exceptions. + $CFG->debug = DEBUG_DEVELOPER; + $this->helper_test_clean_output(); + } + + public function test_output_capture_error_debug_none() { + global $CFG; + // With DEBUG_NONE set, we should not throw any exception, but the output will be returned. + $CFG->debug = DEBUG_NONE; + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debug_normal() { + global $CFG; + // With DEBUG_NORMAL set, we should not throw any exception, but the output will be returned. + $CFG->debug = DEBUG_NORMAL; + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debug_all() { + global $CFG; + // In error conditions, and with DEBUG_ALL set, we should not receive any output or throw any exceptions. + $CFG->debug = DEBUG_ALL; + $this->helper_test_dirty_output(); + } + + public function test_output_capture_error_debugdeveloper() { + global $CFG; + // With DEBUG_DEVELOPER set, we should throw an exception. + $CFG->debug = DEBUG_DEVELOPER; + $this->helper_test_dirty_output(true); + } + +} diff --git a/repository/filepicker.js b/repository/filepicker.js index 1beaf56fe22..afc23449d8d 100644 --- a/repository/filepicker.js +++ b/repository/filepicker.js @@ -583,27 +583,23 @@ M.core_filepicker.init = function(Y, options) { method: 'POST', on: { complete: function(id,o,p) { - if (!o) { - // TODO - alert('IO FATAL'); - return; - } var data = null; try { data = Y.JSON.parse(o.responseText); } catch(e) { - scope.print_msg(M.str.repository.invalidjson, 'error'); - scope.display_error(M.str.repository.invalidjson+'
'+stripHTML(o.responseText)+'
', 'invalidjson') - return; + if (o && o.status && o.status > 0) { + Y.use('moodle-core-notification', function() { + new M.core.exception(e); + }); + return; + } } // error checking if (data && data.error) { - scope.print_msg(data.error, 'error'); - if (args.onerror) { - args.onerror(id,data,p); - } else { - this.fpnode.one('.fp-content').setContent(''); - } + Y.use('moodle-core-notification', function() { + new M.core.ajaxException(data); + }); + this.fpnode.one('.fp-content').setContent(''); return; } else { if (data.msg) { diff --git a/repository/repository_ajax.php b/repository/repository_ajax.php index 7f6e4a641b7..fa8f488523f 100644 --- a/repository/repository_ajax.php +++ b/repository/repository_ajax.php @@ -75,6 +75,8 @@ $repooptions = array( 'ajax' => true, 'mimetypes' => $accepted_types ); + +ajax_capture_output(); $repo = repository::get_repository_by_id($repo_id, $contextid, $repooptions); // Check permissions @@ -128,6 +130,7 @@ switch ($action) { if ($repo->check_login()) { $listing = repository::prepare_listing($repo->get_listing($req_path, $page)); $listing['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($listing); break; } else { @@ -136,23 +139,27 @@ switch ($action) { case 'login': $listing = $repo->print_login(); $listing['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($listing); break; case 'logout': $logout = $repo->logout(); $logout['repo_id'] = $repo_id; + ajax_check_captured_output(); echo json_encode($logout); break; case 'searchform': $search_form['repo_id'] = $repo_id; $search_form['form'] = $repo->print_search(); $search_form['allowcaching'] = true; + ajax_check_captured_output(); 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; + ajax_check_captured_output(); echo json_encode($search_result); break; case 'download': @@ -191,6 +198,7 @@ switch ($action) { $info['file'] = $saveas_filename; $info['type'] = 'link'; $info['url'] = $link; + ajax_check_captured_output(); echo json_encode($info); die; } else { @@ -282,6 +290,7 @@ switch ($action) { // You can cache reository file in this callback // or complete other tasks. $repo->cache_file_by_reference($reference, $storedfile); + ajax_check_captured_output(); echo json_encode($event); die; } else if ($repo->has_moodle_files()) { @@ -292,6 +301,7 @@ switch ($action) { // {@link repository::copy_to_area()}. $fileinfo = $repo->copy_to_area($reference, $record, $maxbytes, $areamaxbytes); + ajax_check_captured_output(); echo json_encode($fileinfo); die; } else { @@ -317,12 +327,14 @@ switch ($action) { $info['e'] = get_string('error', 'moodle'); } } + ajax_check_captured_output(); echo json_encode($info); die; } break; case 'upload': $result = $repo->upload($saveas_filename, $maxbytes); + ajax_check_captured_output(); echo json_encode($result); break; @@ -335,6 +347,7 @@ switch ($action) { $newfilename = required_param('newfilename', PARAM_FILE); $info = repository::overwrite_existing_draftfile($itemid, $filepath, $filename, $newfilepath, $newfilename); + ajax_check_captured_output(); echo json_encode($info); break; @@ -342,6 +355,7 @@ switch ($action) { // delete tmp file $newfilepath = required_param('newfilepath', PARAM_PATH); $newfilename = required_param('newfilename', PARAM_FILE); + ajax_check_captured_output(); echo json_encode(repository::delete_tempfile_from_draft($itemid, $newfilepath, $newfilename)); break;