Merge pull request #2600 from NOYB/Firewall_/_Rules_-_Page_Efficiency_Upgrade

8 files changed, 136 insertions, 62 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..81b7a7a 100644
--- a/src/etc/inc/filter.inc
+++ b/src/etc/inc/filter.inc
@@ -4236,4 +4236,40 @@ function filter_get_antilockout_ports($wantarray = false) {
 
 }
 
+/* get rule index within interface */
+function ifridx($if, $ridx) {
+	global $config;
+
+	if ($ridx < 0) {
+		return $ridx;
+	}
+	
+	$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;
+}
+
+/* display rules separators */
+function display_separator($separators, $nrules, $columns_in_table) {
+	if (!empty($separators)) {
+		foreach ($separators 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");
+			}
+		}
+	}
+}
+
 ?>
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..1ccb6d9 100644
--- a/src/etc/inc/upgrade_config.inc
+++ b/src/etc/inc/upgrade_config.inc
@@ -4410,4 +4410,45 @@ function upgrade_141_to_142() {
 	}
 }
 
+function upgrade_142_to_143() {
+	global $config;
+
+	/* Re-index firewall rule separators per interface */
+	if (!empty($config['filter']['separator'])) {
+		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++;
+				}
+			}
+		}
+	}
+
+	/* Re-index nat rule separators */
+	if (!empty($config['nat']['separator'])) {
+		foreach ($config['nat']['separator'] as $sepn => $separator) {
+			$seprow = substr($separator['row']['0'], 2);
+			$config['nat']['separator'][$sepn]['row'] = 'fr' . ($seprow + 1);
+		}
+	}
+}
+
 ?>
diff --git a/src/usr/local/www/firewall_nat.php b/src/usr/local/www/firewall_nat.php
index ee0315c..80f603e 100644
--- a/src/usr/local/www/firewall_nat.php
+++ b/src/usr/local/www/firewall_nat.php
@@ -145,7 +145,7 @@ if ($_GET['act'] == "del") {
 
 		for ($idx=0; isset($a_separators['sep' . $idx]); $idx++ ) {
 			$seprow = substr($a_separators['sep' . $idx]['row']['0'], 2);
-			if ($seprow >= $_GET['id']) {
+			if ($seprow > $_GET['id']) {
 				$a_separators['sep' . $idx]['row']['0'] = 'fr' . ($seprow - 1);
 			}
 		}
@@ -182,7 +182,7 @@ if (isset($_POST['del_x'])) {
 			// 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 > $rulei) {
 					$a_separators['sep' . $idx]['row']['0'] = 'fr' . ($seprow - 1);
 				}
 			}
@@ -258,15 +258,10 @@ $columns_in_table = 13;
 <?php
 
 $nnats = $i = 0;
+$separators = $config['nat']['separator'];
 
 // There can be a separator before any rules are listed
-if ($config['nat']['separator']['sep0']['row'][0] == "fr-1") {
-	$cellcolor = $config['nat']['separator']['sep0']['color'];
-	print('<tr class="ui-sortable-handle separator">' .
-		'<td class="' . $cellcolor . '" colspan="' . ($columns_in_table -1) . '">' . '<span class="' . $cellcolor . '">' . $config['nat']['separator']['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($separators, $nnats, $columns_in_table);
 
 foreach ($a_nat as $natent):
 
@@ -428,20 +423,12 @@ foreach ($a_nat as $natent):
 						</td>
 					</tr>
 <?php
-
-		if (isset($config['nat']['separator']['sep0'])) {
-			foreach ($config['nat']['separator'] as $rulesep) {
-				if ($rulesep['row']['0'] == "fr" . $nnats) {
-					$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");
-				}
-			}
-		}
 	$i++;
 	$nnats++;
+
+	// There can be a separator before the next rule listed, or after the last rule listed
+	display_separator($separators, $nnats, $columns_in_table);
+
 endforeach;
 ?>
 				</tbody>
diff --git a/src/usr/local/www/firewall_rules.php b/src/usr/local/www/firewall_rules.php
index 2e15ef9..a65fab5 100644
--- a/src/usr/local/www/firewall_rules.php
+++ b/src/usr/local/www/firewall_rules.php
@@ -220,12 +220,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 +259,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 +300,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 +486,17 @@ $columns_in_table = 13;
 			<tbody class="user-entries">
 <?php
 $nrules = 0;
-$seps = 0;
+$separators = $config['filter']['separator'][strtolower($if)];
 
 // 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($separators, $nrules, $columns_in_table);
 
 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>
@@ -789,20 +799,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($separators, $nrules, $columns_in_table);
+	}
+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..715d8e6 100644
--- a/src/usr/local/www/firewall_rules_edit.php
+++ b/src/usr/local/www/firewall_rules_edit.php
@@ -913,14 +913,23 @@ if ($_POST) {
 			if (is_numeric($after)) {
 				array_splice($a_filter, $after+1, 0, array($filterent));
 
+				if (isset($pconfig['floating'])) {
+					$tmpif = 'FloatingRules';
+				} else {
+					$tmpif = $if;
+				}
+
+				// get rule index within interface
+				$ifridx = ifridx($tmpif, $after);
+
 				// Update the separators
-				$a_separators = &$config['filter']['separator'][strtolower($if)];
+				$a_separators = &$config['filter']['separator'][strtolower($tmpif)];
 
 				for ($idx=0; isset($a_separators['sep' . $idx]); $idx++ ) {
 					$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) {
 						$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++;
+				}
 			}
 		});
 	}