Ticket #2850 - Ipactl exception not handled well

Ticket #2850 - Ipactl exception not handled well

There were various places in ipactl which intialized IpactlError with
None as the msg. If you called str() on that exception all was well
because ScriptError.__str__() converted a msg with None to the empty
string (IpactlError is subclassed from ScriptError). But a few places
directly access e.msg which will be None if initialized that way. It's
hard to tell from the stack traces but I'm pretty sure it's those
places which use e.msg directly which will cause the problems seen in
the bug report.

I do not believe it is ever correct to initialize an exception message
to None, I don't even understand what that means. On the other hand
initializing to the empty string is sensible and for that matter is
the default for the class.

This patch makes two fixes:

1) The ScriptError initializer will now convert a msg parameter of
None to the empty string.

2) All places that initialized IpactlError's msg parameter to None
removed the None initializer allowing the msg parameter to default
to the empty string.

I don't know how to test the fix for Ticket #2850 because it's not
clear how it got into that state in the first place, but I do believe
initialing the msg value to None is clearly wrong and should fix the
problem.

2 files changed, 8 insertions, 6 deletions

diff --git a/install/tools/ipactl b/install/tools/ipactl
index e173d10c1..d4b2c0878 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -186,9 +186,9 @@ def ipa_start(options):
             pass
         if isinstance(e, IpactlError):
             # do not display any other error message
-            raise IpactlError(None, e.rval)
+            raise IpactlError(rval=e.rval)
         else:
-            raise IpactlError(None)
+            raise IpactlError()
 
     if len(svc_list) == 0:
         # no service to stop
@@ -235,7 +235,7 @@ def ipa_stop(options):
                 # just try to stop it, do not read a result
                 dirsrv.stop()
             finally:
-                raise IpactlError(None)
+                raise IpactlError()
 
     if len(svc_list) == 0:
         # no service to stop
@@ -277,9 +277,9 @@ def ipa_restart(options):
             pass
         if isinstance(e, IpactlError):
             # do not display any other error message
-            raise IpactlError(None, e.rval)
+            raise IpactlError(rval=e.rval)
         else:
-            raise IpactlError(None)
+            raise IpactlError()
 
     if len(svc_list) == 0:
         # no service to stop
diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 1ba8b6bbb..b644516dd 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -36,11 +36,13 @@ class ScriptError(StandardError):
     """An exception that records an error message and a return value
     """
     def __init__(self, msg='', rval=1):
+        if msg is None:
+            msg = ''
         self.msg = msg
         self.rval = rval
 
     def __str__(self):
-        return self.msg or ''
+        return self.msg
 
 
 class AdminTool(object):