Skip to content

Commit 0d090a3

Browse files
committed
feat: check file upload restrictions
1 parent 215abd1 commit 0d090a3

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

classes/local/files/options_file_service.php

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,14 @@
1717
namespace qtype_questionpy\local\files;
1818

1919
use coding_exception;
20+
use context;
2021
use context_user;
2122
use DateTimeImmutable;
2223
use file_exception;
2324
use moodle_exception;
2425
use moodle_url;
26+
use qtype_questionpy\local\form\elements\file_upload_element;
27+
use qtype_questionpy\local\form\elements\file_upload_options;
2528
use qtype_questionpy_question;
2629
use stored_file;
2730
use stored_file_creation_exception;
@@ -44,6 +47,7 @@ class options_file_service implements handles_qpy_url_type {
4447
/** @var string */
4548
public const FILEAREA_UPLOADS = 'options';
4649

50+
4751
/**
4852
* Saves all the files in the given draft item to the permanent file area for the given question.
4953
*
@@ -168,9 +172,85 @@ public function get_qpy_files_metadata_from_draftitem(int $userid, int $draftite
168172
size: $file->get_filesize(),
169173
);
170174
}
175+
171176
return $metadata;
172177
}
173178

179+
/**
180+
* Checks that `maxfiles`, `maxbytestotal` and `maxbytesperfile` (or whatever the LMS overrides it with) are fulfilled.
181+
*
182+
* Breaking any of the three by accident should be impossible through UI design or checked by earlier parts of the Moodle code.
183+
* It might be possible if intentional, so we also check it here, but just throw unhelpful {@see coding_exception}s.
184+
*
185+
* `minfiles` is _not_ checked. Since that option isn't really security-relevant, it's left to the package/SDK.
186+
*
187+
* @param file_upload_options|file_upload_element $options
188+
* @param int $contextid
189+
* @param int|null $questionid Question being edited, or `null` if a new question is being created.
190+
* @param int $draftitemid
191+
* @param file_metadata[] $draftfilemetas
192+
* @return void An exception is thrown if the restrictions are not fulfilled.
193+
* @throws coding_exception
194+
*/
195+
public function check_upload_restrictions(
196+
file_upload_options|file_upload_element $options,
197+
int $contextid, ?int $questionid, int $draftitemid, array $draftfilemetas
198+
): void {
199+
global $CFG;
200+
$fs = get_file_storage();
201+
202+
if ($options->maxfiles !== null && count($draftfilemetas) > $options->maxfiles) {
203+
throw new coding_exception("Draft area $draftitemid exceeds limit of $options->maxfiles files.");
204+
}
205+
206+
if (file_is_draft_area_limit_reached($draftitemid, $options->maxbytestotal ?? FILE_AREA_MAX_BYTES_UNLIMITED)) {
207+
throw new coding_exception("Draft area $draftitemid exceeds limit of $options->maxbytestotal bytes.");
208+
}
209+
210+
$coursemaxbytes = 0;
211+
if (!empty($PAGE->course->maxbytes)) {
212+
$coursemaxbytes = $PAGE->course->maxbytes;
213+
}
214+
215+
$effectivemaxbytes = get_user_max_upload_file_size(
216+
context::instance_by_id($contextid),
217+
$CFG->maxbytes,
218+
$coursemaxbytes,
219+
$options->maxbytesperfile ?? FILE_AREA_MAX_BYTES_UNLIMITED
220+
);
221+
222+
if ($effectivemaxbytes === USER_CAN_IGNORE_FILE_SIZE_LIMITS) {
223+
// No need to check then.
224+
return;
225+
}
226+
227+
$existingfiles = $questionid ? $fs->get_area_files(
228+
$contextid,
229+
'qtype_questionpy',
230+
self::FILEAREA_UPLOADS,
231+
$questionid,
232+
includedirs: false
233+
) : [];
234+
$existingfilerefs = array_map(fn($file) => $file->get_filename(), $existingfiles);
235+
236+
foreach ($draftfilemetas as $draftfilemeta) {
237+
if (in_array($draftfilemeta->fileref, $existingfilerefs)) {
238+
// The file (a file with the same content and metadata) had already been successfully uploaded.
239+
// The current user might have changed its path or filename or even moved it to a different upload element, but
240+
// since the original uploader must've been allowed to upload it, we don't check whether the current user would be
241+
// allowed to upload it again.
242+
// TODO: Catch when a file is copied/moved by the package to a different element with a lower file size limit.
243+
// That would probably need to happen at import (when loading the form data from the package), not here.
244+
continue;
245+
}
246+
247+
if ($draftfilemeta->size > $effectivemaxbytes) {
248+
throw new coding_exception("File '{$draftfilemeta->path}{$draftfilemeta->filename}' exceeds limit of"
249+
. " $effectivemaxbytes bytes.");
250+
}
251+
}
252+
}
253+
174254
/**
175255
* Retrieves a saved file from the file area belonging to the given question.
176256
*

classes/local/form/elements/file_upload_element.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ public function __construct(
6666
* @throws moodle_exception
6767
*/
6868
public function render_to(render_context $context): void {
69+
global $PAGE, $CFG;
70+
6971
/** @var MoodleQuickForm_filemanager $element */
7072
$element = $context->add_element('filemanager', $this->name, $context->contextualize($this->label), null, [
7173
'subdirs' => self::SUBDIRS,
@@ -88,8 +90,16 @@ public function render_to(render_context $context): void {
8890
}
8991

9092
global $USER;
93+
$ofs = di::get(options_file_service::class);
9194
/** @var file_metadata[] $filemetas */
92-
$filemetas = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
95+
$filemetas = $ofs->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
96+
$ofs->check_upload_restrictions(
97+
$this,
98+
$context->question->contextid,
99+
$context->question->id ?? null,
100+
$draftitemid,
101+
$filemetas
102+
);
93103

94104
// At this time, we don't know whether the question will be saved or the draft validated etc., and we don't know the
95105
// question id, so we don't save the draft files ourselves. But we do need to let question_service know which draft

classes/local/form/elements/wysiwyg_editor_element.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,13 @@ public function __construct(
8484
* @throws moodle_exception
8585
*/
8686
public function render_to(render_context $context): void {
87+
global $PAGE, $CFG;
88+
89+
$coursemaxbytes = 0;
90+
if (!empty($PAGE->course->maxbytes)) {
91+
$coursemaxbytes = $PAGE->course->maxbytes;
92+
}
93+
8794
$uploadsoptions = match ($this->fileuploads) {
8895
null => [
8996
'enable_filemanagement' => false,
@@ -134,8 +141,16 @@ public function render_to(render_context $context): void {
134141
file_remove_editor_orphaned_files($mydata);
135142

136143
global $USER;
144+
$ofs = di::get(options_file_service::class);
137145
/** @var file_metadata[] $filemetas */
138-
$filemetas = di::get(options_file_service::class)->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
146+
$filemetas = $ofs->get_qpy_files_metadata_from_draftitem($USER->id, $draftitemid);
147+
$ofs->check_upload_restrictions(
148+
$this->fileuploads,
149+
$context->question->contextid,
150+
$context->question->id ?? null,
151+
$draftitemid,
152+
$filemetas
153+
);
139154

140155
$text = self::replace_draftfile_urls_with_qpy_urls($text, $filemetas, $draftitemid);
141156

0 commit comments

Comments
 (0)