Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
MHajoha committed Nov 20, 2024
1 parent dc1db81 commit 4310502
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 8 deletions.
42 changes: 34 additions & 8 deletions classes/question_ui_renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

use coding_exception;
use DOMAttr;
use DOMChildNode;
use DOMDocument;
use DOMElement;
use DOMNameSpaceNode;
Expand Down Expand Up @@ -101,17 +102,29 @@ public function render(): string {

mt_srand($id);
try {
$this->resolve_placeholders();
// These handle our custom elements and attributes.
$this->hide_unwanted_feedback();
$this->hide_if_role();
$this->shuffle_contents();
$this->format_floats();

// Remove all unhandled custom elements, attributes, comments, and non-default xmlns declarations.
$this->clean_up();

// These modify standard HTML.
$this->set_input_values_and_readonly();
$this->soften_validation();
$this->defuse_buttons();
$this->shuffle_contents();

// TODO: Do we want to style placeholder expansions? (Which may be student-generated)
$this->add_styles();
$this->format_floats();

// We don't want to support QPy elements (and attributes, etc.) in placeholder expansions, so we resolve
// them after replacing QPy elements.
$this->resolve_placeholders();
// I'm not sure whether we should support names and IDs in placeholder expansion, but if we do, we should
// probably mangle them as well.
$this->mangle_ids_and_names();
$this->clean_up();
} finally {
// I'm not sure whether it is strictly necessary to reset the PRNG seed here, but it feels safer.
// Resetting it to its original state would be ideal, but that doesn't seem to be possible.
Expand Down Expand Up @@ -343,7 +356,7 @@ private function clean_up(): void {
* Replace placeholder PIs such as `<?p my_key plain?>` with the appropriate value from `$this->placeholders`.
*
* Since QPy transformations should not be applied to the content of the placeholders, this method should be called
* last.
* near the end.
*
* @return void
*/
Expand All @@ -362,22 +375,35 @@ private function resolve_placeholders(): void {
// Allow (X)HTML, but clean using Moodle's clean_text to prevent XSS.
$element = $this->xpath->document->createDocumentFragment();
$element->appendXML(clean_text($rawvalue));
$pi->parentNode->replaceChild($element, $pi);
} else if (strcasecmp($cleanoption, 'noclean') == 0) {
$element = $this->xpath->document->createDocumentFragment();
$element->appendXML($rawvalue);
// TODO: DocumentFragment::appendXML breaks upon invalid XML, it'd be great if we could support
// _some_ of it though. Testing DOMDocument::loadXML here.
$this->append_xml_before($pi, $rawvalue);
$pi->parentNode->removeChild($pi);
} else {
if (strcasecmp($cleanoption, 'plain') != 0) {
debugging("Unrecognized placeholder cleaning option: '$cleanoption', using 'plain'");
}
// Treat the value as plain text and don't allow any kind of markup.
// Since we're adding a text node, the DOM handles escaping for us.
$element = new DOMText($rawvalue);
$pi->parentNode->replaceChild($element, $pi);
}
$pi->parentNode->replaceChild($element, $pi);
}
}
}

private function append_xml_before(DOMNode $before, string $rawxml): void {
$newdoc = new DOMDocument();
// TODO: Would interpreting placeholder values as HTML make more sense?
$newdoc->loadXML($rawxml);

while($newdoc->hasChildNodes()) {
$before->parentNode->insertBefore($before, $newdoc->firstChild);
}
}

/**
* Replaces the HTML attributes `pattern`, `required`, `minlength`, `maxlength`, `min, `max` so that submission is
* not prevented.
Expand Down
29 changes: 29 additions & 0 deletions tests/question_ui_renderer_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,35 @@ public function test_should_resolve_placeholders(): void {
EXPECTED, $result);
}

/**
* Tests that placeholders are replaced.
*
* @return void
* @throws coding_exception
* @covers \qtype_questionpy\question_ui_renderer
*/
public function test_should_not_replace_qpy_elements_in_placeholder_expansion(): void {
$input = file_get_contents(__DIR__ . '/question_uis/placeholder.xhtml');
$qa = $this->create_question_attempt_stub();

$ui = new question_ui_renderer($input, [
'param' => "<qpy:format-float>123</qpy:format-float>",
'description' => 'My simple description.',
], new \question_display_options(), $qa);
$result = $ui->render();

$this->assert_html_string_equals_html_string(<<<EXPECTED
<div xmlns="http://www.w3.org/1999/xhtml">
<div>My simple description.</div>
<span>By default cleaned parameter: Value of param <b>one</b>.</span>
<span>Explicitly cleaned parameter: Value of param <b>one</b>.</span>
<span>Noclean parameter: Value of param <b>one</b>.<script>'Oh no, danger!'</script></span>
<span>Plain parameter: Value of param &lt;b>one&lt;/b>.&lt;script>'Oh no, danger!'&lt;/script>
</span>
</div>
EXPECTED, $result);
}

/**
* Tests that placeholders are just removed when the corresponding value is missing.
*
Expand Down

0 comments on commit 4310502

Please sign in to comment.