[Bug 29799] Add tests for TLS server channels

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Sep 14 16:54:27 CEST 2010


https://bugs.freedesktop.org/show_bug.cgi?id=29799

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review-
         AssignedTo|telepathy-bugs at lists.freede |cosimoc at gnome.org
                   |sktop.org                   |
                 CC|                            |will.thompson at collabora.co.
                   |                            |uk

--- Comment #1 from Will Thompson <will.thompson at collabora.co.uk> 2010-09-14 07:54:26 PDT ---
Pleases rename the test to tls/server-tls-channel.py (removing the 'test-'
prefix)? It's obviously a test. It's in the directory full of tests. (I know
other tests violate this.)


+    def auth(self, auth):
+        assert (base64.b64decode(str(auth)) ==
+            '\x00%s\x00%s' % (self.username, self.password))
+
+        success = domish.Element((ns.NS_XMPP_SASL, 'success'))
+        self.xmlstream.send(success)
+        self.xmlstream.reset()
+        self.sasl_authenticated = True

Can't this subclass XmppAuthenticator and chain up to the superclass rather
than re-implementing it all? Ditto the streamStarted() method; it seems to
copy-paste a lot of code. bindIq() is literally a verbatim copy.

+        file = open(CA_KEY, 'rb')
+        pem_key = file.read()
+        file.close()
+        pkey = crypto.load_privatekey(crypto.FILETYPE_PEM, pem_key, "")

Better written as:

  with open(CA_KEY, 'rb') as file:
      pkey = crypto.load_privatekey(crypto.FILETYPE_PEM, pem_key, "")


+    q.expect_many(
+        EventPattern('dbus-signal', signal='Rejected'),
+        EventPattern('dbus-signal', signal='Closed'),
+        EventPattern('dbus-signal', signal='ChannelClosed'),
+        EventPattern('dbus-signal', signal='ConnectionError',
+                     args=[cs.CERT_UNTRUSTED, {}]),
+        EventPattern('dbus-signal', signal='StatusChanged',
+                     args=[cs.CONN_STATUS_DISCONNECTED,
cs.CSR_CERT_UNTRUSTED])
+        )

Don't we expect these to happen in a particular order? expect_many is for
ignoring the order of events.


+    # we expect the fallback verification process to connect successfully

There should be a test for when it doesn't, presumably by setting {
'require-encryption': True, 'ignore-ssl-errors': False }. And, why do we expect
the fallback verification process to connect successfully in this case? I think
we should document exactly what domain the certificate is for, probably in this
test, rather than relying on later readers asking you on IRC or digging out a
tool to examine certificates.

+    assertEquals (len(rejections), 0)

assertLength(0, rejections)

Python coding style doesn't put a space before the opening paren.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list