Am 30. Juni 2010 19:14 schrieb Iustin Pop <[email protected]>: > On Wed, Jun 30, 2010 at 06:04:56PM +0200, Michael Hanselmann wrote: >> There's one small drawback: Programs using libcurl have to call >> curl_global_init(3) (available as pycurl.global_init) when only >> one thread is running and are supposed to call >> curl_global_cleanup(3) (available as pycurl.global_cleanup) upon >> exiting. A decorator is provided to simplify this. > > What do you mean "when only one thread is running"? Can we skip this > step in multithreaded programs?
In short: it needs to called in all programs. In multi-threaded programs before starting any thread. Not calling global_init causes libcurl to do so automatically when doing the first transfer, which might be fine in single-threaded programs, but not when more than one thread is running. curl_global_init(3), referenced by the docstring, describes it in more detail: “This function must be called at least once within a program (a program is all the code that shares a memory space) before the program calls any other function in libcurl. […] This function is not thread safe. You must not call it when any other thread in the program (i.e. a thread sharing the same memory) is running. This doesn't just mean no other thread that is using libcurl. Because curl_global_init() calls functions of other libraries that are similarly thread unsafe, it could conflict with any other thread that uses these other libraries.” (http://curl.haxx.se/libcurl/c/curl_global_init.html) >> - �...@staticmethod >> - def _VerifySslCertCb(logger, _, cert, errnum, errdepth, ok): >> - """Callback for SSL certificate verification. >> - >> - �...@param logger: Logging object >> - >> - """ >> - if ok: >> - log_fn = logger.debug >> + logger.debug("Using cURL version %s", pycurl.version) >> + >> + sslver = _pycurl_version_fn()[5] > > this seems a bit cryptic; could you add either a comment with that that > function should return, or a comment with the actual function being > used? (I had to dig it up from the keyword args to this function). Done: + # pycurl.version_info returns a tuple with information about the used + # version of libcurl. Item 5 is the SSL library linked to it. + # e.g.: (3, '7.18.0', 463360, 'x86_64-pc-linux-gnu', 1581, 'GnuTLS/2.0.4', + # 0, '1.2.3.3', ...) >> - handlers = [_HTTPSHandler(self._logger, config_ssl_verification)] >> - >> + # Create pycURL object if not supplied >> + if not curl: >> + curl = pycurl.Curl() >> + >> + # Default cURL settings >> + curl.setopt(pycurl.VERBOSE, False) >> + curl.setopt(pycurl.FOLLOWLOCATION, True) > > why followlocation? we shouldn't have this in ganeti rapi daemon, right? Not at the moment, but I'd rather not have old(er) clients break just because we started using redirects for some of the resources they use. The number of redirects is limited: >> + curl.setopt(pycurl.MAXREDIRS, 5) > […] >> + # Get HTTP response code >> + http_code = curl.getinfo(pycurl.RESPONSE_CODE) >> + >> + if encoded_resp_body.tell(): >> + response_content = simplejson.loads(encoded_resp_body.getvalue()) >> else: >> response_content = None > > I'm a bit confused how this tell() works… Can you clarify? It checks whether anything was written to the buffer. Added a comment to this effect. >> + def testCertVerifyCurlBundle(self): >> + cfgfn = client.GenericCurlConfig(use_curl_cabundle=True) >> + >> + curl = FakeCurl(RapiMock()) >> + cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, >> + curl=curl) > > s/foo.com/example.com/ ? Done. >> + self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER)) >> + self.assertFalse(curl.getopt(pycurl.CAINFO)) >> + self.assertFalse(curl.getopt(pycurl.CAPATH)) >> + >> + def testCertVerifyCafile(self): >> + mycert = "/tmp/some/cert/file.pem" > > maybe adding a comment that these paths (here and below) are not actually > used would be > helpful :) No comment, but changed the paths to include /UNUSED/. Michael --- diff --git a/lib/rapi/client.py b/lib/rapi/client.py index a03fb40..65ce794 100644 --- a/lib/rapi/client.py +++ b/lib/rapi/client.py @@ -170,6 +170,10 @@ def GenericCurlConfig(verbose=False, use_signal=False, """ logger.debug("Using cURL version %s", pycurl.version) + # pycurl.version_info returns a tuple with information about the used + # version of libcurl. Item 5 is the SSL library linked to it. + # e.g.: (3, '7.18.0', 463360, 'x86_64-pc-linux-gnu', 1581, 'GnuTLS/2.0.4', + # 0, '1.2.3.3', ...) sslver = _pycurl_version_fn()[5] if not sslver: raise Error("No SSL support in cURL") @@ -383,6 +387,7 @@ class GanetiRapiClient(object): # Get HTTP response code http_code = curl.getinfo(pycurl.RESPONSE_CODE) + # Was anything written to the response buffer? if encoded_resp_body.tell(): response_content = simplejson.loads(encoded_resp_body.getvalue()) else: diff --git a/test/ganeti.rapi.client_unittest.py b/test/ganeti.rapi.client_unittest.py index 57e4e55..adfe1be 100755 --- a/test/ganeti.rapi.client_unittest.py +++ b/test/ganeti.rapi.client_unittest.py @@ -167,7 +167,7 @@ def _FakeGnuTlsPycurlVersion(): class TestExtendedConfig(unittest.TestCase): def testAuth(self): curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", + cl = client.GanetiRapiClient("master.example.com", username="user", password="pw", curl=curl) @@ -177,10 +177,10 @@ class TestExtendedConfig(unittest.TestCase): def testInvalidAuth(self): # No username self.assertRaises(client.Error, client.GanetiRapiClient, - "master.bar.com", password="pw") + "master-a.example.com", password="pw") # No password self.assertRaises(client.Error, client.GanetiRapiClient, - "master.bar.com", username="user") + "master-b.example.com", username="user") def testCertVerifyInvalidCombinations(self): self.assertRaises(client.Error, client.GenericCurlConfig, @@ -191,7 +191,7 @@ class TestExtendedConfig(unittest.TestCase): use_curl_cabundle=True, cafile="cert1.pem", capath="certs/") - def testProxy(self): + def testProxySignalVerifyHostname(self): for use_gnutls in [False, True]: if use_gnutls: pcverfn = _FakeGnuTlsPycurlVersion @@ -206,8 +206,8 @@ class TestExtendedConfig(unittest.TestCase): _pycurl_version_fn=pcverfn) curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, - curl=curl) + cl = client.GanetiRapiClient("master.example.com", + curl_config_fn=cfgfn, curl=curl) self.assertEqual(curl.getopt(pycurl.PROXY), proxy) self.assertEqual(curl.getopt(pycurl.NOSIGNAL), not use_signal) @@ -221,7 +221,7 @@ class TestExtendedConfig(unittest.TestCase): cfgfn = client.GenericCurlConfig() curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, + cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn, curl=curl) self.assertFalse(curl.getopt(pycurl.SSL_VERIFYPEER)) @@ -232,7 +232,7 @@ class TestExtendedConfig(unittest.TestCase): cfgfn = client.GenericCurlConfig(use_curl_cabundle=True) curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, + cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn, curl=curl) self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER)) @@ -240,11 +240,11 @@ class TestExtendedConfig(unittest.TestCase): self.assertFalse(curl.getopt(pycurl.CAPATH)) def testCertVerifyCafile(self): - mycert = "/tmp/some/cert/file.pem" + mycert = "/tmp/some/UNUSED/cert/file.pem" cfgfn = client.GenericCurlConfig(cafile=mycert) curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, + cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn, curl=curl) self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER)) @@ -252,13 +252,13 @@ class TestExtendedConfig(unittest.TestCase): self.assertFalse(curl.getopt(pycurl.CAPATH)) def testCertVerifyCapath(self): - certdir = "/tmp/some/cert/directory" + certdir = "/tmp/some/UNUSED/cert/directory" pcverfn = _FakeOpenSslPycurlVersion cfgfn = client.GenericCurlConfig(capath=certdir, _pycurl_version_fn=pcverfn) curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, + cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn, curl=curl) self.assert_(curl.getopt(pycurl.SSL_VERIFYPEER)) @@ -266,34 +266,34 @@ class TestExtendedConfig(unittest.TestCase): self.assertFalse(curl.getopt(pycurl.CAINFO)) def testCertVerifyCapathGnuTls(self): - certdir = "/tmp/some/cert/directory" + certdir = "/tmp/some/UNUSED/cert/directory" pcverfn = _FakeGnuTlsPycurlVersion cfgfn = client.GenericCurlConfig(capath=certdir, _pycurl_version_fn=pcverfn) curl = FakeCurl(RapiMock()) self.assertRaises(client.Error, client.GanetiRapiClient, - "master.foo.com", curl_config_fn=cfgfn, curl=curl) + "master.example.com", curl_config_fn=cfgfn, curl=curl) def testCertVerifyNoSsl(self): - certdir = "/tmp/some/cert/directory" + certdir = "/tmp/some/UNUSED/cert/directory" pcverfn = _FakeNoSslPycurlVersion cfgfn = client.GenericCurlConfig(capath=certdir, _pycurl_version_fn=pcverfn) curl = FakeCurl(RapiMock()) self.assertRaises(client.Error, client.GanetiRapiClient, - "master.foo.com", curl_config_fn=cfgfn, curl=curl) + "master.example.com", curl_config_fn=cfgfn, curl=curl) def testCertVerifyFancySsl(self): - certdir = "/tmp/some/cert/directory" + certdir = "/tmp/some/UNUSED/cert/directory" pcverfn = _FakeFancySslPycurlVersion cfgfn = client.GenericCurlConfig(capath=certdir, _pycurl_version_fn=pcverfn) curl = FakeCurl(RapiMock()) self.assertRaises(NotImplementedError, client.GanetiRapiClient, - "master.foo.com", curl_config_fn=cfgfn, curl=curl) + "master.example.com", curl_config_fn=cfgfn, curl=curl) def testCertVerifyCapath(self): for connect_timeout in [None, 1, 5, 10, 30, 60, 300]: @@ -302,7 +302,7 @@ class TestExtendedConfig(unittest.TestCase): timeout=timeout) curl = FakeCurl(RapiMock()) - cl = client.GanetiRapiClient("master.foo.com", curl_config_fn=cfgfn, + cl = client.GanetiRapiClient("master.example.com", curl_config_fn=cfgfn, curl=curl) self.assertEqual(curl.getopt(pycurl.CONNECTTIMEOUT), connect_timeout) @@ -315,7 +315,7 @@ class GanetiRapiClientTests(testutils.GanetiTestCase): self.rapi = RapiMock() self.curl = FakeCurl(self.rapi) - self.client = client.GanetiRapiClient("master.foo.com", + self.client = client.GanetiRapiClient("master.example.com", curl=self.curl) # Signals should be disabled by default
