The original code did a mixed work: the part in interfaces_assign.php first renamed the interfaces, and then called cleanup_opt_interfaces_after_removal(). The latter didn't do anything at all: it never entered the loop, it didn't save the result of str_replace, it didn't save the resulting config after the processing. And if it had worked, it would have renamed the interfaces a second time as a side effect, completely messing-up the config.

Ticket #1532

2 files changed, 20 insertions, 21 deletions

diff --git a/etc/inc/filter.inc b/etc/inc/filter.inc
index 576f8d1..6f23345 100644
--- a/etc/inc/filter.inc
+++ b/etc/inc/filter.inc
@@ -1337,10 +1337,12 @@ function generate_user_filter_rule_arr($rule, $ngcounter) {
 	$line = generate_user_filter_rule($rule, $ngcounter);
 	$ret['rule'] = $line;
 	$ret['interface'] = $rule['interface'];
-	if($rule['descr'] != "" and $line != "")
-		$ret['descr'] = "label \"USER_RULE: " . str_replace('"', '', $rule['descr']) . "\"";
-	else
-		$ret['descr'] = "label \"USER_RULE\"";
+	if ($line[0] != '#') {
+		if($rule['descr'] != "" and $line != "")
+			$ret['descr'] = "label \"USER_RULE: " . str_replace('"', '', $rule['descr']) . "\"";
+		else
+			$ret['descr'] = "label \"USER_RULE\"";
+	}
 	$ret['ackq'] = get_ack_queue($rule['interface']);
 
 	return $ret;
@@ -1393,7 +1395,7 @@ function generate_user_filter_rule($rule, $ngcounter) {
 
 		/* don't include disabled rules */
 		if (isset($rule['disabled'])) {
-			return "# rule " . $rule['descr'] . " disabled \n";
+			return "# rule " . $rule['descr'] . " disabled ";
 		}
 
 		$pptpdcfg = $config['pptpd'];
@@ -1431,6 +1433,8 @@ function generate_user_filter_rule($rule, $ngcounter) {
 			if($config['pppoe']['n_pppoe_units'] <> "")
 				$nif = $config['pppoe']['n_pppoe_units'];
 			$ispppoe = true;
+		} else if(!isset($rule['interface'])) {
+			return '# Interface empty for rule: '.$rule['descr'];
 		} else {
 
 			/* Check to see if the interface is opt and in our opt list */
diff --git a/etc/inc/pfsense-utils.inc b/etc/inc/pfsense-utils.inc
index 38228bd..bfc3808 100644
--- a/etc/inc/pfsense-utils.inc
+++ b/etc/inc/pfsense-utils.inc
@@ -2452,21 +2452,15 @@ function cleanup_opt_interfaces_after_removal($opt_interface_num) {
 	unlink_if_exists("{$g['tmp_path']}/config.cache");
 	$config_file = file_get_contents("/cf/conf/config.xml");
 	/* loop through and reassign deleted items */
-	for ($i = 500; isset ($config['interfaces']['opt' . $i]); $i--) {
-		if ($i < $opt_interface_num)
-			break;
-		if ($i == $opt_interface_num) {
-			/* item should be deleted */
-			str_replace("opt" . $i, "optXXXX", $config_file);
-		}
-	}
-	/* loop through and reassign optional items */
-	for ($i = 500; isset ($config['interfaces']['opt' . $i]); $i--) {
-		if ($i < $opt_interface_num)
-			break;
-		/* replace opt$i with $i -1 */
-		str_replace("opt" . $i, "opt" . ($i -1), $config_file);
-	}
+	$orig = array('opt'.$opt_interface_num,'OPT'.$opt_interface_num);
+	$repl = array('optXXXX','OPTXXXX');
+	for ($i = $opt_interface_num+1; isset ($config['interfaces']['opt' . $i]); $i++) {
+		array_push($orig,'opt'.$i);
+		array_push($repl,'opt'.($i -1));
+		array_push($orig,'OPT'.$i);
+		array_push($repl,'OPT'.($i -1));
+	}
+	$config_file = str_replace($orig, $repl, $config_file);
 	$fd = fopen("/cf/conf/config.xml", "w");
 	fwrite($fd, $config_file);
 	fclose($fd);
@@ -2490,6 +2484,7 @@ function cleanup_opt_interfaces_after_removal($opt_interface_num) {
 			if($config['nat']['rule'][$x]['interface'] == "optXXXX")
 		 		unset($config['nat']['rule'][$x]['interface']);
 	}
+	write_config();
 	conf_mount_ro();
 	config_unlock();
 	return true;
@@ -3622,4 +3617,4 @@ function is_wan_interface_up($interface) {
 	return false;
 }
 
-?>
\ No newline at end of file
+?>