On 11/23/2015 01:11 PM, Jan Cholasta wrote:
> On 23.11.2015 12:53, Tomas Babej wrote:
>> If the code within the private_ccache contextmanager does not
>> set/removes the KRB5CCNAME, the pop method will raise KeyError, which
>> will cause unnecessary termination of the code flow.
>> Make sure the KRB5CCNAME is popped out of os.environ only if present.
> NACK, use os.environ.pop('KRB5CCNAME', None) to safely remove it.
Yeah, well, both ways are equivalent, I found the if statement more
explicit. We can go with the suggested version, if it's more pleasing
though - patch is attached.
> Also I don't think the comment is necessary, it's quite obvious what the
> code does.
I don't think an explanatory comment can hurt, ever. Worst thing that
happens is that somebody is assured that he understands the code correctly.
From 65d4efa341ff73ef6a3a2da9d443577305aa82dd Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 23 Nov 2015 12:47:56 +0100
Subject: [PATCH] private_ccache: Harden the removal of KRB5CCNAME env variable
If the code within the private_ccache contextmanager does not
set/removes the KRB5CCNAME, the pop method will raise KeyError, which
will cause unnecessary termination of the code flow.
Make sure the KRB5CCNAME is popped out of os.environ only if present.
ipapython/ipautil.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4acdd1a98818bf311a8fef103e7219cc62a28ec1..b4af0868c15178f75007b4f1e407070ae102ceb4 100644
@@ -1367,7 +1367,8 @@ def private_ccache(path=None):
if original_value is not None:
os.environ['KRB5CCNAME'] = original_value
+ # No value was set originally, make sure no value is set now
+ os.environ.pop('KRB5CCNAME', None)
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code