Bug #9074
closed
Alias URL lists only storing last-most list in config.
100%
Description
When creating an Alias URL list under Firewall->Aliases->URLs, only the IP's from the last-most URL in the list is what is stored in the /cf/conf/config.xml file. The other previous URLs in the list, even though correctly validated & checked, are not kept.
It looks as though the issue lies within the /usr/local/www/firewall_aliases_edit.php file. The issue appears to be that during the loop where each url is downloaded & parsed, the $addresses array is replaced & not appended to when each URL is pulled in & validated. (lines 292 to 299)
if (file_exists("{$temp_filename}/aliases")) {
$address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 5000);
if ($address == null) {
/* nothing was found */
$input_errors[] = sprintf(gettext("A valid URL must be provided. Could not fetch usable data from '%s'."), $_POST['address' . $x]);
}
mwexec("/bin/rm -rf " . escapeshellarg($temp_filename));
} else {
The following patch solves the issue for me, where the array is appended during each iteration.
@@ -253,6 +253,7 @@
for ($x = 0; $x < $max_alias_addresses - 1; $x++) {
$_POST['address' . $x] = trim($_POST['address' . $x]);
if ($_POST['address' . $x]) {
+ $t_address = array();
/* fetch down and add in */
$temp_filename = tempnam("{$g['tmp_path']}/", "alias_import");
unlink_if_exists($temp_filename);
@@ -290,10 +291,13 @@
}
if (file_exists("{$temp_filename}/aliases")) {
- $address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 5000);
- if ($address == null) {
+ $t_address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 5000);
+ if ($t_address == null) {
/* nothing was found */
$input_errors[] = sprintf(gettext("A valid URL must be provided. Could not fetch usable data from '%s'."), $_POST['address' . $x]);
+ } else {
+ $address += $t_address;
}
+ unset($t_address);
mwexec("/bin/rm -rf " . escapeshellarg($temp_filename));
} else {
``` mwexec("/bin/rm -rf " . escapeshellarg($temp_filename));
} else {
Updated by Paul Bramhall over 5 years ago
There still appeared to be some odd behaviour with the change I did above where it was not always appending the array, the following fixes it, as instead of just appending (+=) I am now using the array_push functionality:
@@ -253,6 +253,7 @@
for ($x = 0; $x < $max_alias_addresses - 1; $x++) {
$_POST['address' . $x] = trim($_POST['address' . $x]);
if ($_POST['address' . $x]) {
+ $t_address = array();
/* fetch down and add in */
$temp_filename = tempnam("{$g['tmp_path']}/", "alias_import");
unlink_if_exists($temp_filename);
@@ -290,10 +291,13 @@
}
if (file_exists("{$temp_filename}/aliases")) {
- $address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 5000);
- if ($address == null) {
+ $t_address = parse_aliases_file("{$temp_filename}/aliases", $_POST['type'], 5000);
+ if ($t_address == null) {
/* nothing was found */
$input_errors[] = sprintf(gettext("A valid URL must be provided. Could not fetch usable data from '%s'."), $_POST['address' . $x]);
+ } else {
+ array_push($address, ...$t_address);
}
+ unset($t_address);
mwexec("/bin/rm -rf " . escapeshellarg($temp_filename));
} else {
Updated by Paul Bramhall over 5 years ago
Submitted pull request:
https://github.com/pfsense/pfsense/pull/4002
Updated by Danilo Zrenjanin about 5 years ago
Applied https://github.com/pfsense/pfsense/pull/4002/commits/f5c56bf8189d515af203c398f473c9b3adfff98b and https://github.com/pfsense/pfsense/pull/4002/commits/9df78d6b92b98b88055c2feffc29111ee15ea2b0 via system_patches and not only the IPs from the last-most URL appeared in config.xml
Before the patch:
<alias>
<name>Bug_9074_test</name>
<type>url</type>
<aliasurl>https://127.0.0.1/danilo_test.txt</aliasurl>
<aliasurl>https://127.0.0.1/danilo_test1.txt</aliasurl>
<address>1.1.1.1 8.8.8.8 8.8.4.4</address>
<descr><![CDATA[test]]></descr>
<detail><![CDATA[Entry added Sat, 02 Mar 2019 10:53:28 +0100||Entry added Sat, 02 Mar 2019 11:57:33 +0100]]></detail>
</alias>
After the patch:
<alias>
<name>Bug_9074_test</name>
<type>url</type>
<aliasurl>https://127.0.0.1/danilo_test.txt</aliasurl>
<aliasurl>https://127.0.0.1/danilo_test1.txt</aliasurl>
<address>195.252.110.81 18.211.55.24 18.210.212.54 18.191.54.162 18.219.105.141 8.8.8.8 1.1.1.1 8.8.4.4</address>
<descr></descr>
<detail><![CDATA[Entry added Sat, 02 Mar 2019 12:53:50 +0000||Entry added Sat, 02 Mar 2019 12:53:50 +0000]]></detail>
</alias>
Looks good now.
Updated by Jim Pingle about 5 years ago
- Target version set to 2.5.0
- Affected Architecture All added
- Affected Architecture deleted (
)
Updated by Renato Botelho almost 5 years ago
- Assignee set to Renato Botelho
PR has been merged. Thanks!
Updated by Renato Botelho almost 5 years ago
- Status changed from New to Feedback
- % Done changed from 0 to 100
Updated by Chris Linstruth over 4 years ago
Tested. Table populated with last URL contents under 2.4.4-p3 and both URL contents using latest snapshot. Looks good.
Updated by Jim Pingle over 4 years ago
- Category changed from Rules / NAT to Aliases / Tables