MDL-59544 forms: Implicit validation of the filemanager and filepicker

The patch adds an extra validation step against accepted file types.
Even if the repository checks are bypassed (as illustrated in the
Behat), the invalid file is still caught by the element's validation
rules.

It turns out there is no way to test the filepicker element easily via
Behat. Additionally, it provides the renaming features only with
disabled javascript. So the Behat tests are provided for the filemanager
only.

AMOS BEGIN
 CPY [err_wrongfileextension,mod_workshop],[err_wrongfileextension,core_form]
AMOS END
This commit is contained in:
David Mudrák 2017-07-14 14:48:19 +02:00
parent 16a68a2f76
commit 1661204a6c
5 changed files with 97 additions and 1 deletions

View file

@ -41,6 +41,7 @@ $string['err_nopunctuation'] = 'You must enter no punctuation characters here.';
$string['err_numeric'] = 'You must enter a number here.'; $string['err_numeric'] = 'You must enter a number here.';
$string['err_rangelength'] = 'You must enter between {$a->format[0]} and {$a->format[1]} characters here.'; $string['err_rangelength'] = 'You must enter between {$a->format[0]} and {$a->format[1]} characters here.';
$string['err_required'] = 'You must supply a value here.'; $string['err_required'] = 'You must supply a value here.';
$string['err_wrongfileextension'] = 'Some files ({$a->wrongfiles}) cannot be uploaded. Only file types {$a->whitelist} are allowed.';
$string['filetypesany'] = 'All file types'; $string['filetypesany'] = 'All file types';
$string['filetypesnotall'] = 'It is not allowed to select \'All file types\' here'; $string['filetypesnotall'] = 'It is not allowed to select \'All file types\' here';
$string['filetypesnotwhitelisted'] = 'These file types are not allowed here: {$a}'; $string['filetypesnotwhitelisted'] = 'These file types are not allowed here: {$a}';

View file

@ -308,6 +308,47 @@ class MoodleQuickForm_filemanager extends HTML_QuickForm_element implements temp
$context['html'] = $this->toHtml(); $context['html'] = $this->toHtml();
return $context; return $context;
} }
/**
* Check that all files have the allowed type.
*
* @param array $value Draft item id with the uploaded files.
* @return string|null Validation error message or null.
*/
public function validateSubmitValue($value) {
$filetypesutil = new \core_form\filetypes_util();
$whitelist = $filetypesutil->normalize_file_types($this->_options['accepted_types']);
if (empty($whitelist) || $whitelist === ['*']) {
// Any file type is allowed, nothing to check here.
return;
}
$draftfiles = file_get_drafarea_files($value);
$wrongfiles = array();
if (empty($draftfiles)) {
// No file uploaded, nothing to check here.
return;
}
foreach ($draftfiles->list as $file) {
if (!$filetypesutil->is_allowed_file_type($file->filename, $whitelist)) {
$wrongfiles[] = $file->filename;
}
}
if ($wrongfiles) {
$a = array(
'whitelist' => implode(', ', $whitelist),
'wrongfiles' => implode(', ', $wrongfiles),
);
return get_string('err_wrongfileextension', 'core_form', $a);
}
return;
}
} }
/** /**

View file

@ -230,4 +230,45 @@ class MoodleQuickForm_filepicker extends HTML_QuickForm_input implements templat
$context['html'] = $this->toHtml(); $context['html'] = $this->toHtml();
return $context; return $context;
} }
/**
* Check that the file has the allowed type.
*
* @param array $value Draft item id with the uploaded files.
* @return string|null Validation error message or null.
*/
public function validateSubmitValue($value) {
$filetypesutil = new \core_form\filetypes_util();
$whitelist = $filetypesutil->normalize_file_types($this->_options['accepted_types']);
if (empty($whitelist) || $whitelist === ['*']) {
// Any file type is allowed, nothing to check here.
return;
}
$draftfiles = file_get_drafarea_files($value);
$wrongfiles = array();
if (empty($draftfiles)) {
// No file uploaded, nothing to check here.
return;
}
foreach ($draftfiles->list as $file) {
if (!$filetypesutil->is_allowed_file_type($file->filename, $whitelist)) {
$wrongfiles[] = $file->filename;
}
}
if ($wrongfiles) {
$a = array(
'whitelist' => implode(', ', $whitelist),
'wrongfiles' => implode(', ', $wrongfiles),
);
return get_string('err_wrongfileextension', 'core_form', $a);
}
return;
}
} }

View file

@ -35,3 +35,14 @@ Feature: There is a form element allowing to select filetypes
And I should see "These file types are not allowed here: .doc, .docx, .rtf" And I should see "These file types are not allowed here: .doc, .docx, .rtf"
And I should see "It is not allowed to select 'All file types' here" And I should see "It is not allowed to select 'All file types' here"
And I should not see "Unknown file types: .neverminditdoesnotexist" And I should not see "Unknown file types: .neverminditdoesnotexist"
@javascript @_file_upload
Scenario: File manager element implicitly validates submitted files
# We can't directly upload the invalid file here as the upload repository would throw an exception.
# So instead we try to trick the filemanager, to be finally stopped by the implicit validation.
And I upload "lib/tests/fixtures/empty.txt" file to "Picky file manager" filemanager
And I follow "empty.txt"
And I set the field "Name" to "renamed.exe"
And I press "Update"
When I press "Save changes"
Then I should see "Some files (renamed.exe) cannot be uploaded. Only file types .txt are allowed."

View file

@ -57,6 +57,8 @@ class test_form extends moodleform {
$mform->addElement('filetypes', 'filetypes3', 'Unknown file types are allowed here', $mform->addElement('filetypes', 'filetypes3', 'Unknown file types are allowed here',
['allowunknown' => true]); ['allowunknown' => true]);
$mform->addElement('filemanager', 'fileman1', 'Picky file manager', null, ['accepted_types' => '.txt']);
$this->add_action_buttons(false); $this->add_action_buttons(false);
} }
} }