summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohn Dennis <jdennis@redhat.com>2012-08-20 16:47:52 -0400
committerMartin Kosek <mkosek@redhat.com>2012-08-27 15:30:28 +0200
commit2bf68115cea1bae09c3b45e88ed9e405c57e70d2 (patch)
tree6b719293d0be563b4c71490285db4e8946470ed4
parent3eadcdf123c6c58bdbf25f3e8bdf12c2ddf9127f (diff)
downloadfreeipa-2bf68115cea1bae09c3b45e88ed9e405c57e70d2.tar.gz
freeipa-2bf68115cea1bae09c3b45e88ed9e405c57e70d2.tar.xz
freeipa-2bf68115cea1bae09c3b45e88ed9e405c57e70d2.zip
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.
-rwxr-xr-xinstall/tools/ipactl10
-rw-r--r--ipapython/admintool.py4
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):