You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2019/07/07 16:17:01 UTC
[thrift] 02/03: THRIFT-4904: Fix python unit test errors and
exception escapes
This is an automated email from the ASF dual-hosted git repository.
jking pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git
commit 3131fe975ce2efd2887e41f86e73f7205a02a2a4
Author: James E. King III <jk...@apache.org>
AuthorDate: Tue Jul 2 14:21:05 2019 -0400
THRIFT-4904: Fix python unit test errors and exception escapes
Due to the way SSL layers on top of sockets, it was possible
to complete a connection and then have the server close it.
This would happen if the client is not checking certificates
but the server is. The TSSLSocket unit test was enhanced to
do a read and a write as well as just connecting to ensure a
more complete test.
The TSocket read() and write() calls were leaking OSError,
socker.error, and ssl.Error exceptions. These cases are now
wrapped into a TTransportException of the appropriate type,
and the original exception is added as an argument named inner.
---
lib/py/src/transport/TSSLSocket.py | 6 +++---
lib/py/src/transport/TSocket.py | 27 ++++++++++++++++-----------
lib/py/src/transport/TTransport.py | 3 ++-
lib/py/test/test_sslsocket.py | 10 ++++++++--
4 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/lib/py/src/transport/TSSLSocket.py b/lib/py/src/transport/TSSLSocket.py
index 066d8da..5b3ae59 100644
--- a/lib/py/src/transport/TSSLSocket.py
+++ b/lib/py/src/transport/TSSLSocket.py
@@ -291,11 +291,11 @@ class TSSLSocket(TSocket.TSocket, TSSLBase):
plain_sock = socket.socket(family, socktype)
try:
return self._wrap_socket(plain_sock)
- except Exception:
+ except Exception as ex:
plain_sock.close()
msg = 'failed to initialize SSL'
logger.exception(msg)
- raise TTransportException(TTransportException.NOT_OPEN, msg)
+ raise TTransportException(type=TTransportException.NOT_OPEN, message=msg, inner=ex)
def open(self):
super(TSSLSocket, self).open()
@@ -307,7 +307,7 @@ class TSSLSocket(TSocket.TSocket, TSSLBase):
except TTransportException:
raise
except Exception as ex:
- raise TTransportException(TTransportException.UNKNOWN, str(ex))
+ raise TTransportException(message=str(ex), inner=ex)
class TSSLServerSocket(TSocket.TServerSocket, TSSLBase):
diff --git a/lib/py/src/transport/TSocket.py b/lib/py/src/transport/TSocket.py
index c8be25a..df25d42 100644
--- a/lib/py/src/transport/TSocket.py
+++ b/lib/py/src/transport/TSocket.py
@@ -94,13 +94,13 @@ class TSocket(TSocketBase):
def open(self):
if self.handle:
- raise TTransportException(TTransportException.ALREADY_OPEN)
+ raise TTransportException(type=TTransportException.ALREADY_OPEN, message="already open")
try:
addrs = self._resolveAddr()
- except socket.gaierror:
+ except socket.gaierror as gai:
msg = 'failed to resolve sockaddr for ' + str(self._address)
logger.exception(msg)
- raise TTransportException(TTransportException.NOT_OPEN, msg)
+ raise TTransportException(type=TTransportException.NOT_OPEN, message=msg, inner=gai)
for family, socktype, _, _, sockaddr in addrs:
handle = self._do_open(family, socktype)
@@ -119,7 +119,7 @@ class TSocket(TSocketBase):
msg = 'Could not connect to any of %s' % list(map(lambda a: a[4],
addrs))
logger.error(msg)
- raise TTransportException(TTransportException.NOT_OPEN, msg)
+ raise TTransportException(type=TTransportException.NOT_OPEN, message=msg)
def read(self, sz):
try:
@@ -134,8 +134,10 @@ class TSocket(TSocketBase):
self.close()
# Trigger the check to raise the END_OF_FILE exception below.
buff = ''
+ elif e.args[0] == errno.ETIMEDOUT:
+ raise TTransportException(type=TTransportException.TIMED_OUT, message="read timeout", inner=e)
else:
- raise
+ raise TTransportException(message="unexpected exception", inner=e)
if len(buff) == 0:
raise TTransportException(type=TTransportException.END_OF_FILE,
message='TSocket read 0 bytes')
@@ -148,12 +150,15 @@ class TSocket(TSocketBase):
sent = 0
have = len(buff)
while sent < have:
- plus = self.handle.send(buff)
- if plus == 0:
- raise TTransportException(type=TTransportException.END_OF_FILE,
- message='TSocket sent 0 bytes')
- sent += plus
- buff = buff[plus:]
+ try:
+ plus = self.handle.send(buff)
+ if plus == 0:
+ raise TTransportException(type=TTransportException.END_OF_FILE,
+ message='TSocket sent 0 bytes')
+ sent += plus
+ buff = buff[plus:]
+ except socket.error as e:
+ raise TTransportException(message="unexpected exception", inner=e)
def flush(self):
pass
diff --git a/lib/py/src/transport/TTransport.py b/lib/py/src/transport/TTransport.py
index 8573ba7..9dbe95d 100644
--- a/lib/py/src/transport/TTransport.py
+++ b/lib/py/src/transport/TTransport.py
@@ -34,9 +34,10 @@ class TTransportException(TException):
SIZE_LIMIT = 6
INVALID_CLIENT_TYPE = 7
- def __init__(self, type=UNKNOWN, message=None):
+ def __init__(self, type=UNKNOWN, message=None, inner=None):
TException.__init__(self, message)
self.type = type
+ self.inner = inner
class TTransportBase(object):
diff --git a/lib/py/test/test_sslsocket.py b/lib/py/test/test_sslsocket.py
index 598c174..f4c87f1 100644
--- a/lib/py/test/test_sslsocket.py
+++ b/lib/py/test/test_sslsocket.py
@@ -75,6 +75,9 @@ class ServerAcceptor(threading.Thread):
try:
self._client = self._server.accept()
+ if self._client:
+ self._client.read(5) # hello
+ self._client.write(b"there")
except Exception:
logging.exception('error on server side (%s):' % self.name)
if not self._expect_failure:
@@ -141,7 +144,8 @@ class TSSLSocketTest(unittest.TestCase):
client.setTimeout(20)
with self._assert_raises(TTransportException):
client.open()
- self.assertTrue(acc.client is None)
+ client.write(b"hello")
+ client.read(5) # b"there"
finally:
logging.disable(logging.NOTSET)
@@ -153,8 +157,10 @@ class TSSLSocketTest(unittest.TestCase):
def _assert_connection_success(self, server, path=None, **client_args):
with self._connectable_client(server, path=path, **client_args) as (acc, client):
- client.open()
try:
+ client.open()
+ client.write(b"hello")
+ self.assertEqual(client.read(5), b"there")
self.assertTrue(acc.client is not None)
finally:
client.close()