The removed assignments are wrong because both are trying to assign something different to the same key and will overwrite each other.
Secondary the assignment to the hash is not needed at this place, so it safely can be removed.
Signed-off-by: Stefan Schantl stefan.schantl@ipfire.org --- html/cgi-bin/firewall.cgi | 2 -- 1 file changed, 2 deletions(-)
diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi index 70dee8d3c..d3cddfa23 100644 --- a/html/cgi-bin/firewall.cgi +++ b/html/cgi-bin/firewall.cgi @@ -1022,8 +1022,6 @@ sub gen_dd_block $checked{'TIME_SUN'}{$fwdfwsettings{'TIME_SUN'}} = 'CHECKED'; $selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected'; $selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected'; - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected'; - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected'; print<<END; <table width='100%' border='0'> <tr><td width='50%' valign='top'>
When configuring a standard network as source or target the same interface would be pre-selected as firewall interface when editing an existing rule.
In case an existing input rule with an configured firewall interface should be changed, the same network device has been pre-selected in the standard networks dropdown box.
This easily confuses users and may lead to false configurations when saving an edited rule.
Signed-off-by: Stefan Schantl stefan.schantl@ipfire.org --- html/cgi-bin/firewall.cgi | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi index d3cddfa23..8d6adee96 100644 --- a/html/cgi-bin/firewall.cgi +++ b/html/cgi-bin/firewall.cgi @@ -1032,7 +1032,12 @@ END { next if($defaultNetworks{$network}{'NAME'} eq "IPFire"); print "<option value='$defaultNetworks{$network}{'NAME'}'"; - print " selected='selected'" if ($fwdfwsettings{$fwdfwsettings{$grp}} eq $defaultNetworks{$network}{'NAME'}); + + # Check if the the key handles a standard network. + if ( grep(/std_net_/, $fwdfwsettings{$grp}) ) { + print " selected='selected'" if ($fwdfwsettings{$fwdfwsettings{$grp}} eq $defaultNetworks{$network}{'NAME'}); + } + my $defnet="$defaultNetworks{$network}{'NAME'}_NETADDRESS"; my $defsub="$defaultNetworks{$network}{'NAME'}_NETMASK"; my $defsub1=&General::subtocidr($ifaces{$defsub}); @@ -1510,8 +1515,8 @@ sub newrule $checked{'USE_NAT'}{$fwdfwsettings{'USE_NAT'}} = 'CHECKED'; $selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected'; $selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected'; - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected'; - $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected'; + $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} = 'selected' if ($fwdfwsettings{'grp2'} eq "ipfire"); + $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} = 'selected' if ($fwdfwsettings{'grp1'} eq "ipfire_src"); #check if update and get values if($fwdfwsettings{'updatefwrule'} eq 'on' || $fwdfwsettings{'copyfwrule'} eq 'on' && !$errormessage){ &General::readhasharray("$config", %hash); @@ -1582,8 +1587,8 @@ sub newrule $checked{'RATE_LIMIT'}{$fwdfwsettings{'RATE_LIMIT'}} = 'CHECKED'; $selected{'TIME_FROM'}{$fwdfwsettings{'TIME_FROM'}} = 'selected'; $selected{'TIME_TO'}{$fwdfwsettings{'TIME_TO'}} = 'selected'; - $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} ='selected'; - $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} ='selected'; + $selected{'ipfire'}{$fwdfwsettings{$fwdfwsettings{'grp2'}}} = 'selected' if ($fwdfwsettings{'grp2'} eq "ipfire"); + $selected{'ipfire_src'}{$fwdfwsettings{$fwdfwsettings{'grp1'}}} = 'selected' if ($fwdfwsettings{'grp1'} eq "ipfire_src"); $selected{'dnat'}{$fwdfwsettings{'dnat'}} ='selected'; $selected{'snat'}{$fwdfwsettings{'snat'}} ='selected'; $selected{'RATETIME'}{$fwdfwsettings{'RATETIME'}} ='selected';
Some functions uses those two hashes and are altering them - making them private will erase and fill it with new data.
Signed-off-by: Stefan Schantl stefan.schantl@ipfire.org --- html/cgi-bin/firewall.cgi | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi index 8d6adee96..0eace5f11 100644 --- a/html/cgi-bin/firewall.cgi +++ b/html/cgi-bin/firewall.cgi @@ -1005,6 +1005,10 @@ sub gen_dd_block my $grp=shift; my $helper=''; my $show=''; + + my %checked = (); + my %selected = (); + $checked{'grp1'}{$fwdfwsettings{'grp1'}} = 'CHECKED'; $checked{'grp2'}{$fwdfwsettings{'grp2'}} = 'CHECKED'; $checked{'grp3'}{$fwdfwsettings{'grp3'}} = 'CHECKED'; @@ -1482,7 +1486,10 @@ sub newrule &General::readhasharray("$configlocationgrp", %customlocationgrp); &General::readhasharray("$configipsec", %ipsecconf); &General::get_aliases(%aliases); - my %checked=(); + + my %checked = (); + my %selected = (); + my $helper; my $sum=0; if($fwdfwsettings{'config'} eq ''){$fwdfwsettings{'config'}=$configfwdfw;}