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.

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 1f1c320..e51fed3 100644
--- a/src/usr/local/www/firewall_rules_edit.php
+++ b/src/usr/local/www/firewall_rules_edit.php
@@ -1281,9 +1281,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);
@@ -1881,20 +1881,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');
@@ -1912,7 +1904,7 @@ events.push(function() {
 
 		ext_change();
 
-		if ($('#proto').find(":selected").index() <= 2) {
+		if (is_tcpudp) {
 			hideClass('dstprtr', false);
 			hideInput('btnsrcadv', false);
 			if ((($('#srcbeginport').val() == "any") || ($('#srcbeginport').val() == "")) &&