URL: https://github.com/freeipa/freeipa/pull/818
Author: stlaz
 Title: #818: Avoid possible recursion in RPC call from client
Action: opened

PR body:
"""
This commit removes recursion which may lack end condition.

https://pagure.io/freeipa/issue/6796

===============================
This is my try to fix ^--. The methods and their arguments in the module are 
very poorly documented (they are not documented), so I just hope I can 
initialize the variables used in logs prior to the cycle.

I also think this actually relates more to https://pagure.io/freeipa/issue/6775 
than the ticket mentioned here and that the person "fixing" 
https://pagure.io/freeipa/issue/6796 messed up the tickets but it's hopefully 
OK to fix it this way.

Aside from just moving everything into a cycle, I also improved logging a bit 
and had same error handling for different errors merged into one `except` block 
(`SSLError`, `socket.error`).
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/818/head:pr818
git checkout pr818
From 4387b3195ae2d8b534dce85885357156d6662fe1 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Fri, 26 May 2017 08:37:36 +0200
Subject: [PATCH] Avoid possible recursion in RPC call from client

This commit removes recursion which may lack end condition.

https://pagure.io/freeipa/issue/6796
---
 ipalib/rpc.py | 93 +++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 52 insertions(+), 41 deletions(-)

diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index e23ca3d061..18cd3babc6 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -1088,50 +1088,61 @@ def forward(self, name, *args, **kw):
         :param kw: Keyword arguments to pass to remote command.
         """
         server = getattr(context, 'request_url', None)
-        self.log.info("Forwarding '%s' to %s server '%s'",
-                      name, self.protocol, server)
         command = getattr(self.conn, name)
         params = [args, kw]
-        try:
-            return self._call_command(command, params)
-        except Fault as e:
-            e = decode_fault(e)
-            self.debug('Caught fault %d from server %s: %s', e.faultCode,
-                server, e.faultString)
-            if e.faultCode in errors_by_code:
-                error = errors_by_code[e.faultCode]
-                raise error(message=e.faultString)
-            raise UnknownError(
-                code=e.faultCode,
-                error=e.faultString,
-                server=server,
-            )
-        except SSLError as e:
-            raise NetworkError(uri=server, error=str(e))
-        except ProtocolError as e:
-            # By catching a 401 here we can detect the case where we have
-            # a single IPA server and the session is invalid. Otherwise
-            # we always have to do a ping().
-            session_cookie = getattr(context, 'session_cookie', None)
-            if session_cookie and e.errcode == 401:
-                # Unauthorized. Remove the session and try again.
-                delattr(context, 'session_cookie')
-                try:
-                    principal = getattr(context, 'principal', None)
-                    delete_persistent_client_session_data(principal)
-                except Exception as e:
-                    # This shouldn't happen if we have a session but it isn't fatal.
-                    pass
 
-                # Create a new serverproxy with the non-session URI
-                serverproxy = self.create_connection(os.environ.get('KRB5CCNAME'), self.env.verbose, self.env.fallback, self.env.delegate)
-                setattr(context, self.id, Connection(serverproxy, self.disconnect))
-                return self.forward(name, *args, **kw)
-            raise NetworkError(uri=server, error=e.errmsg)
-        except socket.error as e:
-            raise NetworkError(uri=server, error=str(e))
-        except (OverflowError, TypeError) as e:
-            raise XMLRPCMarshallError(error=str(e))
+        # we'll be trying to connect multiple times with a new session cookie
+        # each time should we be getting UNAUTHORIZED error from the server
+        max_tries = 5
+        for try_num in range(0, max_tries):
+            self.log.info("[%d/%d]: Trying to forward '%s' to %s server '%s'",
+                          try_num, max_tries, name, self.protocol, server)
+            try:
+                return self._call_command(command, params)
+            except Fault as e:
+                e = decode_fault(e)
+                self.debug('Caught fault %d from server %s: %s', e.faultCode,
+                           server, e.faultString)
+                if e.faultCode in errors_by_code:
+                    error = errors_by_code[e.faultCode]
+                    raise error(message=e.faultString)
+                raise UnknownError(
+                    code=e.faultCode,
+                    error=e.faultString,
+                    server=server,
+                )
+            except ProtocolError as e:
+                # By catching a 401 here we can detect the case where we have
+                # a single IPA server and the session is invalid. Otherwise
+                # we always have to do a ping().
+                session_cookie = getattr(context, 'session_cookie', None)
+                if session_cookie and e.errcode == 401:
+                    # Unauthorized. Remove the session and try again.
+                    delattr(context, 'session_cookie')
+                    try:
+                        principal = getattr(context, 'principal', None)
+                        delete_persistent_client_session_data(principal)
+                    except Exception as e:
+                        # This shouldn't happen if we have a session
+                        # but it isn't fatal.
+                        self.debug("Error trying to remove persisent session "
+                                   "data: {err}".format(err=e))
+                        pass
+
+                    # Create a new serverproxy with the non-session URI
+                    serverproxy = self.create_connection(
+                        os.environ.get('KRB5CCNAME'), self.env.verbose,
+                        self.env.fallback, self.env.delegate)
+
+                    setattr(context, self.id,
+                            Connection(serverproxy, self.disconnect))
+                    # try to connect again with the new session cookie
+                    continue
+                raise NetworkError(uri=server, error=e.errmsg)
+            except (SSLError, socket.error) as e:
+                raise NetworkError(uri=server, error=str(e))
+            except (OverflowError, TypeError) as e:
+                raise XMLRPCMarshallError(error=str(e))
 
 
 class xmlclient(RPCClient):
_______________________________________________
FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org
To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org

Reply via email to