Re: [PATCH] BUG10941: fix single ip-addresses when no subnet given

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2414 bytes --]

On Fri, 2015-10-16 at 19:48 +0200, Alexander Marx wrote:
> 
> 
> Am 16.10.2015 um 17:54 schrieb Michael Tremer:
> > On Fri, 2015-10-16 at 09:57 +0200, Alexander Marx wrote:
> > > Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
> > > ---
> > >  html/cgi-bin/routing.cgi | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/html/cgi-bin/routing.cgi b/html/cgi-bin/routing.cgi
> > > index c460a74..3b5a2f7 100644
> > > --- a/html/cgi-bin/routing.cgi
> > > +++ b/html/cgi-bin/routing.cgi
> > > @@ -118,6 +118,11 @@ if ($settings{'ACTION'} eq $Lang::tr{'toggle
> > > enable disable'}) {
> > >  }
> > >  
> > >  if ($settings{'ACTION'} eq $Lang::tr{'add'}) {
> > > +
> > > +	if (!&Network::check_prefix($settings{'IP'})){
> > > +		$settings{'IP'} .= '/32';
> > > +	}
> > > +
> > This won't work. The function &Network::check_prefix() takes the
> > prefix
> > (i.e. the number after the slash, e.g. 24). You are passing the IP
> > address to the function which will never be a valid prefix and /32
> > will
> > always be appended, even to valid inputs like 192.168.0.0/24
> > (result:
> > 192.168.0.0/24/32).
> I think it's not true that it's not true ;-)
> The function takes a whole ip with subnetmask, true. But if you pass
> only an ip to it or an ip with a wrong subnetmask , the function
> returns false in which case i add /32 to it.

http://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=config/cfgroot/network-functions.pl;h=cb4ca3dd88306f61290fab895308c0725c1ecdac;hb=HEAD#l136

This function clearly does what I said it would do.

I don't know why you don't get an error, but this is not a solution to
this bug. Is it possible that &General::iporsubtocidr() just ignores
everything after the prefix?

> 
> In addition i tested several combinations of ip-addresses in cidr or
> decimal notation and without subnetmask.
> All tests passed with expected behaviour.
> 
> I just checked it twice. I could not find any side effects. But you
> may drop this patch and i will try to find a new solution after my
> holidays.

When will you be back?

> 
> 
> > >  	# Convert subnet masks to CIDR notation.
> > >  	$settings{'IP'} =
> > > &General::iporsubtocidr($settings{'IP'});
> > > 
> > The remaining code should be amended that only prefixes are allowed
> > when /32 is appended to hosts.
> > 
> > -Michael 

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]