Fixed bug with handling of redirect data returned by some SCAN listeners

(#39).

Author: anthony-tuininga

doc/src/release_notes.rst

@@ -18,6 +18,8 @@ Thin Mode Changes
     the database sends a password challenge with a verifier type that is not
     recognized, instead of `ORA-01017: invalid username/password`
     (`issue 26 <https://github.com/oracle/python-oracledb/issues/26>`__).
+#)  Fixed bug with handling of redirect data returned by some SCAN listeners
+    (`issue 39 <https://github.com/oracle/python-oracledb/issues/39>`__).
 
 Common Changes
 ++++++++++++++

src/oracledb/base_impl.pxd

@@ -186,7 +186,6 @@ cdef class ConnectParamsImpl:
     cdef str _get_wallet_password(self)
     cdef int _parse_connect_string(self, str connect_string) except -1
     cdef int _process_connect_descriptor(self, dict args) except -1
-    cdef int _process_redirect_data(self, str redirect_data) except -1
     cdef int _set_new_password(self, str password) except -1
     cdef int _set_password(self, str password) except -1
     cdef int _set_wallet_password(self, str password) except -1

src/oracledb/connect_params.py

@@ -264,12 +264,9 @@ def _address_attr(f):
         """
         @functools.wraps(f)
         def wrapped(self):
-            output = []
-            for description in self._impl.description_list.descriptions:
-                for address_list in description.address_lists:
-                    for address in address_list.addresses:
-                        output.append(getattr(address, f.__name__))
-            return output if len(output) > 1 else output[0]
+            values = [getattr(a, f.__name__) \
+                      for a in self._impl._get_addresses()]
+            return values if len(values) > 1 else values[0]
         return wrapped
 
     def _description_attr(f):
@@ -278,10 +275,9 @@ def _description_attr(f):
         """
         @functools.wraps(f)
         def wrapped(self):
-            output = []
-            for description in self._impl.description_list.descriptions:
-                output.append(getattr(description, f.__name__))
-            return output if len(output) > 1 else output[0]
+            values = [getattr(d, f.__name__) \
+                      for d in self._impl.description_list.descriptions]
+            return values if len(values) > 1 else values[0]
         return wrapped
 
     @property

src/oracledb/impl/base/connect_params.pyx

@@ -335,21 +335,6 @@ cdef class ConnectParamsImpl:
                     address.set_from_args(addr_args)
                     address_list.addresses.append(address)
 
-    cdef int _process_redirect_data(self, str redirect_data) except -1:
-        """
-        Internal method used for parsing the redirect data that is returned
-        from a listener in order to determine the new host and part that should
-        be used to connect to the database.
-        """
-        cdef:
-            dict args = {}
-        pos = redirect_data.find('\x00')
-        if pos < 0:
-            errors._raise_err(errors.ERR_INVALID_REDIRECT_DATA,
-                              data=redirect_data)
-        _parse_connect_descriptor(redirect_data[:pos], args)
-        self._process_connect_descriptor(args)
-
     cdef int _set_new_password(self, str password) except -1:
         """
         Sets the new password on the instance after first obfuscating it.
@@ -402,6 +387,18 @@ cdef class ConnectParamsImpl:
         new_params._copy(self)
         return new_params
 
+    def _get_addresses(self):
+        """
+        Return a list of the stored addresses.
+        """
+        cdef:
+            AddressList addr_list
+            Description desc
+            Address addr
+        return [addr for desc in self.description_list.descriptions \
+                for addr_list in desc.address_lists \
+                for addr in addr_list.addresses]
+
     def get_connect_string(self):
         """
         Internal method for getting the connect string. This will either be the

src/oracledb/impl/thin/connection.pyx

@@ -85,9 +85,37 @@ cdef class ThinConnImpl(BaseConnImpl):
             elif stmt._cursor_id != 0:
                 self._add_cursor_to_close(stmt)
 
-    cdef object _connect_with_description(self, Description description,
-                                          ConnectParamsImpl params,
-                                          bint final_desc):
+    cdef int _connect_with_address(self, Address address,
+                                   Description description,
+                                   ConnectParamsImpl params,
+                                   bint raise_exception) except -1:
+        """
+        Internal method used for connecting with the given description and
+        address.
+        """
+        try:
+            self._protocol._connect_phase_one(self, params, description,
+                                              address)
+        except exceptions.DatabaseError:
+            if raise_exception:
+                raise
+            return 0
+        except (socket.gaierror, ConnectionRefusedError) as e:
+            if raise_exception:
+                errors._raise_err(errors.ERR_CONNECTION_FAILED, cause=e,
+                                  exception=str(e))
+            return 0
+        except Exception as e:
+            errors._raise_err(errors.ERR_CONNECTION_FAILED, cause=e,
+                              exception=str(e))
+        self._drcp_enabled = description.server_type == "pooled"
+        if self._cclass is None:
+            self._cclass = description.cclass
+        self._protocol._connect_phase_two(self, description, params)
+
+    cdef int _connect_with_description(self, Description description,
+                                       ConnectParamsImpl params,
+                                       bint final_desc) except -1:
         cdef:
             bint load_balance = description.load_balance
             bint raise_exc = False
@@ -126,25 +154,18 @@ cdef class ThinConnImpl(BaseConnImpl):
                         raise_exc = i == num_attempts - 1 \
                                 and j == num_lists - 1 \
                                 and k == num_addresses - 1
-                    redirect_params = self._connect_with_address(address,
-                                                                 description,
-                                                                 params,
-                                                                 raise_exc)
-                    if redirect_params is not None:
-                        return redirect_params
+                    self._connect_with_address(address, description, params,
+                                               raise_exc)
                     if self._protocol._in_connect:
                         continue
                     address_list.lru_index = (idx1 + 1) % num_addresses
                     description.lru_index = (idx2 + 1) % num_lists
-                    return
+                    return 0
             time.sleep(description.retry_delay)
 
-    cdef ConnectParamsImpl _connect_with_params(self,
-                                                ConnectParamsImpl params):
+    cdef int _connect_with_params(self, ConnectParamsImpl params) except -1:
         """
-        Internal method used for connecting with the given parameters. If the
-        listener requests a redirect, the redirect data is returned so that
-        this process can be repeated as needed.
+        Internal method used for connecting with the given parameters.
         """
         cdef:
             DescriptionList description_list = params.description_list
@@ -160,14 +181,10 @@ cdef class ThinConnImpl(BaseConnImpl):
             else:
                 idx = i
             description = descriptions[idx]
-            redirect_params = self._connect_with_description(description,
-                                                             params,
-                                                             final_desc)
-            if redirect_params is not None \
-                    or not self._protocol._in_connect:
+            self._connect_with_description(description, params, final_desc)
+            if not self._protocol._in_connect:
                 description_list.lru_index = (idx + 1) % num_descriptions
                 break
-        return redirect_params
 
     cdef Message _create_message(self, type typ):
         """
@@ -183,74 +200,6 @@ cdef class ThinConnImpl(BaseConnImpl):
         self._pool = None
         self._protocol._force_close()
 
-    cdef object _connect_with_address(self, Address address,
-                                      Description description,
-                                      ConnectParamsImpl params,
-                                      bint raise_exception):
-        """
-        Creates a socket on which to communicate using the provided parameters.
-        If a proxy is configured, a connection to the proxy is established and
-        the target host and port is forwarded to the proxy. The socket is used
-        to establish a connection with the database. If a redirect is
-        required, the redirect parameters are returned.
-        """
-        cdef:
-            bint use_proxy = (address.https_proxy is not None)
-            double timeout = description.tcp_connect_timeout
-        if use_proxy:
-            if address.protocol != "tcps":
-                errors._raise_err(errors.ERR_HTTPS_PROXY_REQUIRES_TCPS)
-            connect_info = (address.https_proxy, address.https_proxy_port)
-        else:
-            connect_info = (address.host, address.port)
-        try:
-            sock = socket.create_connection(connect_info, timeout)
-            if use_proxy:
-                data = f"CONNECT {address.host}:{address.port} HTTP/1.0\r\n\r\n"
-                sock.send(data.encode())
-                reply = sock.recv(1024)
-                match = re.search('HTTP/1.[01]\\s+(\\d+)\\s+', reply.decode())
-                if match is None or match.groups()[0] != '200':
-                    errors._raise_err(errors.ERR_PROXY_FAILURE,
-                                    response=reply.decode())
-            if description.expire_time > 0:
-                sock.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)
-                if hasattr(socket, "TCP_KEEPIDLE") \
-                        and hasattr(socket, "TCP_KEEPINTVL") \
-                        and hasattr(socket, "TCP_KEEPCNT"):
-                    sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPIDLE,
-                                    description.expire_time * 60)
-                    sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPINTVL,
-                                    6)
-                    sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_KEEPCNT,
-                                    10)
-            sock.settimeout(None)
-            if address.protocol == "tcps":
-                sock = get_ssl_socket(sock, params, description, address)
-            self._drcp_enabled = description.server_type == "pooled"
-            if self._cclass is None:
-                self._cclass = description.cclass
-            self._protocol._set_socket(sock)
-            redirect_params = self._protocol._connect_phase_one(self, params,
-                                                                description,
-                                                                address)
-            if redirect_params is not None:
-                return redirect_params
-        except exceptions.DatabaseError:
-            if raise_exception:
-                raise
-            return
-        except (socket.gaierror, ConnectionRefusedError) as e:
-            if raise_exception:
-                errors._raise_err(errors.ERR_CONNECTION_FAILED, cause=e,
-                                  exception=str(e))
-            return
-        except Exception as e:
-            errors._raise_err(errors.ERR_CONNECTION_FAILED, cause=e,
-                              exception=str(e))
-            return
-        self._protocol._connect_phase_two(self, description, params)
-
     cdef Statement _get_statement(self, str sql, bint cache_statement):
         """
         Get a statement from the statement cache, or prepare a new statement
@@ -340,14 +289,9 @@ cdef class ThinConnImpl(BaseConnImpl):
         self._protocol._process_single_message(message)
 
     def connect(self, ConnectParamsImpl params):
-        cdef ConnectParamsImpl redirect_params
         if params._password is None:
             errors._raise_err(errors.ERR_NO_PASSWORD)
-        while True:
-            redirect_params = self._connect_with_params(params)
-            if redirect_params is None:
-                break
-            params = redirect_params
+        self._connect_with_params(params)
         self._statement_cache = collections.OrderedDict()
         self._statement_cache_size = params.stmtcachesize
         self._statement_cache_lock = threading.Lock()

src/oracledb/impl/thin/messages.pyx

@@ -1540,7 +1540,8 @@ cdef class ConnectMessage(Message):
         bytes connect_string_bytes
         Description description
         str redirect_data
-        Address address
+        str host
+        int port
 
     cdef int process(self, ReadBuffer buf) except -1:
         cdef:
@@ -1572,13 +1573,11 @@ cdef class ConnectMessage(Message):
             if error_code_int == TNS_ERR_INVALID_SERVICE_NAME:
                 errors._raise_err(errors.ERR_INVALID_SERVICE_NAME,
                                   service_name=self.description.service_name,
-                                  host=self.address.host,
-                                  port=self.address.port)
+                                  host=self.host, port=self.port)
             elif error_code_int == TNS_ERR_INVALID_SID:
                 errors._raise_err(errors.ERR_INVALID_SID,
                                   sid=self.description.sid,
-                                  host=self.address.host,
-                                  port=self.address.port)
+                                  host=self.host, port=self.port)
             errors._raise_err(errors.ERR_LISTENER_REFUSED_CONNECTION,
                               error_code=error_code)