Improve URL and URL ports alias update data:

- Move redundant code to a function parse_aliases_file(). Before the max
number of items was not being respected when URL content is updated,
only when alias was saved. Same was happening with ip/subnet/port
validation and user could end up with a bad pf.conf
- Remove unused variables

These changes were based on Pull Request #1264. It should fix #4189

Submitted by:▸  PiBa-NL

2 files changed, 45 insertions, 53 deletions

diff --git a/etc/inc/pfsense-utils.inc b/etc/inc/pfsense-utils.inc
index 07db39a..1bfa1a4 100644
--- a/etc/inc/pfsense-utils.inc
+++ b/etc/inc/pfsense-utils.inc
@@ -1889,6 +1889,45 @@ function update_alias_names_upon_change($section, $field, $new_alias_name, $orig
 
 }
 
+function parse_aliases_file($filename, $type = "url", $max_items = -1) {
+	/*
+	 * $filename = file to process for example blocklist like DROP:  http://www.spamhaus.org/drop/drop.txt
+	 * $type = if set to 'url' then subnets and ips will be returned,
+	 *         if set to 'url_ports' port-ranges and ports will be returned
+	 * $max_items = sets the maximum amount of valid items to load, -1 the default defines there is no limit.
+	 *
+	 * RETURNS an array of ip subnets and ip's or ports and port-ranges, returns NULL upon a error conditions (file not found)
+	 */
+
+	$fd = @fopen($filename, 'r');
+	if (!$fd) {
+		log_error(gettext("Could not process aliases from alias: {$alias_url}"));
+		return null;
+	}
+	$items = array();
+	/* NOTE: fgetss() is not a typo RTFM before being smart */
+	while (($fc = fgetss($fd)) !== FALSE) {
+		$tmp = trim($fc, " \t\n\r");
+		if (empty($tmp))
+			continue;
+		$tmp_str = strstr($tmp, '#', true);
+		if (!empty($tmp_str))
+			$tmp = $tmp_str;
+		$tmp_str = strstr($tmp, ' ', true);
+		if (!empty($tmp_str))
+			$tmp = $tmp_str;
+		$valid = ($type == "url" && (is_ipaddr($tmp) || is_subnet($tmp))) ||
+		         ($type == "url_ports" && (is_port($tmp) || is_portrange($tmp)));
+		if ($valid) {
+			$items[] = $tmp;
+			if (count($items) == $max_items)
+				break;
+		}
+	}
+	fclose($fd);
+	return $items;
+}
+
 function update_alias_url_data() {
 	global $config, $g;
 
@@ -1901,8 +1940,7 @@ function update_alias_url_data() {
 			if (empty($alias['aliasurl']))
 				continue;
 
-			$address = "";
-			$isfirst = 0;
+			$address = null;
 			foreach ($alias['aliasurl'] as $alias_url) {
 				/* fetch down and add in */
 				$temp_filename = tempnam("{$g['tmp_path']}/", "alias_import");
@@ -1920,30 +1958,12 @@ function update_alias_url_data() {
 						continue;
 				}
 				if (file_exists("{$temp_filename}/aliases")) {
-					$fd = @fopen("{$temp_filename}/aliases", 'r');
-					if (!$fd) {
-						log_error(gettext("Could not process aliases from alias: {$alias_url}"));
-						continue;
-					}
-					/* NOTE: fgetss() is not a typo RTFM before being smart */
-					while (($fc = fgetss($fd)) !== FALSE) {
-						$tmp = trim($fc, " \t\n\r");
-						if (empty($tmp))
-							continue;
-						$tmp_str = strstr($tmp, '#', true);
-						if (!empty($tmp_str))
-							$tmp = $tmp_str;
-						if ($isfirst == 1)
-							$address .= ' ';
-						$address .= $tmp;
-						$isfirst = 1;
-					}
-					fclose($fd);
+					$address = parse_aliases_file("{$temp_filename}/aliases", $alias['type'], 3000);
 					mwexec("/bin/rm -rf {$temp_filename}");
 				}
 			}
-			if (!empty($address)) {
-				$config['aliases']['alias'][$x]['address'] = $address;
+			if ($address != null) {
+				$config['aliases']['alias'][$x]['address'] = implode(" ", $address);
 				$updated = true;
 			}
 		}
diff --git a/usr/local/www/firewall_aliases_edit.php b/usr/local/www/firewall_aliases_edit.php
index 80ef2d4..c55658c 100755
--- a/usr/local/www/firewall_aliases_edit.php
+++ b/usr/local/www/firewall_aliases_edit.php
@@ -171,13 +171,11 @@ if ($_POST) {
 
 	if (preg_match("/urltable/i", $_POST['type'])) {
 		$address = "";
-		$isfirst = 0;
 
 		/* item is a url table type */
 		if ($_POST['address0']) {
 			/* fetch down and add in */
 			$_POST['address0'] = trim($_POST['address0']);
-			$isfirst = 0;
 			$address[] = $_POST['address0'];
 			$alias['url'] = $_POST['address0'];
 			$alias['updatefreq'] = $_POST['address_subnet0'] ? $_POST['address_subnet0'] : 7;
@@ -199,8 +197,6 @@ if ($_POST) {
 				$final_address_details[] = sprintf(gettext("Entry added %s"), date('r'));
 		}
 	} else if ($_POST['type'] == "url" || $_POST['type'] == "url_ports") {
-		$isfirst = 0;
-		$address_count = 2;
 		$desc_fmt_err_found = false;
 
 		/* item is a url type */
@@ -208,7 +204,6 @@ if ($_POST) {
 			$_POST['address' . $x] = trim($_POST['address' . $x]);
 			if($_POST['address' . $x]) {
 				/* fetch down and add in */
-				$isfirst = 0;
 				$temp_filename = tempnam("{$g['tmp_path']}/", "alias_import");
 				unlink_if_exists($temp_filename);
 				$verify_ssl = isset($config['system']['checkaliasesurlcert']);
@@ -241,31 +236,8 @@ if ($_POST) {
 					$final_address_details[] = sprintf(gettext("Entry added %s"), date('r'));
 
 				if(file_exists("{$temp_filename}/aliases")) {
-					$file_contents = file_get_contents("{$temp_filename}/aliases");
-					$file_contents = str_replace("#", "\n#", $file_contents);
-					$file_contents_split = explode("\n", $file_contents);
-					foreach($file_contents_split as $fc) {
-						// Stop at 3000 items, aliases larger than that tend to break both pf and the WebGUI.
-						if ($address_count >= 3000)
-							break;
-						$tmp = trim($fc);
-						if(stristr($fc, "#")) {
-							$tmp_split = explode("#", $tmp);
-							$tmp = trim($tmp_split[0]);
-						}
-						$tmp = trim($tmp);
-						if ($_POST['type'] == "url")
-							$is_valid = (is_ipaddr($tmp) || is_subnet($tmp));
-						else
-							$is_valid = (is_port($tmp) || is_portrange($tmp));
-
-						if (!empty($tmp) && $is_valid) {
-							$address[] = $tmp;
-							$isfirst = 1;
-							$address_count++;
-						}
-					}
-					if($isfirst == 0) {
+					$address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 3000);
+					if($address == null) {
 						/* nothing was found */
 						$input_errors[] = sprintf(gettext("You must provide a valid URL. Could not fetch usable data from '%s'."), $_POST['address' . $x]);
 					}