Fix to correctly preserve full quoted action params in Fail2Ban jail editor #2647

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## Changelog
 
+#### 2.631 (April, 2026)
+* Fix to correctly preserve full quoted action parameters in the Fail2Ban jail editor [#2647](https://github.com/webmin/webmin/issues/2647)
+
 #### 2.630 (March 24, 2026)
 * Add improvements to user input validation across all modules
 * Update Authentic theme to the latest version with various improvements and fixes:

fail2ban/edit_jail.cgi

@@ -83,18 +83,12 @@ my $atable = &ui_columns_start([
 		]);
 my $i = 0;
 foreach my $a (@{$actionlist->{'words'}}, undef) {
-	my $action;
-	my %opts;
-	if ($a && $a =~ /^(\S.*\S)\[(.*)\]$/) {
-		$action = $1;
-		%opts = map { my ($n, $v) = split(/=/, $_);
-			      $v =~ s/^"(.*)"/$1/;
-			      ($n, $v) } split(/,\s+/, $2);
-		}
-	else {
-		$action = $a;
-		}
-	my @oopts = grep { !/^(name|port|protocol)$/ } (keys %opts);
+	my ($action, $opts) = $a ? &parse_action_definition($a) : (undef, []);
+	my %opts = map { ($_->[0], $_->[1]) } @$opts;
+	my @oopts = grep { $_->[0] !~ /^(name|port|protocol)$/ } @$opts;
+	my $others = join("", map { $_->[2] } @oopts);
+	$others =~ s/^\s*,?\s*//;
+	$others =~ s/\s*,?\s*$//;
 	$atable .= &ui_columns_row([
 		&ui_select("action_$i", $action,
 		   [ [ "", " " ],
@@ -107,8 +101,7 @@ foreach my $a (@{$actionlist->{'words'}}, undef) {
 			     [ 'tcp', 'TCP' ],
 			     [ 'udp', 'UDP' ],
 			     [ 'icmp', 'ICMP' ] ]),
-		&ui_textbox("others_$i",
-			join(" ", map { $_."=".$opts{$_} } @oopts), 40),
+		&ui_textbox("others_$i", $others, 40),
 		]);
 	$i++;
 	}

fail2ban/fail2ban-lib.pl

@@ -222,6 +222,122 @@ while($v =~ /\S/) {
 $dir->{'words'} = \@w;
 }
 
+# parse_action_definition(string)
+# Splits an action definition into a name and list of option triples,
+# each containing a name, parsed value and raw text chunk
+sub parse_action_definition
+{
+my ($str) = @_;
+return (undef, []) if (!defined($str));
+if ($str =~ /^(\S.*\S)\[(.*)\]$/) {
+	return ($1, [ &parse_action_options($2) ]);
+	}
+return ($str, []);
+}
+
+# parse_action_options(string)
+# Parses action options, handling quoted values and comma/space separators
+sub parse_action_options
+{
+my ($str) = @_;
+my @rv;
+my $len = length($str);
+my $i = 0;
+while ($i < $len) {
+	while ($i < $len) {
+		my $ch = substr($str, $i, 1);
+		last if ($ch !~ /[\s,]/);
+		$i++;
+		}
+	last if ($i >= $len);
+	my $start = $i;
+
+	my $name = "";
+	while ($i < $len) {
+		my $ch = substr($str, $i, 1);
+		last if ($ch eq "=" || $ch =~ /[\s,]/);
+		$name .= $ch;
+		$i++;
+		}
+	last if (!length($name));
+
+	while ($i < $len && substr($str, $i, 1) =~ /\s/) {
+		$i++;
+		}
+
+	# Support option names without values, although most action
+	# parameters are expected to be name=value pairs.
+	if ($i >= $len || substr($str, $i, 1) ne "=") {
+		while ($i < $len && substr($str, $i, 1) =~ /\s/) {
+			$i++;
+			}
+		$i++ if ($i < $len && substr($str, $i, 1) eq ",");
+		push(@rv, [ $name, undef, substr($str, $start, $i - $start) ]);
+		next;
+		}
+
+	$i++;
+	while ($i < $len && substr($str, $i, 1) =~ /\s/) {
+		$i++;
+		}
+
+	my ($value, $ni) = &parse_action_option_value($str, $i);
+	push(@rv, [ $name, $value, substr($str, $start, $ni - $start) ]);
+	$i = $ni;
+	}
+return @rv;
+}
+
+# parse_action_option_value(string, index)
+# Returns an option value and the next parse position
+sub parse_action_option_value
+{
+my ($str, $i) = @_;
+my $len = length($str);
+my $value = "";
+if ($i < $len) {
+	my $quote = substr($str, $i, 1);
+	if ($quote eq "'" || $quote eq "\"") {
+		$i++;
+		while ($i < $len) {
+			my $ch = substr($str, $i, 1);
+			if ($ch eq "\\") {
+				if ($i+1 < $len) {
+					my $next = substr($str, $i+1, 1);
+					if ($next eq $quote || $next eq "\\") {
+						$value .= $next;
+						$i += 2;
+						next;
+						}
+					}
+				$value .= $ch;
+				$i++;
+				next;
+				}
+			if ($ch eq $quote) {
+				$i++;
+				last;
+				}
+			$value .= $ch;
+			$i++;
+			}
+		}
+	else {
+		while ($i < $len) {
+			my $ch = substr($str, $i, 1);
+			last if ($ch eq "," || $ch =~ /\s/);
+			$value .= $ch;
+			$i++;
+			}
+		}
+	}
+while ($i < $len && substr($str, $i, 1) =~ /\s/) {
+	$i++;
+	}
+$i++ if ($i < $len && substr($str, $i, 1) eq ",");
+return ($value, $i);
+}
+
 # create_section(file, &section)
 # Add a new section to a file
 sub create_section

fail2ban/list_jails.cgi

@@ -28,7 +28,7 @@ foreach my $j (@jails) {
 	my $action_dir = &find("action", $j);
 	my $action = "";
 	if ($action_dir) {
-		$action = join(" | ",
+		$action = join(", ",
 			map { /^([^\[]+)/; &html_escape("$1") }
 			    @{$action_dir->{'words'}});
 		}

fail2ban/save_jail.cgi

@@ -85,12 +85,15 @@ else {
 		if ($in{"protocol_$i"}) {
 			push(@opts, "protocol=".$in{"protocol_$i"});
 			}
-		foreach my $oo (split(/\s+/, $in{"others_$i"})) {
-			my ($n, $v) = split(/=/, $oo, 2);
-			$v = "\"$v\"" if ($v =~ /\s|,|=/ && $v !~ /['"]/);
-			push(@opts, "$n=$v");
+		if ($in{"others_$i"}) {
+			my $others = $in{"others_$i"};
+			$others =~ s/^\s+//;
+			$others =~ s/\s+$//;
+			push(@opts, $others) if (length($others));
 			}
-		push(@actions, $in{"action_$i"}."[".join(", ", @opts)."]");
+		my $action = $in{"action_$i"};
+		$action .= "[".join(", ", @opts)."]" if (@opts);
+		push(@actions, $action);
 		}
 
 	# Split and validate log file paths