[PATCH] Provides validation for the image file argument.

[Darryl L. Pierce]

Checks for the standard fields for a URL, and enforces some simple
requirements.

An exception is raised if the filename:

 * has a hostname but not a protocol and port,
 * has a protocol but not hostname

Signed-off-by: Darryl L. Pierce <dpierce(a)redhat.com&gt;
---
 cobbler/item_image.py |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/cobbler/item_image.py b/cobbler/item_image.py
index 98e0255..2ce6738 100644
--- a/cobbler/item_image.py
+++ b/cobbler/item_image.py
@@ -113,11 +113,26 @@ class Image(item.Item):
     def set_file(self,filename):
         """
         Stores the image location.  This should be accessible on all nodes
-        that need to access it.  Format: either /mnt/commonpath/foo.iso or 
+        that need to access it.  Format: either /mnt/commonpath/foo.iso or
         nfs://host/path/foo.iso
         """
         # FIXME: this should accept NFS paths or filesystem paths
-        self.file = filename
+        uri = filename
+
+        protocol = auth = hostname = port = path = file = ""
+        if filename.find("://") != -1: protocol, filename =
filename.split("://")
+        if filename.find("@") != -1: auth, filename =
filename.split("@")
+        if filename.find("/")   > 0: hostname, filename =
filename.split("/", 1)
+        if hostname.find(":") != -1: hostname, port =
hostname.split(":")
+        if filename.find("/") != -1: path, filename =
filename.rsplit("/", 1)
+
+        if len(filename) == 0:  raise CX(_("missing filename"))
+        if len(hostname) > 0 and len(protocol) == 0 and len(port) == 0:
+            raise CX(_("if protocol is not supplied then a port must be
provided"))
+        if len(hostname) == 0 and len(protocol) > 0:
+            raise CX(_("no hostname supplied but is required for protocol"))
+
+        self.file = uri
         return True
 
     def set_os_version(self,os_version):
-- 
1.6.0.6

[Michael DeHaan]

Darryl L. Pierce wrote:
>  Checks for the standard fields for a URL, and enforces some simple
>  requirements.
>
>  An exception is raised if the filename:
>
>   * has a hostname but not a protocol and port,
>   * has a protocol but not hostname
>
>  Signed-off-by: Darryl L. Pierce <dpierce(a)redhat.com&gt;
>  ---
>   cobbler/item_image.py |   19 +++++++++++++++++--
>   1 files changed, 17 insertions(+), 2 deletions(-)
>
>  diff --git a/cobbler/item_image.py b/cobbler/item_image.py
>  index 98e0255..2ce6738 100644
>  --- a/cobbler/item_image.py
>  +++ b/cobbler/item_image.py
>  @@ -113,11 +113,26 @@ class Image(item.Item):
>       def set_file(self,filename):
>           """
>           Stores the image location.  This should be accessible on all nodes
>  -        that need to access it.  Format: either /mnt/commonpath/foo.iso or 
>  +        that need to access it.  Format: either /mnt/commonpath/foo.iso or
>           nfs://host/path/foo.iso
>           """
>           # FIXME: this should accept NFS paths or filesystem paths
>  -        self.file = filename
>  +        uri = filename
>  +
>  +        protocol = auth = hostname = port = path = file = ""
>  +        if filename.find("://") != -1: protocol, filename =
> filename.split("://")
>  +        if filename.find("@") != -1: auth, filename =
> filename.split("@")
>  +        if filename.find("/")   > 0: hostname, filename =
> filename.split("/", 1)
>  +        if hostname.find(":") != -1: hostname, port =
> hostname.split(":")
>  +        if filename.find("/") != -1: path, filename =
> filename.rsplit("/", 1)
>  +
>  +        if len(filename) == 0:  raise CX(_("missing filename"))
>  +        if len(hostname) > 0 and len(protocol) == 0 and len(port) == 0:
>  +            raise CX(_("if protocol is not supplied then a port must be
> provided"))
>  +        if len(hostname) == 0 and len(protocol) > 0:
>  +            raise CX(_("no hostname supplied but is required for protocol"))
>  +
>  +        self.file = uri
>           return True
>   
>       def set_os_version(self,os_version):
>     
I think we can be more explicit WRT the "if protocol is not supplied" 
based on only supporting what koan can support today:

The valid forms are:

nfs://hostspec:/path_not_ending_in_slash
hostspec can be either "user@host" or "host".
the literal "nfs://" is required for this format.

/absolute_path_not_ending_in_slash

I'm not sure what the port details were? Where would a port be supplied?

Thanks,

--Michael

[Darryl L. Pierce]

On Mon, Dec 22, 2008 at 02:37:43PM -0500, Michael DeHaan wrote:
>  I think we can be more explicit WRT the "if protocol is not
> supplied" 
>  based on only supporting what koan can support today:
>  
>  The valid forms are:
>  
>  nfs://hostspec:/path_not_ending_in_slash
>  hostspec can be either "user@host" or "host". 
It can also be user:password@host to include authentication
details[1], though probably not a good idea to put it into 
the clear like that.

>  the literal "nfs://" is required for this format. 
I'll add a check for that as well. If auth details are provided then
a hostname and a protocol must be provided.

>  /absolute_path_not_ending_in_slash 
Well, for an image record, there needs to be a filename provide,
right? Therefore, a missing filename would be an exception. Or do
you mean something else?
 
>  I'm not sure what the port details were? Where would a port be
> supplied? 
If the hostname is followed by a : then a port is *supposed* to be
provided or else the : should be excluded/ignored[2]. 

[1] http://labs.apache.org/webarch/uri/rfc/rfc3986.html#userinfo
[2] http://labs.apache.org/webarch/uri/rfc/rfc3986.html#port

-- 
Darryl L. Pierce, Sr. Software Engineer @ Red Hat, Inc.
Virtual Machine Management - http://www.ovirt.org/
"What do you care what other people think, Mr. Feynman?"

[Michael DeHaan]

Darryl L. Pierce wrote:
>  On Mon, Dec 22, 2008 at 02:37:43PM -0500, Michael DeHaan wrote:
>    
> > I think we can be more explicit WRT the "if protocol is not supplied" 
> > based on only supporting what koan can support today:
> >
> > The valid forms are:
> >
> > nfs://hostspec:/path_not_ending_in_slash
> > hostspec can be either "user@host" or "host".
> >     
>
>  It can also be user:password@host to include authentication
>  details[1], though probably not a good idea to put it into 
>  the clear like that.
>
>    
> > the literal "nfs://" is required for this format.
> >     
>
>  I'll add a check for that as well. If auth details are provided then
>  a hostname and a protocol must be provided.
>
>     
Specifically what I'm getting at is there's only one protocol, so we can 
mention that in the error message.

> > /absolute_path_not_ending_in_slash
> >     
>
>  Well, for an image record, there needs to be a filename provide,
>  right? Therefore, a missing filename would be an exception. Or do
>  you mean something else?
>   
>     
That's what I mean, if it ends in slash, we know for sure it's a 
directory but cannot do more validation without mounting it, which we 
don't want to do.

> > I'm not sure what the port details were? Where would a port
> be supplied?
> >     
>
>  If the hostname is followed by a : then a port is *supposed* to be
>  provided or else the : should be excluded/ignored[2]. 
>     
Hostname followed by a slash could be followed by a mount point, so I 
guess what you're saying is there could be yet another colon.
>  [1] http://labs.apache.org/webarch/uri/rfc/rfc3986.html#userinfo
>  [2] http://labs.apache.org/webarch/uri/rfc/rfc3986.html#port
>
>    
>  ------------------------------------------------------------------------
>
>  _______________________________________________
>  cobbler mailing list
>  cobbler(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/cobbler
>