On 11/23/2015 01:11 PM, Jan Cholasta wrote:
> On 23.11.2015 12:53, Tomas Babej wrote:
>> Hi,
>>
>> 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.
>>
>> Tomas
> 
> 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.

Tomas
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
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -1367,7 +1367,8 @@ def private_ccache(path=None):
         if original_value is not None:
             os.environ['KRB5CCNAME'] = original_value
         else:
-            os.environ.pop('KRB5CCNAME')
+            # No value was set originally, make sure no value is set now
+            os.environ.pop('KRB5CCNAME', None)
 
         if os.path.exists(path):
             os.remove(path)
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to