mbox

BUG10806: fix wrong customhostgroupcheck

Message ID 1444979636-4110-1-git-send-email-alexander.marx@ipfire.org
State Accepted
Commit 8b7417c50b8d3de46003bd40d779bef222dc4171
Headers

Message

Alexander Marx Oct. 16, 2015, 6:13 p.m. UTC 
  Signed-off-by: Alexander Marx <alexander.marx@ipfire.org>
---
 html/cgi-bin/firewall.cgi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Tremer Oct. 17, 2015, 2:51 a.m. UTC | #1 
What does this patch do?

http://wiki.ipfire.org/devel/git/commit-messages

On Fri, 2015-10-16 at 09:13 +0200, Alexander Marx wrote:
>  Signed-off-by: Alexander Marx <alexander.marx@ipfire.org>
> ---
>  html/cgi-bin/firewall.cgi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
> index c207ec7..682c285 100644
> --- a/html/cgi-bin/firewall.cgi
> +++ b/html/cgi-bin/firewall.cgi
> @@ -597,7 +597,7 @@ sub checktarget
>  		&General::readhasharray("$confighost",
> \%customhost);
>  		foreach my $grpkey (sort keys %customgrp){
>  			foreach my $hostkey (sort keys %customhost){
> -				if ($customgrp{$grpkey}[2] eq
> $customhost{$hostkey}[0] && $customhost{$hostkey}[1] eq 'mac'){
> +				if ($customgrp{$grpkey}[2] eq
> $customhost{$hostkey}[0] && $customgrp{$grpkey}[2] eq
> $fwdfwsettings{$fwdfwsettings{'grp2'}} && $customhost{$hostkey}[1] eq
> 'mac'){
>  					$hint=$Lang::tr{'fwdfw hint
> mac'};
>  					return $hint;
>  				}
Alexander Marx Oct. 17, 2015, 4:54 a.m. UTC | #2 
Am 16.10.2015 um 17:51 schrieb Michael Tremer:
> What does this patch do?
I think it should fix a design error. As you can see, the function is 
only executed, if a hostgroup is used as target.
within the function, we read the hasharray of all defined hostgroups. 
unfortunately it checks not only the TARGET hostgroup but the source 
hostgroup as well if it contains a mac address. This leads to an error, 
if a hostgroup is used as source and contains a mac address.
so with this patch i adapted the if construct only to raise the error, 
if a mac address is present in the TARGET hostgroup.

>
> http://wiki.ipfire.org/devel/git/commit-messages
>
> On Fri, 2015-10-16 at 09:13 +0200, Alexander Marx wrote:
>>   Signed-off-by: Alexander Marx <alexander.marx@ipfire.org>
>> ---
>>   html/cgi-bin/firewall.cgi | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/html/cgi-bin/firewall.cgi b/html/cgi-bin/firewall.cgi
>> index c207ec7..682c285 100644
>> --- a/html/cgi-bin/firewall.cgi
>> +++ b/html/cgi-bin/firewall.cgi
>> @@ -597,7 +597,7 @@ sub checktarget
>>   		&General::readhasharray("$confighost",
>> \%customhost);
>>   		foreach my $grpkey (sort keys %customgrp){
>>   			foreach my $hostkey (sort keys %customhost){
>> -				if ($customgrp{$grpkey}[2] eq
>> $customhost{$hostkey}[0] && $customhost{$hostkey}[1] eq 'mac'){
>> +				if ($customgrp{$grpkey}[2] eq
>> $customhost{$hostkey}[0] && $customgrp{$grpkey}[2] eq
>> $fwdfwsettings{$fwdfwsettings{'grp2'}} && $customhost{$hostkey}[1] eq
>> 'mac'){
>>   					$hint=$Lang::tr{'fwdfw hint
>> mac'};
>>   					return $hint;
>>   				}
Michael Tremer Oct. 17, 2015, 8:14 a.m. UTC | #3 
Okay, could you please include descriptions like these in the commit
messages? Please resend this patch, too.

-Michael

On Fri, 2015-10-16 at 19:54 +0200, Alexander Marx wrote:
> 
> 
> Am 16.10.2015 um 17:51 schrieb Michael Tremer:
> > What does this patch do?
> I think it should fix a design error. As you can see, the function is
> only executed, if a hostgroup is used as target.
> within the function, we read the hasharray of all defined hostgroups.
> unfortunately it checks not only the TARGET hostgroup but the source
> hostgroup as well if it contains a mac address. This leads to an
> error, if a hostgroup is used as source and contains a mac address.
> so with this patch i adapted the if construct only to raise the
> error, if a mac address is present in the TARGET hostgroup.
> 
> > http://wiki.ipfire.org/devel/git/commit-messages
> > 
> > On Fri, 2015-10-16 at 09:13 +0200, Alexander Marx wrote:
> > >  Signed-off-by: Alexander Marx <alexander.marx@ipfire.org>
> > > ---
> > >  html/cgi-bin/firewall.cgi | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/html/cgi-bin/firewall.cgi b/html/cgi
> > > -bin/firewall.cgi
> > > index c207ec7..682c285 100644
> > > --- a/html/cgi-bin/firewall.cgi
> > > +++ b/html/cgi-bin/firewall.cgi
> > > @@ -597,7 +597,7 @@ sub checktarget
> > >  		&General::readhasharray("$confighost",
> > > \%customhost);
> > >  		foreach my $grpkey (sort keys %customgrp){
> > >  			foreach my $hostkey (sort keys
> > > %customhost){
> > > -				if ($customgrp{$grpkey}[2] eq
> > > $customhost{$hostkey}[0] && $customhost{$hostkey}[1] eq 'mac'){
> > > +				if ($customgrp{$grpkey}[2] eq
> > > $customhost{$hostkey}[0] && $customgrp{$grpkey}[2] eq
> > > $fwdfwsettings{$fwdfwsettings{'grp2'}} &&
> > > $customhost{$hostkey}[1] eq
> > > 'mac'){
> > >  					$hint=$Lang::tr{'fwdfw
> > > hint
> > > mac'};
> > >  					return $hint;
> > >  				}