inc: refactor code and improve error messages of parse_filter_line()

src/etc/inc/filter_log.inc

@@ -121,18 +121,18 @@ function in_arrayi($needle, $haystack) {
 	return in_array(strtolower($needle), array_map('strtolower', $haystack));
 }
 
-function parse_filter_line($line) {
-	global $config, $g;
-
+function parse_filter_line($line)
+{
 	$flent = array();
-	$log_split = "";
+	$log_split = '';
 
-	if (!preg_match("/(.*)\s(.*)\sfilterlog:\s(.*)$/", $line, $log_split))
-		return "";
+	if (!preg_match('/(.*)\s(.*)\sfilterlog:\s(.*)$/', $line, $log_split)) {
+		return '';
+	}
 
 	list($all, $flent['time'], $host, $rule) = $log_split;
 
-	$rule_data = explode(",", $rule);
+	$rule_data = explode(',', $rule);
 	$field = 0;
 
 	$flent['rulenum'] = $rule_data[$field++];
@@ -146,8 +146,15 @@ function parse_filter_line($line) {
 	$flent['direction'] = $rule_data[$field++];
 	$flent['version'] = $rule_data[$field++];
 
+	if ($flent['version'] != '4' && $flent['version'] != '6') {
+		log_error(sprintf(
+			gettext("There was a error parsing rule number: %s -- not IPv4 or IPv6 (`%s')"),
+			$flent['rulenum'],
+			$rule
+		));
+		return '';
+	}
 
-	if ($flent['version'] == '4' || $flent['version'] == '6') {
 	if ($flent['version'] == '4') {
 		$flent['tos'] = $rule_data[$field++];
 		$flent['ecn'] = $rule_data[$field++];
@@ -192,36 +199,36 @@ function parse_filter_line($line) {
 		$flent['icmp_type'] = $rule_data[$field++];
 
 		switch ($flent['icmp_type']) {
-			case "request":
-			case "reply":
+		case 'request':
+		case 'reply':
 			$flent['icmp_id'] = $rule_data[$field++];
 			$flent['icmp_seq'] = $rule_data[$field++];
 			break;
-			case "unreachproto":
+		case 'unreachproto':
 			$flent['icmp_dstip'] = $rule_data[$field++];
 			$flent['icmp_protoid'] = $rule_data[$field++];
 			break;
-			case "unreachport":
+		case 'unreachport':
 			$flent['icmp_dstip'] = $rule_data[$field++];
 			$flent['icmp_protoid'] = $rule_data[$field++];
 			$flent['icmp_port'] = $rule_data[$field++];
 			break;
-			case "unreach":
-			case "timexceed":
-			case "paramprob":
-			case "redirect":
-			case "maskreply":
+		case 'unreach':
+		case 'timexceed':
+		case 'paramprob':
+		case 'redirect':
+		case 'maskreply':
 			$flent['icmp_descr'] = $rule_data[$field++];
 			break;
-			case "needfrag":
+		case 'needfrag':
 			$flent['icmp_dstip'] = $rule_data[$field++];
 			$flent['icmp_mtu'] = $rule_data[$field++];
 			break;
-			case "tstamp":
+		case 'tstamp':
 			$flent['icmp_id'] = $rule_data[$field++];
 			$flent['icmp_seq'] = $rule_data[$field++];
 			break;
-			case "tstampreply":
+		case 'tstampreply':
 			$flent['icmp_id'] = $rule_data[$field++];
 			$flent['icmp_seq'] = $rule_data[$field++];
 			$flent['icmp_otime'] = $rule_data[$field++];
@@ -232,7 +239,6 @@ function parse_filter_line($line) {
 			$flent['icmp_descr'] = $rule_data[$field++];
 			break;
 		}
-
 	} else if ($flent['protoid'] == '112') { // CARP
 		$flent['type'] = $rule_data[$field++];
 		$flent['ttl'] = $rule_data[$field++];
@@ -241,18 +247,18 @@ function parse_filter_line($line) {
 		$flent['advskew'] = $rule_data[$field++];
 		$flent['advbase'] = $rule_data[$field++];
 	}
-	} else {
-		log_error(sprintf(gettext("There was a error parsing rule number: %s.   Please report to mailing list or forum."), $flent['rulenum']));
-		return "";
+
+	/* If there isn't a src, a dst, or a time, then the line is unusable/bad */
+	if (trim($flent['src']) == '' || trim($flent['dst']) == '' || trim($flent['time']) == '') {
+		log_error(sprintf(
+			gettext("There was a error parsing rule: %s -- no src or test or time (`%s')"),
+			$flent['rulenum'],
+			$rule
+		));
+		return '';
 	}
 
-	/* If there is a src, a dst, and a time, then the line should be usable/good */
-	if (!((trim($flent['src']) == "") || (trim($flent['dst']) == "") || (trim($flent['time']) == ""))) {
 	return $flent;
-	} else {
-		log_error(sprintf(gettext("There was a error parsing rule: %s.   Please report to mailing list or forum."), $errline));
-		return "";
-	}
 }
 
 function get_port_with_service($port, $proto) {