ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
20 stars 33 forks source link

Use requests for HTTP/HTTPS calls in library #240

Closed chris-allan closed 4 years ago

chris-allan commented 4 years ago

CentOS 7 ships with version 1.0.2k of OpenSSL. These versions of OpenSSL had various forms of global state that the Ice developers decided, likely for a very good reason, to cleanup when the Ice OpenSSL engine is destroyed:

Unfortunately, calling EVP_Cleanup() like this causes the global algorithms table to end up missing and/or corrupt:

This will break other uses of OpenSSL in the same process. The most obvious condition where this can occur is making an HTTPS request after an OMERO session has been closed. For example (Ubuntu 16.04, OpenSSL 1.0.2g):

$ omero shell --login
Created session for chris.allan@localhost:4064. Idle timeout: 10 min. Current group: chris.allan
Python 3.5.2 (default, Oct  8 2019, 13:06:37)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.9.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: client.closeSession()

In [2]: import urllib

In [3]: urllib.request.urlopen('http://www.glencoesoftware.com/').read()
---------------------------------------------------------------------------
SSLError                                  Traceback (most recent call last)
<ipython-input-3-474f67ef9573> in <module>
----> 1 urllib.request.urlopen('http://www.glencoesoftware.com/').read()

/usr/lib/python3.5/urllib/request.py in urlopen(url, data, timeout, cafile, capath, cadefault, context)
    161     else:
    162         opener = _opener
--> 163     return opener.open(url, data, timeout)
    164
    165 def install_opener(opener):

/usr/lib/python3.5/urllib/request.py in open(self, fullurl, data, timeout)
    470         for processor in self.process_response.get(protocol, []):
    471             meth = getattr(processor, meth_name)
--> 472             response = meth(req, response)
    473
    474         return response

/usr/lib/python3.5/urllib/request.py in error(self, proto, *args)
    502             http_err = 0
    503         args = (dict, proto, meth_name) + args
--> 504         result = self._call_chain(*args)
    505         if result:
    506             return result

/usr/lib/python3.5/urllib/request.py in _call_chain(self, chain, kind, meth_name, *args)
    442         for handler in handlers:
    443             func = getattr(handler, meth_name)
--> 444             result = func(*args)
    445             if result is not None:
    446                 return result

/usr/lib/python3.5/urllib/request.py in http_error_302(self, req, fp, code, msg, headers)
    694         fp.close()
    695
--> 696         return self.parent.open(new, timeout=req.timeout)
    697
    698     http_error_301 = http_error_303 = http_error_307 = http_error_302

/usr/lib/python3.5/urllib/request.py in open(self, fullurl, data, timeout)
    464             req = meth(req)
    465
--> 466         response = self._open(req, data)
    467
    468         # post-process response

/usr/lib/python3.5/urllib/request.py in _open(self, req, data)
    482         protocol = req.type
    483         result = self._call_chain(self.handle_open, protocol, protocol +
--> 484                                   '_open', req)
    485         if result:
    486             return result

/usr/lib/python3.5/urllib/request.py in _call_chain(self, chain, kind, meth_name, *args)
    442         for handler in handlers:
    443             func = getattr(handler, meth_name)
--> 444             result = func(*args)
    445             if result is not None:
    446                 return result

/usr/lib/python3.5/urllib/request.py in https_open(self, req)
   1295         def https_open(self, req):
   1296             return self.do_open(http.client.HTTPSConnection, req,
-> 1297                 context=self._context, check_hostname=self._check_hostname)
   1298
   1299         https_request = AbstractHTTPHandler.do_request_

/usr/lib/python3.5/urllib/request.py in do_open(self, http_class, req, **http_conn_args)
   1221
   1222         # will parse host:port
-> 1223         h = http_class(host, timeout=req.timeout, **http_conn_args)
   1224         h.set_debuglevel(self._debuglevel)
   1225

/usr/lib/python3.5/http/client.py in __init__(self, host, port, key_file, cert_file, timeout, source_address, context, check_hostname)
   1251             self.cert_file = cert_file
   1252             if context is None:
-> 1253                 context = ssl._create_default_https_context()
   1254             will_verify = context.verify_mode != ssl.CERT_NONE
   1255             if check_hostname is None:

/usr/lib/python3.5/ssl.py in create_default_context(purpose, cafile, capath, cadata)
    439         raise TypeError(purpose)
    440
--> 441     context = SSLContext(PROTOCOL_SSLv23)
    442
    443     # SSLv2 considered harmful.

/usr/lib/python3.5/ssl.py in __new__(cls, protocol, *args, **kwargs)
    359
    360     def __new__(cls, protocol, *args, **kwargs):
--> 361         self = _SSLContext.__new__(cls, protocol)
    362         if protocol != _SSLv2_IF_EXISTS:
    363             self.set_ciphers(_DEFAULT_CIPHERS)

SSLError: ('failed to allocate SSL context',)

The easiest way to work around this is to bring pyopenssl, which is linked against a version of OpenSSL we control, into the equation. In order to have both OpenSSL versions functional in the process we need to use a Python package that is mindful of these conditions and will optionally use pyopenssl if it is available. requests is such a package. Using requests with pyopenssl installed this should be your output:

$ omero shell --login
Using session for chris.allan@localhost:4064. Idle timeout: 10 min. Current group: chris.allan
Python 3.5.2 (default, Oct  8 2019, 13:06:37)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.9.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: client.closeSession()

In [2]: import requests

In [3]: r = requests.get('https://www.glencoesoftware.com/')

In [4]: r.status_code
Out[4]: 200

In [5]:

This PR makes changes to the usage of urllib in the library to use requests instead.

chris-allan commented 4 years ago

/cc @kkoz

snoopycrimecop commented 4 years ago

Conflicting PR. Removed from build OMERO-python-superbuild-push#403. See the console output for more details. Possible conflicts:

--conflicts Conflict resolved in build OMERO-python-superbuild-push#404. See the console output for more details.

mtbc commented 4 years ago
# requests timeout, default is no timeout
# * https://requests.readthedocs.io/en/master/user/quickstart/#timeouts
#
DEFAULT_TIMEOUT = 6.0

reads oddly contradictorily but perhaps it makes sense?

jburel commented 4 years ago

No test failure today merging