Firewall / Rules - Page Efficiency Upgrade

1) Only load the rules of the selected interface instead of loading all the rules of all interfaces and hiding them.  Very inefficient and can result in sluggishness.  Especially with large number of rules and/or large alias tables.

1.1) Rule separators indexed to their position within their interface so they work with only loading the selected interface rules.

2) Supports consecutive separators.

3) Fixes #5559 - horizontal scrollbar when no rules to display (all hidden).  No longer hiding rules.

4) Corrects case where a single rule defined on any interface would prevent the alert messages from being displayed on all interfaces.

7 files changed, 107 insertions, 40 deletions

diff --git a/src/conf.default/config.xml b/src/conf.default/config.xml
index d09cd06..f77bd89 100644
--- a/src/conf.default/config.xml
+++ b/src/conf.default/config.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <pfsense>
-	<version>14.2</version>
+	<version>14.3</version>
 	<lastchange/>
 	<system>
 		<optimization>normal</optimization>
diff --git a/src/etc/inc/filter.inc b/src/etc/inc/filter.inc
index efa0d09..f41c091 100644
--- a/src/etc/inc/filter.inc
+++ b/src/etc/inc/filter.inc
@@ -4236,4 +4236,21 @@ function filter_get_antilockout_ports($wantarray = false) {
 
 }
 
+// get rule index within interface
+function ifridx($if, $ridx) {
+	global $config;
+
+	$i = $ifridx = 0;
+	foreach ($config['filter']['rule'] as $rulen => $filterent) {
+		if (($filterent['interface'] == $if && !isset($filterent['floating'])) || (isset($filterent['floating']) && "FloatingRules" == $if)) {
+			if ($i == $ridx) {
+				return $ifridx;
+			}
+			$ifridx++;
+		}
+		$i++;
+	}
+	return $i;
+}
+
 ?>
diff --git a/src/etc/inc/globals.inc b/src/etc/inc/globals.inc
index bcb4043..a9d2ef5 100644
--- a/src/etc/inc/globals.inc
+++ b/src/etc/inc/globals.inc
@@ -99,7 +99,7 @@ $g = array(
 	"disablecrashreporter" => false,
 	"crashreporterurl" => "https://crashreporter.pfsense.org/crash_reporter.php",
 	"debug" => false,
-	"latest_config" => "14.2",
+	"latest_config" => "14.3",
 	"nopkg_platforms" => array("cdrom"),
 	"minimum_ram_warning" => "101",
 	"minimum_ram_warning_text" => "128 MB",
diff --git a/src/etc/inc/upgrade_config.inc b/src/etc/inc/upgrade_config.inc
index 0848d0e..d3abb0b 100644
--- a/src/etc/inc/upgrade_config.inc
+++ b/src/etc/inc/upgrade_config.inc
@@ -4410,4 +4410,35 @@ function upgrade_141_to_142() {
 	}
 }
 
+function upgrade_142_to_143() {
+	global $config;
+	/* Index firewall rule separators per interface */
+
+	foreach ($config['filter']['separator'] as $interface => $separators) {
+
+		foreach ($separators as $sepn => $separator) {
+
+			$seprow = substr($separator['row']['0'], 2);
+			$sepif  = $separator['if'];
+
+			// Determine position of separator within the interface rules.
+			$i = -1; $j = 0;
+			foreach ($config['filter']['rule'] as $rulen => $filterent) {
+
+				if ($i == $seprow) {
+					// Set separator row to it's position within the interface rules.
+					$config['filter']['separator'][$sepif][$sepn]['row'] = 'fr' . $j;
+					continue 2;	// Advance to next separator
+				}
+
+				// Position within the interface rules.
+				if (($filterent['interface'] == $sepif && !isset($filterent['floating'])) || (isset($filterent['floating']) && "floatingrules" == $sepif)) {
+					$j++;
+				}
+				$i++;
+			}
+		}
+	}
+}
+
 ?>
diff --git a/src/usr/local/www/firewall_rules.php b/src/usr/local/www/firewall_rules.php
index d719e7c..b64b506 100644
--- a/src/usr/local/www/firewall_rules.php
+++ b/src/usr/local/www/firewall_rules.php
@@ -133,6 +133,22 @@ function delete_nat_association($id) {
 	}
 }
 
+function display_separator() {
+	global $config, $if, $nrules, $columns_in_table;
+
+	if (!empty($config['filter']['separator'][strtolower($if)])) {
+		foreach ($config['filter']['separator'][strtolower($if)] as $sepn => $separator) {
+			if ($separator['row'][0] == "fr" . $nrules) {
+				$cellcolor = $separator['color'];
+				print('<tr class="ui-sortable-handle separator">' .
+					'<td class="' . $cellcolor . '" colspan="' . ($columns_in_table -1) . '">' . '<span class="' . $cellcolor . '">' . $separator['text'] . '</span></td>' .
+					'<td  class="' . $cellcolor . '"><a href="#"><i class="fa fa-trash no-confirm sepdel" title="delete this separator"></i></a></td>' .
+					'</tr>' . "\n");
+			}
+		}
+	}
+}
+
 if (!is_array($config['filter']['rule'])) {
 	$config['filter']['rule'] = array();
 }
@@ -220,12 +236,15 @@ if ($_GET['act'] == "del") {
 		}
 		unset($a_filter[$_GET['id']]);
 
+		// get rule index within interface
+		$ifridx = ifridx($if, $_GET['id']);
+
 		// Update the separators
 		$a_separators = &$config['filter']['separator'][strtolower($if)];
 
 		for ($idx=0; isset($a_separators['sep' . $idx]); $idx++ ) {
 			$seprow = substr($a_separators['sep' . $idx]['row']['0'], 2);
-			if ($seprow >= $_GET['id']) {
+			if ($seprow > $ifridx) {
 				$a_separators['sep' . $idx]['row']['0'] = 'fr' . ($seprow - 1);
 			}
 		}
@@ -256,10 +275,13 @@ if (isset($_POST['del_x'])) {
 			unset($a_filter[$rulei]);
 			$deleted = true;
 
+			// get rule index within interface
+			$ifridx = ifridx($if, $rulei);
+
 			// Update the separators
 			for ($idx=0; isset($a_separators['sep' . $idx]); $idx++ ) {
 				$seprow = substr($a_separators['sep' . $idx]['row']['0'], 2);
-				if ($seprow >= $rulei) {
+				if ($seprow > $ifridx) {
 					$a_separators['sep' . $idx]['row']['0'] = 'fr' . ($seprow - 1);
 				}
 			}
@@ -294,11 +316,26 @@ if (isset($_POST['del_x'])) {
 	if (is_array($_POST['rule']) && !empty($_POST['rule'])) {
 		$a_filter_new = array();
 
+		// get the rules of other interfaces listed in config before this interface.
+		for ($i = 0; (isset($a_filter[$i]) && 
+			(($a_filter[$i]['interface'] != $if && !isset($a_filter[$i]['floating'])) || (isset($a_filter[$i]['floating']) && "FloatingRules" != $if)) 
+			); $i++) {
+			$a_filter_new[] = $a_filter[$i];
+		}
+
+		// include the rules of this interface.
 		// if a rule is not in POST[rule], it has been deleted by the user
 		foreach ($_POST['rule'] as $id) {
 			$a_filter_new[] = $a_filter[$id];
 		}
 
+		// get the rules of other interfaces listed in config after this interface.
+		for ( ; (isset($a_filter[$i])); $i++) {
+			if (($a_filter[$i]['interface'] != $if && !isset($a_filter[$i]['floating'])) || (isset($a_filter[$i]['floating']) && "FloatingRules" != $if)) {
+				$a_filter_new[] = $a_filter[$i];
+			}
+		}
+
 		$a_filter = $a_filter_new;
 
 		$config['filter']['separator'][strtolower($if)] = "";
@@ -465,28 +502,16 @@ $columns_in_table = 13;
 			<tbody class="user-entries">
 <?php
 $nrules = 0;
-$seps = 0;
 
 // There can be a separator before any rules are listed
-if ($config['filter']['separator'][strtolower($if)]['sep0']['row'][0] == "fr-1") {
-	$cellcolor = $config['filter']['separator'][strtolower($if)]['sep0']['color'];
-	print('<tr class="ui-sortable-handle separator">' .
-		'<td class="' . $cellcolor . '" colspan="' . ($columns_in_table -1) . '">' . '<span class="' . $cellcolor . '">' . $config['filter']['separator'][strtolower($if)]['sep0']['text'] . '</span></td>' .
-		'<td  class="' . $cellcolor . '"><a href="#"><i class="fa fa-trash no-confirm sepdel" title="delete this separator"></i></a></td>' .
-		'</tr>' . "\n");
-}
+display_separator();
 
 for ($i = 0; isset($a_filter[$i]); $i++):
 	$filterent = $a_filter[$i];
 
-	if (($filterent['interface'] != $if && !isset($filterent['floating'])) || (isset($filterent['floating']) && "FloatingRules" != $if)) {
-		$display = 'style="display: none;"';
-	} else {
-		$display = "";
-	}
-
+	if (($filterent['interface'] == $if && !isset($filterent['floating'])) || (isset($filterent['floating']) && "FloatingRules" == $if)) {
 ?>
-					<tr id="fr<?=$nrules;?>" <?=$display?> onClick="fr_toggle(<?=$nrules;?>)" ondblclick="document.location='firewall_rules_edit.php?id=<?=$i;?>';" <?=(isset($filterent['disabled']) ? ' class="disabled"' : '')?>>
+					<tr id="fr<?=$nrules;?>" onClick="fr_toggle(<?=$nrules;?>)" ondblclick="document.location='firewall_rules_edit.php?id=<?=$i;?>';" <?=(isset($filterent['disabled']) ? ' class="disabled"' : '')?>>
 						<td>
 							<input type="checkbox" id="frc<?=$nrules;?>" onClick="fr_toggle(<?=$nrules;?>)" name="rule[]" value="<?=$i;?>"/>
 						</td>
@@ -785,20 +810,11 @@ for ($i = 0; isset($a_filter[$i]); $i++):
 						</td>
 					</tr>
 <?php
-		if (isset($config['filter']['separator'][strtolower($if)]['sep0'])) {
-			foreach ($config['filter']['separator'][strtolower($if)] as $rulesep) {
-				if ($rulesep['row']['0'] == "fr" . $nrules) {
-					$cellcolor = $rulesep['color'];
-					print('<tr class="ui-sortable-handle separator">' .
-						'<td class="' . $cellcolor . '" colspan="' . ($columns_in_table -1) . '">' . '<span class="' . $cellcolor . '">' . $rulesep['text'] . '</span></td>' .
-						'<td  class="' . $cellcolor . '"><a href="#"><i class="fa fa-trash no-confirm sepdel" title="delete this separator"></i></a></td>' .
-						'</tr>' . "\n");
-				}
-			}
-		}
-
 		$nrules++;
-		endfor;
+		// There can be a separator before the next rule listed, or after the last rule listed
+		display_separator();
+	}
+endfor;
 ?>
 				</tbody>
 			</table>
diff --git a/src/usr/local/www/firewall_rules_edit.php b/src/usr/local/www/firewall_rules_edit.php
index ad87e18..cdd33c3 100644
--- a/src/usr/local/www/firewall_rules_edit.php
+++ b/src/usr/local/www/firewall_rules_edit.php
@@ -913,6 +913,9 @@ if ($_POST) {
 			if (is_numeric($after)) {
 				array_splice($a_filter, $after+1, 0, array($filterent));
 
+				// get rule index within interface
+				$ifridx = ifridx($if, $after);
+
 				// Update the separators
 				$a_separators = &$config['filter']['separator'][strtolower($if)];
 
@@ -920,7 +923,7 @@ if ($_POST) {
 					$seprow = substr($a_separators['sep' . $idx]['row']['0'], 2);
 
 					// If the separator is located after the place where the new rule is to go, increment the separator row
-					if ($seprow > $after) {
+					if (($seprow > $ifridx) || ($after == -1)) {
 						$a_separators['sep' . $idx]['row']['0'] = 'fr' . ($seprow + 1);
 					}
 				}
diff --git a/src/usr/local/www/jquery/pfSenseHelpers.js b/src/usr/local/www/jquery/pfSenseHelpers.js
index 76aaf51..5d33d83 100644
--- a/src/usr/local/www/jquery/pfSenseHelpers.js
+++ b/src/usr/local/www/jquery/pfSenseHelpers.js
@@ -553,15 +553,15 @@ $('.container .panel-heading a[data-toggle="collapse"]').each(function (idx, el)
 
 	// Compose an inout array containing the row #, color and text for each separator
 	function save_separators() {
-		var seprow = 0;
+		var row = 0;
 		var sepinput;
 		var sepnum = 0;
 
 		$('#ruletable > tbody > tr').each(function() {
 			if ($(this).hasClass('separator')) {
-				seprow = $(this).prev('tr').attr("id");
+				seprow = $(this).next('tr').attr("id");
 				if (seprow == undefined) {
-					seprow = "fr-1";
+					seprow = "fr" + row;
 				}
 
 				sepinput = '<input type="hidden" name="separator[' + sepnum + '][row]" value="' + seprow + '"></input>';
@@ -573,10 +573,10 @@ $('.container .panel-heading a[data-toggle="collapse"]').each(function (idx, el)
 				sepinput = '<input type="hidden" name="separator[' + sepnum + '][if]" value="' + iface + '"></input>';
 				$('form').append(sepinput);
 				sepnum++;
-			}
-
-			if ($(this).parent('tbody').hasClass('user-entries')) {
-				seprow++;
+			} else {
+				if ($(this).parent('tbody').hasClass('user-entries')) {
+					row++;
+				}
 			}
 		});
 	}