[et-mgmt-tools] [PATCH] Strengthen port number validation

[Masayuki Sunou]

Hi

Thanks for reviewing.

> I think the nice way to check the port would be to have a function that
> actually attempts to bind the port, to test that it is empty. You would
> understandably have to release it if you succeeded so the install can use
> it in the future. I'm not sure if this would carry any residual effects,
> maybe someone else has a better idea?
> 
This idea looks better.
I remake the patch.

This patch checks the port by using socket. 

Thanks,
Masayuki Sunou

-------------------------------------------------------------------------------
diff -r f40212ea1fd6 virtinst/cli.py
--- a/virtinst/cli.py	Wed Nov 07 16:31:59 2007 -0500
+++ b/virtinst/cli.py	Fri Nov 09 11:55:28 2007 +0900
@@ -15,6 +15,7 @@ import os, sys
 import os, sys
 import logging
 import logging.handlers
+import socket
 
 import libvirt
 import util
@@ -207,6 +208,8 @@ def get_graphics(vnc, vncport, nographic
         guest.graphics = False
         return
     if vnc is not None:
+        if vncport is not None and vncport >= 5900:
+            vncport = port_search(vncport)
         guest.graphics = (True, "vnc", vncport, keymap)
         return
     if sdl is not None:
@@ -216,6 +219,8 @@ def get_graphics(vnc, vncport, nographic
         res = prompt_for_input(_("Would you like to enable graphics support? (yes or no)"))
         try:
             vnc = yes_or_no(res)
+            if vncport is not None and vncport >= 5900:
+                vncport = port_search(vncport)
         except ValueError, e:
             print _("ERROR: "), e
             continue
@@ -224,6 +229,26 @@ def get_graphics(vnc, vncport, nographic
         else:
             guest.graphics = False
         break
+
+def port_search(vncport):
+    while 1:
+        try:
+            s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+            s.bind(('', vncport))
+        except socket.error, e:
+            s.close()
+            s = None
+        if s is not None:
+            s.close()
+            return vncport
+        port = prompt_for_input(_("Installation cannot be continued, because the specified port number is already used.\n Please set another port number or just push enter to allocate a port number automatically."))
+        try:
+            if port is "":
+                return None
+            else:
+                vncport = int(port)
+        except ValueError, e:
+            print _("ERROR: "), e
 
 ### Option parsing
 def check_before_store(option, opt_str, value, parser):
-------------------------------------------------------------------------------

In message <473231BD.7010101 at redhat.com>
   "Re: [et-mgmt-tools] [PATCH] Strengthen port number validation"
   "Cole Robinson <crobinso at redhat.com>" wrote:

> Masayuki Sunou wrote:
> > Hi
> > 
> > Installation fails when port number used by other processes is set 
> > to --vncport of virt-install, because graphical console is not displayed.
> > The same problem occurs when port number exceeds upper bound. 
> > 
> > One of patches fixes to request re-input when port number used is set.
> >  --> check_vncport_used.patch
> > Other fixes to output error message when port number exceeds upper bound. 
> >  --> check_vncport_upperbound.patch
> > 
> > Signed-off-by: Masayuki Sunou <fj1826dm at aa.jp.fujitsu.com>
> > 
> > Thanks,
> > Masayuki Sunou.
> 
> 
> Hi,
> 
> The upperbound check looks good, I just applied it.
> 
> The vncport collision detection though I'm a bit worried about. Parsing
> 'netstat' doesn't seem like a nice solution: its a lot of output to parse
> for little gain and requires an external utility to do it.
> 
> I think the nice way to check the port would be to have a function that
> actually attempts to bind the port, to test that it is empty. You would
> understandably have to release it if you succeeded so the install can use
> it in the future. I'm not sure if this would carry any residual effects,
> maybe someone else has a better idea?
> 
> - Cole
> 
> -- 
> Cole Robinson
> crobinso at redhat.com
> 
> _______________________________________________
> et-mgmt-tools mailing list
> et-mgmt-tools at redhat.com
> https://www.redhat.com/mailman/listinfo/et-mgmt-tools