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