Fix messages for existing files or files outside the course directory.

This makes it so that the first thirty files in both lists are shown. If there are more than thirty, then the last item in the list will say that there are more files not shown. To make this work a `min` method was added to Utils.pm (there was a max method, but no min method?), and the change in openwebwork#2194 to make good and bad messages be in a `div` instead of a `p` was added here too.
pstaabp · Sep 5, 2023 · 97a8263 · 97a8263
1 parent f351e9b
commit 97a8263
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 60 deletions.
diff --git a/htdocs/themes/math4/math4.scss b/htdocs/themes/math4/math4.scss
@@ -456,7 +456,7 @@ h2.page-title {
 	gap: 0.25rem;
 	margin: 0 0 0.5rem;
 
-	p {
+	div {
 		margin: 0;
 	}
 }

diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm
@@ -265,8 +265,9 @@ message() template escape handler.
 
 sub addgoodmessage ($c, $message) {
 	$c->addmessage($c->tag(
-		'p',
+		'div',
 		class => 'alert alert-success alert-dismissible fade show ps-1 py-1',
+		role  => 'alert',
 		$c->c(
 			$message,
 			$c->tag(
@@ -290,8 +291,9 @@ message() template escape handler.
 
 sub addbadmessage ($c, $message) {
 	$c->addmessage($c->tag(
-		'p',
+		'div',
 		class => 'alert alert-danger alert-dismissible fade show ps-1 py-1',
+		role  => 'alert',
 		$c->c(
 			$message,
 			$c->tag(

diff --git a/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/FileManager.pm
@@ -31,7 +31,7 @@ use Archive::Tar;
 use Archive::Zip qw(:ERROR_CODES);
 use Archive::Zip::SimpleZip qw($SimpleZipError);
 
-use WeBWorK::Utils qw(readDirectory readFile sortByName listFilesRecursive);
+use WeBWorK::Utils qw(readDirectory readFile sortByName listFilesRecursive min);
 use WeBWorK::Upload;
 use WeBWorK::Utils::CourseManagement qw(archiveCourse);
 
@@ -449,8 +449,7 @@ sub UnpackArchive ($c) {
 sub unpack_archive ($c, $archive) {
 	my $dir = Mojo::File->new($c->{courseRoot}, $c->{pwd});
 
-	# Used for determining non-existing and existing files.
-	my (@members, @existing_files);
+	my (@members, @existing_files, @outside_files);
 	my $num_extracted = 0;
 
 	if ($archive =~ m/\.zip$/) {
@@ -465,8 +464,7 @@ sub unpack_archive ($c, $archive) {
 			$c->addbadmessage($error);
 		});
 
-		my @members = $zip->members;
-		my @outside_files;
+		@members = $zip->members;
 		for (@members) {
 			my $out_file = $dir->child($_->fileName)->realpath;
 			if ($out_file !~ /^$dir/) {
@@ -481,26 +479,7 @@ sub unpack_archive ($c, $archive) {
 			++$num_extracted if $zip->extractMember($_ => $out_file->to_string) == AZ_OK;
 		}
 
-		if (@outside_files) {
-			if (scalar(@outside_files) == 1) {
-				$c->addbadmessage($c->maketext(
-					'The file "[_1]" can not be safely unpacked as it is outside the current working directory.',
-					$outside_files[0]
-				));
-			} else {
-				$c->addbadmessage($c->maketext(
-					'There are [_1] files that already exist including [_2] and [_3]. '
-						. 'Check "Overwrite existing files silently" to unpack these files',
-					scalar(@outside_files),
-					$outside_files[0],
-					$outside_files[1]
-				));
-			}
-		}
-
 		Archive::Zip::setErrorHandler();
-
-		$c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', $num_extracted)) if $num_extracted;
 	} elsif ($archive =~ m/\.(tar(\.gz)?|tgz)$/) {
 		local $Archive::Tar::WARN = 0;
 
@@ -512,43 +491,87 @@ sub unpack_archive ($c, $archive) {
 
 		$tar->setcwd($dir->to_string);
 
-		my @members = $tar->list_files;
+		@members = $tar->list_files;
 		for (@members) {
+			my $out_file = $dir->child($_)->realpath;
+			if ($out_file !~ /^$dir/) {
+				push(@outside_files, $_);
+				next;
+			}
+
 			if (!$c->param('overwrite') && -e $dir->child($_)) {
 				push(@existing_files, $_);
 				next;
 			}
+
 			unless ($tar->extract_file($_)) {
 				$c->addbadmessage($tar->error);
 				next;
 			}
 			++$num_extracted;
 		}
-
-		$c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', $num_extracted)) if $num_extracted;
 	} else {
 		$c->addbadmessage($c->maketext('Unsupported archive type in file "[_1]"', $archive));
 		return 0;
 	}
 
-	if (@existing_files) {
-		if (scalar(@existing_files) == 1) {
-			$c->addbadmessage($c->maketext(
-				'The file "[_1]" already exists. '
-					. 'Check "Overwrite existing files silently" to unpack this file.',
-				$existing_files[0]
-			));
-		} else {
-			$c->addbadmessage($c->maketext(
-				'There are [_1] files that already exist including [_2] and [_3]. '
-					. 'Check "Overwrite existing files silently" to unpack these files',
-				scalar(@existing_files),
-				$existing_files[0],
-				$existing_files[1]
-			));
-		}
+	if (@outside_files) {
+		$c->addbadmessage(
+			$c->tag(
+				'p',
+				$c->maketext(
+					'The following [plural,_1,file is,files are] outside the current working directory '
+						. 'and can not be safely unpacked.',
+					scalar(@outside_files),
+				)
+				)
+				. $c->tag(
+					'div',
+					$c->tag(
+						'ul',
+						$c->c(
+							(map { $c->tag('li', $_) } @outside_files[ 0 .. min(29, $#outside_files) ]),
+							(
+								@outside_files > 30
+								? $c->tag('li',
+									$c->maketext('[quant,_1,more file,more files] not shown', @outside_files - 30))
+								: ()
+							)
+					)->join('')
+					)
+				)
+		);
 	}
 
+	if (@existing_files) {
+		$c->addbadmessage(
+			$c->tag(
+				'p',
+				$c->maketext(
+					'The following [plural,_1,file already exists,files already exist]. '
+						. 'Check "Overwrite existing files silently" to unpack [plural,_1,this file,these files].',
+					scalar(@existing_files),
+				)
+				)
+				. $c->tag(
+					'div',
+					$c->tag(
+						'ul',
+						$c->c(
+							(map { $c->tag('li', $_) } @existing_files[ 0 .. min(29, $#existing_files) ]),
+							(
+								@existing_files > 30
+								? $c->tag('li',
+									$c->maketext('[quant,_1,more file,more files] not shown', @existing_files - 30))
+								: ()
+							)
+					)->join('')
+					)
+				)
+		);
+	}
+
+	$c->addgoodmessage($c->maketext('[quant,_1,file] unpacked successfully', $num_extracted)) if $num_extracted;
 	return $num_extracted == @members;
 }
 
@@ -630,9 +653,8 @@ sub Upload ($c) {
 				$c->Confirm(
 					$c->tag(
 						'p',
-						$c->b(
-							$c->maketext('File <b>[_1]</b> already exists. Overwrite it, or rename it as:', $name)
-						)
+						$c->b($c->maketext(
+							'File <b>[_1]</b> already exists. Overwrite it, or rename it as:', $name))
 					),
 					uniqueName($dir, $name),
 					$c->maketext('Rename'),
@@ -768,9 +790,8 @@ sub directoryListing ($c, $pwd) {
 		for my $name (@values) {
 			my $file = "$dir/$name->[1]";
 			my ($size, $date) = (lstat($file))[ 7, 9 ];
-			$name->[0] =
-				$c->b(
-					sprintf("%-${len}s%-16s%10s", $name->[0], -d $file ? ('', '') : (getDate($date), getSize($size)))
+			$name->[0] = $c->b(
+				sprintf("%-${len}s%-16s%10s", $name->[0], -d $file ? ('', '') : (getDate($date), getSize($size)))
 					=~ s/\s/&nbsp;/gr);
 		}
 	}

diff --git a/lib/WeBWorK/Utils.pm b/lib/WeBWorK/Utils.pm
@@ -81,6 +81,7 @@ our @EXPORT_OK = qw(
 	list2hash
 	listFilesRecursive
 	makeTempDirectory
+	min
 	max
 	nfreeze_base64
 	not_blank
@@ -1049,15 +1050,22 @@ sub thaw_base64 {
 
 }
 
-sub max(@) {
-	my $soFar;
-	foreach my $item (@_) {
-		$soFar = $item unless defined $soFar;
-		if ($item > $soFar) {
-			$soFar = $item;
-		}
+sub min {
+	my @items = @_;
+	my $min   = (shift @items) // 0;
+	for my $item (@items) {
+		$min = $item if ($item < $min);
+	}
+	return $min;
+}
+
+sub max {
+	my @items = @_;
+	my $max   = (shift @items) // 0;
+	for my $item (@items) {
+		$max = $item if ($item > $max);
 	}
-	return defined $soFar ? $soFar : 0;
+	return $max;
 }
 
 sub wwRound(@) {