Bugfixes

1. On creating a new rule, $pconfig['ipprotocol'] is undefined, rather than defaults to what is seen in GUI (IPv4). Form generation logic for the ICMPType list box can't rely on a good value. It was fixed late here and missed when copying changes to Github.  Very likely responsible for above issue by @rbgarga . Please confirm if this fixes it for you. On the off-chance that it still doesn't, can you let me know if _editing an existing rule_ works, which will help.

2. Reordering #proto options affects JS logic, because JS uses index() to identify which protocol is selected.  Generally I feel this isn't the best practice, if the value is what matters then it's better and easier to review, if the code references the value itself (.val()) not the position in the list which could change (.index()). That said, I should have spotted this anyway.
(cherry picked from commit 640462d20b4d06b8ba14b9ce300c218b14998aef)

1 files changed, 7 insertions, 15 deletions

diff --git a/src/usr/local/www/firewall_rules_edit.php b/src/usr/local/www/firewall_rules_edit.php
index 715b5c9..ede74bd 100644
--- a/src/usr/local/www/firewall_rules_edit.php
+++ b/src/usr/local/www/firewall_rules_edit.php
@@ -1318,9 +1318,9 @@ $group->add(new Form_Select(
 	'icmptype',
 	'ICMP subtypes',
 	((isset($pconfig['icmptype']) && strlen($pconfig['icmptype']) > 0) ? explode(',', $pconfig['icmptype']) : 'any'),
-	$icmplookup[$pconfig['ipprotocol']]['icmptypes'],
+	isset($icmplookup[$pconfig['ipprotocol']]) ? $icmplookup[$pconfig['ipprotocol']]['icmptypes'] : array('any' => gettext('any')),
 	true
-))->setHelp('<div id="icmptype_help">' . gettext($icmplookup[$pconfig['ipprotocol']]['helpmsg']) . '</div>');
+))->setHelp('<div id="icmptype_help">' . (isset($icmplookup[$pconfig['ipprotocol']]) ? gettext($icmplookup[$pconfig['ipprotocol']]['helpmsg']) : '') . '</div>');
 $group->addClass('icmptype_section');
 
 $section->add($group);
@@ -1915,20 +1915,12 @@ events.push(function() {
 	}
 
 	function proto_change() {
-		if ($('#proto').find(":selected").index() < 3) {
-			portsenabled = 1;
-			hideClass('tcpflags', false);
-		} else {
-			portsenabled = 0;
-			hideClass('tcpflags', true);
-		}
+		var is_tcpudp = (jQuery.inArray($('#proto :selected').val(), ['tcp','udp', 'tcp/udp']) != -1);
+		portsenabled = (is_tcpudp ? 1 : 0);
+		hideClass('tcpflags', !is_tcpudp);
 
 		// Disable OS if the proto is not TCP.
-		if ($('#proto').find(":selected").index() < 1) {
-			disableInput('os', false);
-		} else {
-			disableInput('os', true);
-		}
+		disableInput('os', ($('#proto :selected').val() != 'tcp'));
 
 		// Hide ICMP types if not icmp rule
 		hideClass('icmptype_section', $('#proto').val() != 'icmp');
@@ -1946,7 +1938,7 @@ events.push(function() {
 
 		ext_change();
 
-		if ($('#proto').find(":selected").index() <= 2) {
+		if (is_tcpudp) {
 			hideClass('dstprtr', false);
 			hideInput('btnsrctoggle', false);
 			if ((($('#srcbeginport').val() == "any") || ($('#srcbeginport').val() == "")) &&