From b9bc201962359079592b606aa07c1d909644d280 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 17 Jan 2010 09:50:55 +0000 Subject: [PATCH] MDL-21233 escaped parameter is far more importatnt than the overrideparams in out() method - especially all JS urls should be converted to out(false); also it is possible to create new url with overrided parameters in constructor which might be actually work around the double encoding problems in the url (we should never use the out() in moodle_url constructor itself!) --- lib/ajax/ajaxlib.php | 2 +- lib/blocklib.php | 8 +++---- lib/editor/tinymce/lib.php | 2 +- lib/listlib.php | 2 +- lib/outputactions.php | 2 +- lib/outputcomponents.php | 4 ++-- lib/outputlib.php | 2 +- lib/outputrenderers.php | 4 ++-- lib/pagelib.php | 4 ++-- lib/portfoliolib.php | 2 +- lib/weblib.php | 29 +++++++++++--------------- mod/quiz/edit.php | 6 +++--- mod/quiz/editlib.php | 2 +- mod/quiz/report/grading/report.php | 2 +- mod/quiz/report/overview/report.php | 6 +++--- mod/quiz/report/responses/report.php | 4 ++-- mod/workshop/allocation/manual/lib.php | 4 ++-- mod/workshop/allocation/random/lib.php | 2 +- question/category_class.php | 2 +- question/editlib.php | 10 ++++----- question/question.php | 2 +- 21 files changed, 48 insertions(+), 53 deletions(-) diff --git a/lib/ajax/ajaxlib.php b/lib/ajax/ajaxlib.php index 75b0894ae89..8c5b3c5917d 100644 --- a/lib/ajax/ajaxlib.php +++ b/lib/ajax/ajaxlib.php @@ -130,7 +130,7 @@ class page_requirements_manager { $config = array( 'wwwroot' => $CFG->httpswwwroot, // Yes, really. See above. 'sesskey' => sesskey(), - 'loadingicon' => $output->pix_url('i/loading_small', 'moodle')->out_raw(), + 'loadingicon' => $output->pix_url('i/loading_small', 'moodle')->out(false), 'themerev' => theme_get_revision(), 'theme' => $page->theme->name, 'yui2loaderBase' => $this->yui2loader->base, diff --git a/lib/blocklib.php b/lib/blocklib.php index 386cd91e9aa..5d1bcacd881 100644 --- a/lib/blocklib.php +++ b/lib/blocklib.php @@ -286,8 +286,8 @@ class block_manager { * @return string URL for moving block $this->movingblock to this position. */ protected function get_move_target_url($region, $weight) { - return $this->page->url->out(array('bui_moveid' => $this->movingblock, - 'bui_newregion' => $region, 'bui_newweight' => $weight, 'sesskey' => sesskey()), false); + return $this->page->url->out(false, array('bui_moveid' => $this->movingblock, + 'bui_newregion' => $region, 'bui_newweight' => $weight, 'sesskey' => sesskey())); } /** @@ -889,7 +889,7 @@ class block_manager { } $controls = array(); - $actionurl = $this->page->url->out(array('sesskey'=> sesskey()), false); + $actionurl = $this->page->url->out(false, array('sesskey'=> sesskey())); // Assign roles icon. if (has_capability('moodle/role:assign', $block->context)) { @@ -897,7 +897,7 @@ class block_manager { // it is shortened because some web servers (e.g. IIS by default) give // a 'security' error if you try to pass a full URL as a GET parameter in another URL. - $return = $this->out_raw(); + $return = $this->out(false); $return = str_replace($CFG->wwwroot . '/', '', $return); $controls[] = array('url' => $CFG->wwwroot . '/' . $CFG->admin . diff --git a/lib/editor/tinymce/lib.php b/lib/editor/tinymce/lib.php index 1547722a38e..0345e0297d8 100644 --- a/lib/editor/tinymce/lib.php +++ b/lib/editor/tinymce/lib.php @@ -75,7 +75,7 @@ class tinymce_texteditor extends texteditor { $strtime = get_string('strftimetime'); $strdate = get_string('strftimedaydate'); $lang = str_replace('_utf8', '', current_language()); // use more standard language codes - $contentcss = $PAGE->theme->editor_css_url()->out_raw(); + $contentcss = $PAGE->theme->editor_css_url()->out(false); $context = empty($options['context']) ? get_context_instance(CONTEXT_SYSTEM) : $options['context']; if (!empty($options['legacy'])) { diff --git a/lib/listlib.php b/lib/listlib.php index 91166b5e08d..b133da203c9 100644 --- a/lib/listlib.php +++ b/lib/listlib.php @@ -267,7 +267,7 @@ class moodle_list { $html .= " $currentpage \n"; } else { - $html .= "pageurl->out(array($this->pageparamname => $currentpage))."\">"; + $html .= "pageurl->out(true, array($this->pageparamname => $currentpage))."\">"; $html .= " $currentpage \n"; } } diff --git a/lib/outputactions.php b/lib/outputactions.php index 6e33f385060..1408db80f36 100644 --- a/lib/outputactions.php +++ b/lib/outputactions.php @@ -132,7 +132,7 @@ class popup_action extends component_action { $this->params[$var] = $params[$var]; } } - parent::__construct($event, 'openpopup', array('url' => $url->out_raw(), 'name' => $name, 'options' => $this->get_js_options($params))); + parent::__construct($event, 'openpopup', array('url' => $url->out(false), 'name' => $name, 'options' => $this->get_js_options($params))); } /** diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index cca4492a0ed..aeb141e44df 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -1069,12 +1069,12 @@ class html_select extends labelled_html_component { } if (!empty($selected)) { - $selectedurl = $baseurl->out(array($name => $selected), false); + $selectedurl = $baseurl->out(false, array($name => $selected)); } // Replace real value by formatted URLs foreach ($options as $value => $label) { - $options[$baseurl->out(array($name => $value), false)] = $label; + $options[$baseurl->out(false, array($name => $value))] = $label; unset($options[$value]); } diff --git a/lib/outputlib.php b/lib/outputlib.php index 49b3e502a1f..73337ea5fa9 100644 --- a/lib/outputlib.php +++ b/lib/outputlib.php @@ -835,7 +835,7 @@ class theme_config { $replaced[$match[0]] = true; $imagename = $match[2]; $component = rtrim($match[1], '|'); - $imageurl = $this->pix_url($imagename, $component)->out_raw(); + $imageurl = $this->pix_url($imagename, $component)->out(false); // we do not need full url because the image.php is always in the same dir $imageurl = str_replace("$CFG->httpswwwroot/theme/", '', $imageurl); $css = str_replace($match[0], $imageurl, $css); diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index 36f3a3ab618..19825613f69 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -740,7 +740,7 @@ class core_renderer extends renderer_base { foreach ($controls as $control) { $controlshtml[] = html_writer::tag('a', array('class' => 'icon', 'title' => $control['caption'], 'href' => $control['url']), - html_writer::empty_tag('img', array('src' => $this->pix_url($control['icon'])->out_raw(), + html_writer::empty_tag('img', array('src' => $this->pix_url($control['icon'])->out(false), 'alt' => $control['caption']))); } return html_writer::tag('div', array('class' => 'commands'), implode('', $controlshtml)); @@ -820,7 +820,7 @@ class core_renderer extends renderer_base { $plaintitle = strip_tags($bc->title); $this->page->requires->js_function_call('new block_hider', array($bc->id, $userpref, get_string('hideblocka', 'access', $plaintitle), get_string('showblocka', 'access', $plaintitle), - $this->pix_url('t/switch_minus')->out_raw(), $this->pix_url('t/switch_plus')->out_raw())); + $this->pix_url('t/switch_minus')->out(false), $this->pix_url('t/switch_plus')->out(false))); } } diff --git a/lib/pagelib.php b/lib/pagelib.php index f75fea82487..343b97cfc3a 100644 --- a/lib/pagelib.php +++ b/lib/pagelib.php @@ -999,7 +999,7 @@ class moodle_page { if (empty($this->_block_actions_done)) { $this->_block_actions_done = true; if ($this->blocks->process_url_actions($this)) { - redirect($this->url->out_raw()); + redirect($this->url->out(false)); } } $this->blocks->create_all_block_instances(); @@ -1403,7 +1403,7 @@ class moodle_page { */ function url_get_full($extraparams = array()) { debugging('Call to deprecated method moodle_page::url_get_full. Use $this->url->out() instead.'); - return $this->url->out($extraparams); + return $this->url->out(true, $extraparams); } /** diff --git a/lib/portfoliolib.php b/lib/portfoliolib.php index eb84a36f7e4..efa67bfbeaf 100644 --- a/lib/portfoliolib.php +++ b/lib/portfoliolib.php @@ -319,7 +319,7 @@ class portfolio_add_button { } // if we just want a url to redirect to, do it now if ($format == PORTFOLIO_ADD_FAKE_URL) { - return $url->out_raw(); + return $url->out(false); } if (empty($addstr)) { diff --git a/lib/weblib.php b/lib/weblib.php index b3b0274e2a9..5b480c0fa5f 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -491,7 +491,7 @@ class moodle_url { * @return string */ public function __toString() { - return $this->out(null, true); + return $this->out(true); } /** @@ -500,11 +500,15 @@ class moodle_url { * If you use the returned URL in HTML code, you want the escaped ampersands. If you use * the returned URL in HTTP headers, you want $escaped=false. * - * @param array $overrideparams params to add to the output url, these override existing ones with the same name. * @param boolean $escaped Use & as params separator instead of plain & + * @param array $overrideparams params to add to the output url, these override existing ones with the same name. * @return string Resulting URL */ - public function out(array $overrideparams = null, $escaped = true) { + public function out($escaped = true, array $overrideparams = null) { + if (!is_bool($escaped)) { + debugging('Escape parameter must be of type boolean, '.gettype($escaped).' given instead.'); + } + $uri = $this->out_omit_querystring(); $querystring = $this->get_query_string($overrideparams, $escaped); @@ -518,15 +522,6 @@ class moodle_url { return $uri; } - /** - * Returns url in raw form without any escaping, - * useful especially when including urls and images in javascript. - * @return string - */ - public function out_raw() { - return $this->out(null, false); - } - /** * Returns url without parameters, everything before '?'. * @return string @@ -552,7 +547,7 @@ class moodle_url { public function out_action(array $overrideparams = null) { $overrideparams = (array)$overrideparams; $overrideparams = array('sesskey'=> sesskey()) + $overrideparams; - return $this->out($overrideparams); + return $this->out(true, $overrideparams); } /** @@ -648,7 +643,7 @@ function prepare_url($url, $stripformparams=false) { if ($stripformparams) { $output = $url->out_omit_querystring(); } else { - $output = $url->out_raw(); + $output = $url->out(false); } } @@ -657,7 +652,7 @@ function prepare_url($url, $stripformparams=false) { if (preg_match('/(.*)\/([A-Za-z0-9-_]*\.php)$/', $PAGE->url->out_omit_querystring(), $matches)) { return $matches[1] . "/$output"; } else if ($output == '') { - return $PAGE->url->out_raw() . '#'; + return $PAGE->url->out(false) . '#'; } else { throw new coding_exception('Unrecognied URL scheme. Please check the formatting of the URL passed to this function. Absolute URLs are the preferred scheme.'); } @@ -2081,7 +2076,7 @@ function print_collapsible_region_start($classes, $id, $caption, $userpref = fal $output .= '
'; $PAGE->requires->js_function_call('new collapsible_region', array($id, $userpref, get_string('clicktohideshow'), - $OUTPUT->pix_url('t/collapsed')->out_raw(), $OUTPUT->pix_url('t/expanded')->out_raw())); + $OUTPUT->pix_url('t/collapsed')->out(false), $OUTPUT->pix_url('t/expanded')->out(false))); if ($return) { return $output; @@ -2485,7 +2480,7 @@ function redirect($url, $message='', $delay=-1) { global $OUTPUT, $PAGE, $SESSION, $CFG; if ($url instanceof moodle_url) { - $url = $url->out_raw(); + $url = $url->out(false); } if (!empty($CFG->usesid) && !isset($_COOKIE[session_name()])) { diff --git a/mod/quiz/edit.php b/mod/quiz/edit.php index b783def4f0b..c10df85fb65 100644 --- a/mod/quiz/edit.php +++ b/mod/quiz/edit.php @@ -449,7 +449,7 @@ echo $OUTPUT->header(); // Initialise the JavaScript. $quizeditconfig = new stdClass; -$quizeditconfig->url = $thispageurl->out(array('qbanktool' => '0')); +$quizeditconfig->url = $thispageurl->out(true, array('qbanktool' => '0')); $quizeditconfig->dialoglisteners = array(); $numberoflisteners = max(quiz_number_of_pages($quiz->questions), 1); for ($pageiter = 1; $pageiter <= $numberoflisteners; $pageiter++) { @@ -477,10 +477,10 @@ if ($quiz_qbanktool) { echo '
'; echo '

'; echo get_string('questionbankcontents', 'quiz') . - ' out(true, array('qbanktool' => '1')) . '" id="showbankcmd">[' . get_string('show'). '] - out(true, array('qbanktool' => '0')) . '" id="hidebankcmd">[' . get_string('hide'). ']'; echo '

'; diff --git a/mod/quiz/editlib.php b/mod/quiz/editlib.php index 3ab3fdc2962..e0e58a9cafc 100644 --- a/mod/quiz/editlib.php +++ b/mod/quiz/editlib.php @@ -758,7 +758,7 @@ function quiz_print_randomquestion(&$question, &$pageurl, &$quiz, $quiz_qbanktoo $a->arrow = $OUTPUT->rarrow(); $strshowcategorycontents = get_string('showcategorycontents', 'quiz', $a); - $openqbankurl = $pageurl->out(array('qbanktool' => 1, + $openqbankurl = $pageurl->out(true, array('qbanktool' => 1, 'cat' => $category->id . ',' . $category->contextid)); $linkcategorycontents = ' ' . $strshowcategorycontents . ''; diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index 5eb7f38ea0b..0aa4d51b369 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -158,7 +158,7 @@ class quiz_grading_report extends quiz_default_report { /// find out current groups mode if ($groupmode = groups_get_activity_groupmode($this->cm)) { // Groups are being used - groups_print_activity_menu($this->cm, $this->viewurl->out(array('userid'=>0, 'attemptid'=>0))); + groups_print_activity_menu($this->cm, $this->viewurl->out(true, array('userid'=>0, 'attemptid'=>0))); } if(empty($this->users)) { diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index dca4463fd69..cb46332b8cd 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -170,12 +170,12 @@ class quiz_overview_report extends quiz_default_report { $this->regrade_all_needed($quiz, $groupstudents); } if ($regradeall || $regradealldry || $regradealldrydo){ - redirect($reporturl->out($displayoptions, false), '', 5); + redirect($reporturl->out(false, $displayoptions), '', 5); } if ($groupmode = groups_get_activity_groupmode($cm)) { // Groups are being used if (!$table->is_downloading()) { - groups_print_activity_menu($cm, $reporturl->out($displayoptions)); + groups_print_activity_menu($cm, $reporturl->out(treu, $displayoptions)); } } @@ -379,7 +379,7 @@ class quiz_overview_report extends quiz_default_report { $table->sortable(true, 'uniqueid'); // Set up the table - $table->define_baseurl($reporturl->out($displayoptions)); + $table->define_baseurl($reporturl->out(true, $displayoptions)); $table->collapsible(false); diff --git a/mod/quiz/report/responses/report.php b/mod/quiz/report/responses/report.php index a76c4bb776a..23560622b74 100644 --- a/mod/quiz/report/responses/report.php +++ b/mod/quiz/report/responses/report.php @@ -138,7 +138,7 @@ class quiz_responses_report extends quiz_default_report { if ($groupmode = groups_get_activity_groupmode($cm)) { // Groups are being used if (!$table->is_downloading()) { - groups_print_activity_menu($cm, $reporturl->out($displayoptions)); + groups_print_activity_menu($cm, $reporturl->out(true, $displayoptions)); } } // Print information on the number of existing attempts @@ -306,7 +306,7 @@ class quiz_responses_report extends quiz_default_report { $table->sortable(true, 'concattedid'); // Set up the table - $table->define_baseurl($reporturl->out($displayoptions)); + $table->define_baseurl($reporturl->out(true, $displayoptions)); $table->collapsible(true); diff --git a/mod/workshop/allocation/manual/lib.php b/mod/workshop/allocation/manual/lib.php index d8f2415aeb5..406e68a073f 100644 --- a/mod/workshop/allocation/manual/lib.php +++ b/mod/workshop/allocation/manual/lib.php @@ -87,7 +87,7 @@ class workshop_manual_allocator implements workshop_allocator { } } $m = implode('-', $m); // serialize message object to be passed via URL - redirect($PAGE->url->out(array('m' => $m), false)); + redirect($PAGE->url->out(false, array('m' => $m))); break; case 'del': if (!confirm_sesskey()) { @@ -119,7 +119,7 @@ class workshop_manual_allocator implements workshop_allocator { } } $m = implode('-', $m); // serialize message object to be passed via URL - redirect($PAGE->url->out(array('m' => $m), false)); + redirect($PAGE->url->out(false, array('m' => $m))); } break; } diff --git a/mod/workshop/allocation/random/lib.php b/mod/workshop/allocation/random/lib.php index 5c506468f27..31c473c0349 100644 --- a/mod/workshop/allocation/random/lib.php +++ b/mod/workshop/allocation/random/lib.php @@ -66,7 +66,7 @@ class workshop_random_allocator implements workshop_allocator { $customdata['workshop'] = $this->workshop; $this->mform = new workshop_random_allocator_form($PAGE->url, $customdata); if ($this->mform->is_cancelled()) { - redirect($PAGE->url->out_raw()); + redirect($PAGE->url->out(false)); } else if ($settings = $this->mform->get_data()) { // process validated data if (!confirm_sesskey()) { diff --git a/question/category_class.php b/question/category_class.php index 7976a37c9b2..e18ec95ec58 100644 --- a/question/category_class.php +++ b/question/category_class.php @@ -93,7 +93,7 @@ class question_category_list_item extends list_item { /// Each section adds html to be displayed as part of this list item $questionbankurl = "{$CFG->wwwroot}/question/edit.php?". $this->parentlist->pageurl->get_query_string(array('category'=>"$category->id,$category->contextid")); - $catediturl = $this->parentlist->pageurl->out(array('edit'=>$this->id)); + $catediturl = $this->parentlist->pageurl->out(true, array('edit'=>$this->id)); $item = "edit}\" href=\"$catediturl\">".$category->name ." ".'('.$category->questioncount.')'; $item .= ' '. $category->info; diff --git a/question/editlib.php b/question/editlib.php index 242d134c08a..b0c036d3310 100644 --- a/question/editlib.php +++ b/question/editlib.php @@ -802,7 +802,7 @@ class question_bank_view { // Create the url of the new question page to forward to. // TODO: it is sloppy to pass around full URLs through page parameters and some servers do not like that $this->editquestionurl = new moodle_url("$CFG->wwwroot/question/question.php", - array('returnurl' => urlencode($pageurl->out_raw()))); + array('returnurl' => urlencode($pageurl->out(false)))); if ($cm !== null){ $this->editquestionurl->param('cmid', $cm->id); } else { @@ -999,7 +999,7 @@ class question_bank_view { if (count($newsort) > question_bank_view::MAX_SORTS) { $newsort = array_slice($newsort, 0, question_bank_view::MAX_SORTS, true); } - return $this->baseurl->out($this->sort_to_params($newsort)); + return $this->baseurl->out(true, $this->sort_to_params($newsort)); } protected function build_query_sql($category, $recurse, $showhidden) { @@ -1078,11 +1078,11 @@ class question_bank_view { } public function edit_question_url($questionid) { - return $this->editquestionurl->out(array('id' => $questionid)); + return $this->editquestionurl->out(true, array('id' => $questionid)); } public function move_question_url($questionid) { - return $this->editquestionurl->out(array('id' => $questionid, 'movecontext' => 1)); + return $this->editquestionurl->out(true, array('id' => $questionid, 'movecontext' => 1)); } public function preview_question_url($questionid) { @@ -1422,7 +1422,7 @@ class question_bank_view { $checkforfiles = true; } } - $returnurl = $this->baseurl->out(array('category'=>"$tocategoryid,$contextid")); + $returnurl = $this->baseurl->out(true, array('category'=>"$tocategoryid,$contextid")); if (!$checkforfiles){ if (!question_move_questions_to_category(implode(',', $questionids), $tocategory->id)) { print_error('errormovingquestions', 'question', $returnurl, $questionids); diff --git a/question/question.php b/question/question.php index d13af26132a..57e31bc9bb5 100644 --- a/question/question.php +++ b/question/question.php @@ -219,7 +219,7 @@ if ($mform->is_cancelled()){ $returnurl = new moodle_url($returnurl); $returnurl->param('category', $fromform->category); // TODO: it is sloppy to pass arounf full URLs through page parameters and some servers do not like that - $returnurl = $returnurl->out_raw(); + $returnurl = $returnurl->out(false); /// Call the appropriate method. if ($movecontext) {