launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/isolate-gpgme into lp:launchpad.

Commit message:
Isolate gpgme from its terminal environment, if any.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/isolate-gpgme/+merge/310906

I've been having problems for ages with spurious gpgme-related test failures in my local environment that went away when I piped the output through cat, and this was particularly annoying any time I needed to apply pdb to one of them.  This isolates things so that that isn't a problem any more.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/isolate-gpgme into lp:launchpad.

=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py	2016-11-03 15:07:36 +0000
+++ lib/lp/services/gpg/handler.py	2016-11-15 17:42:52 +0000
@@ -5,12 +5,14 @@
 
 __all__ = [
     'GPGHandler',
+    'isolate_gpgme',
     'PymeKey',
     'PymeSignature',
     'PymeUserId',
     ]
 
 import atexit
+from contextlib import contextmanager
 import httplib
 import os
 import shutil
@@ -65,6 +67,34 @@
 """
 
 
+@contextmanager
+def isolate_gpgme():
+    """Isolate gpgme from its terminal environment, if any.
+
+    Ensure that stdout is not a tty.  gpgme tries to enable interactive
+    behaviour if this is the case.  We never want that, and it can cause
+    test failures in some environments, so make sure it's not the case while
+    calling into gpgme.
+
+    Make sure that gpgme does not have a DISPLAY.
+    """
+    if os.isatty(1):
+        old_stdout = os.dup(1)
+        devnull = os.open(os.devnull, os.O_WRONLY)
+        os.dup2(devnull, 1)
+    else:
+        old_stdout = None
+    old_display = os.environ.pop('DISPLAY', None)
+    try:
+        yield
+    finally:
+        if old_display is not None:
+            os.environ['DISPLAY'] = old_display
+        if old_stdout is not None:
+            os.dup2(old_stdout, 1)
+            os.close(old_stdout)
+
+
 @implementer(IGPGHandler)
 class GPGHandler:
     """See IGPGHandler."""
@@ -178,7 +208,8 @@
 
         # process it
         try:
-            signatures = ctx.verify(*args)
+            with isolate_gpgme():
+                signatures = ctx.verify(*args)
         except gpgme.GpgmeError as e:
             error = GPGVerificationError(e.strerror)
             for attr in ("args", "code", "signatures", "source"):
@@ -230,7 +261,8 @@
         context.armor = True
 
         newkey = StringIO(content)
-        result = context.import_(newkey)
+        with isolate_gpgme():
+            result = context.import_(newkey)
 
         if len(result.imports) == 0:
             raise GPGKeyNotFoundError(content)
@@ -264,7 +296,8 @@
         context = gpgme.Context()
         context.armor = True
         newkey = StringIO(content)
-        import_result = context.import_(newkey)
+        with isolate_gpgme():
+            import_result = context.import_(newkey)
 
         secret_imports = [
             fingerprint
@@ -277,7 +310,8 @@
 
         fingerprint, result, status = import_result.imports[0]
         try:
-            key = context.get_key(fingerprint, True)
+            with isolate_gpgme():
+                key = context.get_key(fingerprint, True)
         except gpgme.GpgmeError:
             return None
 
@@ -296,8 +330,9 @@
         # Only 'utf-8' encoding is supported by gpgme.
         # See more information at:
         # http://pyme.sourceforge.net/doc/gpgme/Generating-Keys.html
-        result = context.genkey(
-            signing_only_param % {'name': name.encode('utf-8')})
+        with isolate_gpgme():
+            result = context.genkey(
+                signing_only_param % {'name': name.encode('utf-8')})
 
         # Right, it might seem paranoid to have this many assertions,
         # but we have to take key generation very seriously.
@@ -340,9 +375,10 @@
                              % key.fingerprint)
 
         # encrypt content
-        ctx.encrypt(
-            [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
-            plain, cipher)
+        with isolate_gpgme():
+            ctx.encrypt(
+                [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
+                plain, cipher)
 
         return cipher.getvalue()
 
@@ -375,7 +411,8 @@
 
         # Sign the text.
         try:
-            context.sign(plaintext, signature, mode)
+            with isolate_gpgme():
+                context.sign(plaintext, signature, mode)
         except gpgme.GpgmeError:
             return None
 
@@ -393,8 +430,9 @@
         if type(filter) == unicode:
             filter = filter.encode('utf-8')
 
-        for key in ctx.keylist(filter, secret):
-            yield PymeKey.newFromGpgmeKey(key)
+        with isolate_gpgme():
+            for key in ctx.keylist(filter, secret):
+                yield PymeKey.newFromGpgmeKey(key)
 
     def retrieveKey(self, fingerprint):
         """See IGPGHandler."""
@@ -549,7 +587,8 @@
         context = gpgme.Context()
         # retrive additional key information
         try:
-            key = context.get_key(fingerprint, False)
+            with isolate_gpgme():
+                key = context.get_key(fingerprint, False)
         except gpgme.GpgmeError:
             key = None
 
@@ -597,7 +636,8 @@
         context = gpgme.Context()
         context.armor = True
         keydata = StringIO()
-        context.export(self.fingerprint.encode('ascii'), keydata)
+        with isolate_gpgme():
+            context.export(self.fingerprint.encode('ascii'), keydata)
 
         return keydata.getvalue()
 

=== modified file 'lib/lp/testing/gpgkeys/__init__.py'
--- lib/lp/testing/gpgkeys/__init__.py	2016-11-03 15:07:36 +0000
+++ lib/lp/testing/gpgkeys/__init__.py	2016-11-15 17:42:52 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """OpenPGP keys used for testing.
@@ -27,11 +27,13 @@
 
 from lp.registry.interfaces.gpg import IGPGKeySet
 from lp.registry.interfaces.person import IPersonSet
+from lp.services.gpg.handler import isolate_gpgme
 from lp.services.gpg.interfaces import (
     GPGKeyAlgorithm,
     IGPGHandler,
     )
 
+
 gpgkeysdir = os.path.join(os.path.dirname(__file__), 'data')
 
 
@@ -141,9 +143,10 @@
 
     ctx.passphrase_cb = passphrase_cb
 
-    # Do the deecryption.
+    # Do the decryption.
     try:
-        ctx.decrypt(cipher, plain)
+        with isolate_gpgme():
+            ctx.decrypt(cipher, plain)
     except gpgme.GpgmeError:
         return None