Skip to content

Commit

Permalink
Matrix: HTML validation in answers is missing #742244
Browse files Browse the repository at this point in the history
  • Loading branch information
mkassaei committed Dec 5, 2023
1 parent 77c2712 commit c883b43
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 5 deletions.
100 changes: 98 additions & 2 deletions edit_oumatrix_form.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ class qtype_oumatrix_edit_form extends question_edit_form {
/** @var string inputtype of rows. */
private $inputtype;

/** @var array of HTML tags allowed in column and row names. */
protected $allowedhtmltags = [
'sub',
'sup',
'i',
'em',
'span',
];

/** @var string regex to match HTML open tags. */
private $htmltstarttagsandattributes = '~<\s*\w+\b[^>]*>~';

/** @var string regex to match HTML close tags or br. */
private $htmltclosetags = '~<\s*/\s*\w+\b[^>]*>~';

/**
* Set the inputtype and number of rows and columns method.
*
Expand Down Expand Up @@ -339,7 +354,7 @@ public function validation($data, $files) {
// Validate minimum required number of rows.
$countrows = count(array_filter($data['rowname']));
if ($countrows < row::MIN_NUMBER_OF_ROWS) {
$errors['rowoptions[' . $countrows . ']'] = get_string('notenoughquestionrows', 'qtype_oumatrix',
$errors['rowname[' . $countrows . ']'] = get_string('notenoughquestionrows', 'qtype_oumatrix',
row::MIN_NUMBER_OF_ROWS);
}

Expand All @@ -349,7 +364,7 @@ public function validation($data, $files) {
if ($uniquerowscount < count($data['rowname'])) {
foreach ($data['rowname'] as $key => $name) {
if ($name != '' && in_array($name, $duplicate)) {
$errors['rowoptions[' . $key . ']'] = get_string('duplicates', 'qtype_oumatrix', $name);
$errors['rowname[' . $key . ']'] = get_string('duplicates', 'qtype_oumatrix', $name);
}
$duplicate[] = $name;
}
Expand Down Expand Up @@ -408,9 +423,90 @@ public function validation($data, $files) {
}
}
}

// Validate HTML tags used in column names.
$nonemptycolumns = array_filter($data['columnname']);
if ($nonemptycolumns) {
foreach ($nonemptycolumns as $key => $colname) {
$tagerror = $this->get_illegal_tag_error($colname);
if ($tagerror) {
$errors['columnname[' . $key . ']'] = $tagerror;
}
}
}

// Validate HTML tags used in row names.
if ($nonemptycolumns && $nonemptyrows) {
foreach ($nonemptyrows as $key => $rowname) {
$tagerror = $this->get_illegal_tag_error($rowname);
if ($tagerror) {
$errors['rowname[' . $key . ']'] = $tagerror;
}
}
}
return $errors;
}

/**
* Vaidate some input to make sure it does not contain any tags other than
* $this->allowedhtmltags.
* @param string $text the input to validate.
* @return string any validation errors.
*/
public function get_illegal_tag_error(string $text): string {
// Remove legal tags.
$strippedtext = $text;
foreach ($this->allowedhtmltags as $htmltag) {
$tagpair = "~<\s*/?\s*$htmltag\b\s*[^>]*>~";
$strippedtext = preg_replace($tagpair, '', $strippedtext);
}

$textarray = [];
preg_match_all($this->htmltstarttagsandattributes, $strippedtext, $textarray);
if ($textarray[0]) {
return $this->allowed_tags_message($textarray[0][0]);
}

preg_match_all($this->htmltclosetags, $strippedtext, $textarray);
if ($textarray[0]) {
return $this->allowed_tags_message($textarray[0][0]);
}

return '';
}

/**
* Returns a message indicating what tags are allowed.
*
* @param string $badtag The disallowed tag that was supplied
* @return string Message indicating what tags are allowed
*/
private function allowed_tags_message(string $badtag): string {
$a = new stdClass();
$a->tag = htmlspecialchars($badtag, ENT_COMPAT);
$a->allowed = $this->get_list_of_printable_allowed_tags($this->allowedhtmltags);
if ($a->allowed) {
return get_string('tagsnotallowed', 'qtype_oumatrix', $a);
} else {
return get_string('tagsnotallowedatall', 'qtype_oumatrix', $a);
}
}

/**
* Returns a prinatble list of allowed HTML tags.
*
* @param array $allowedhtmltags An array for tag strings that are allowed
* @return string A printable list of tags
*/
private function get_list_of_printable_allowed_tags(array $allowedhtmltags): string {
$allowedtaglist = [];
foreach ($allowedhtmltags as $htmltag) {
$allowedtaglist[] = htmlspecialchars('<' . $htmltag . '>', ENT_COMPAT);
}
return implode(', ', $allowedtaglist);
}


/**
* Returns the question type name.
*
Expand Down
2 changes: 2 additions & 0 deletions lang/en/qtype_oumatrix.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
$string['shuffleanswers'] = 'Shuffle the items?';
$string['shuffleanswers_desc'] = 'Whether options should be randomly shuffled for each attempt by default.';
$string['shuffleanswers_help'] = 'If enabled, the order of the row items is randomly shuffled for each attempt, provided that "Shuffle within questions" in the activity settings is also enabled.';
$string['tagsnotallowed'] = '{$a->tag} is not allowed. (Only {$a->allowed} are permitted.)';
$string['tagsnotallowedatall'] = '{$a->tag} is not allowed. (No HTML is allowed here.)';
$string['toomanyanswercols'] = 'You can have maximum {$a} answer columns.';
$string['toomanyquestionrows'] = 'You can have maximum {$a} sub-questions.';
$string['toomanyselected'] = 'You have selected too many options.';
Expand Down
15 changes: 15 additions & 0 deletions tests/behat/edit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,18 @@ Feature: Test editing an ouMatrix question
| Question name | Edited Multiple matrix name |
And I press "id_submitbutton"
And I should see "Edited Multiple matrix name"

@javascript
Scenario: Edit a Matrix question with multiple response containing HTML tags.
Given I am on the "Multiple matrix for editing" "core_question > edit" page logged in as teacher
When I set the following fields to these values:
| id_columnname_0 | <div>Chicken breast</div> |
| id_rowname_0 | <blink>Proteins</blink> |
And I press "id_submitbutton"
Then I should see "<div> is not allowed. (Only <sub>, <sup>, <i>, <em>, <span> are permitted.)"
And I should see "<blink> is not allowed. (Only <sub>, <sup>, <i>, <em>, <span> are permitted.)"
And I set the following fields to these values:
| id_columnname_0 | <em>Chicken breast<em> |
| id_rowname_0 | <i>Proteins</i> |
And I press "id_submitbutton"
And I should not see "is not allowed. (Only <sub>, <sup>, <i>, <em>, <span> are permitted.)"
87 changes: 84 additions & 3 deletions tests/edit_oumatrix_form_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class edit_oumatrix_form_test extends \advanced_testcase {
* question_edit_form great a question form instance that can be tested.
* stdClass the question category.
*/
protected function get_form($classname) {
protected function get_form(string $classname): array {
$this->setAdminUser();
$this->resetAfterTest();
$syscontext = \context_system::instance();
Expand Down Expand Up @@ -108,7 +108,7 @@ public function test_validation_cols_rows_minimum(): void {
$testdata['rowname'][3] = '';
$errors = $form->validation($testdata, []);
$expected = get_string('notenoughquestionrows', 'qtype_oumatrix', row::MIN_NUMBER_OF_ROWS);
$this->assertEquals($expected, $errors['rowoptions[1]']);
$this->assertEquals($expected, $errors['rowname[1]']);
}

/**
Expand Down Expand Up @@ -136,7 +136,7 @@ public function test_validation_cols_rows_duplicates(): void {
$testdata['rowname'][1] = 'Bee';
$errors = $form->validation($testdata, []);
$expected = get_string('duplicates', 'qtype_oumatrix', 'Bee');
$this->assertEquals($expected, $errors['rowoptions[1]']);
$this->assertEquals($expected, $errors['rowname[1]']);
}

/**
Expand Down Expand Up @@ -218,6 +218,87 @@ public function test_validation_rowanswers_on_empty_columns(): void {
$errors = $form->validation($testdata, []);
$expectedanswer = $errors['rowoptions[1]'] = get_string('correctanswerserror', 'qtype_oumatrix', $a);
$this->assertEquals($expectedanswer, $errors['rowoptions[1]']);
}

/**
* Test the form correctly validates if illegal html tags are added to the column and or row names.
*/
public function test_get_illegal_tag_error(): void {
[$form, $category] = $this->get_form('qtype_oumatrix_edit_form');
$helper = new qtype_oumatrix_test_helper();

// Single choice test.
$formdata = $helper->get_test_question_form_data('animals_single');
$formdata['category'] = $category->id;
$testdata = $formdata;

// Set the allowed tags.
$a = new \stdClass();
$a->allowed = '&lt;sub&gt;, &lt;sup&gt;, &lt;i&gt;, &lt;em&gt;, &lt;span&gt;';

// Add html tags to the column names.
// Illegal html tag.
$testdata['columnname'][0] = '<div>Insects</div>';
$errors = $form->validation($testdata, []);
$this->assertTrue(array_key_exists('columnname[0]', $errors));
$this->assertEquals(1, count($errors));
$a->tag = '&lt;div&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertEquals($expected, $errors['columnname[0]']);
$this->assertEquals($expected, $form->get_illegal_tag_error('<div>'));

// Illegal html tag.
$testdata['columnname'][1] = '<blink><i>Fish</i></blink>';
$errors = $form->validation($testdata, []);
$this->assertTrue(array_key_exists('columnname[1]', $errors));
$this->assertEquals(2, count($errors));
$a->tag = '&lt;blink&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertEquals($expected, $errors['columnname[1]']);
$this->assertEquals($expected, $form->get_illegal_tag_error('<blink>'));

// Allowed html tag.
$testdata['columnname'][2] = '<em>Birds</em>';
$this->assertFalse(array_key_exists('columnname[2]', $errors));
$errors = $form->validation($testdata, ['columnname[2]']);
$this->assertEquals(2, count($errors));
$a->tag = '&lt;em&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertNotEquals($expected, $form->get_illegal_tag_error('<em>'));

// Remove illegal tags from column names to remove errors.
$testdata['columnname'][0] = 'Insect';
$testdata['columnname'][1] = '<i>Fish</i>';
$testdata['columnname'][2] = '<em>Birds</em>';

// Add html tags to the rowname
// Illegal html tag.
$testdata['rowname'][0] = '<div>Bee</div>';
$errors = $form->validation($testdata, []);
$this->assertTrue(array_key_exists('rowname[0]', $errors));
$this->assertEquals(1, count($errors));
$a->tag = '&lt;div&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertEquals($expected, $errors['rowname[0]']);
$this->assertEquals($expected, $form->get_illegal_tag_error('<div>'));

// Illegal html tag.
$testdata['rowname'][1] = '<em><blink>Salmon</blink></em>';
$errors = $form->validation($testdata, []);
$this->assertTrue(array_key_exists('rowname[1]', $errors));
$this->assertEquals(2, count($errors));
$a->tag = '&lt;blink&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertEquals($expected, $errors['rowname[1]']);
$this->assertEquals($expected, $form->get_illegal_tag_error('<blink>'));

// Allowed html tag.
$testdata['rowname'][2] = '<em>Seagull</em>';
$this->assertFalse(array_key_exists('rowname[2]', $errors));
$errors = $form->validation($testdata, ['rowname[2]']);
$this->assertEquals(2, count($errors));
$a->tag = '&lt;em&gt;';
$expected = get_string('tagsnotallowed', 'qtype_oumatrix', $a);
$this->assertNotEquals($expected, $form->get_illegal_tag_error('<em>'));
}
}

0 comments on commit c883b43

Please sign in to comment.