Rev 2327: (Vincent Ladeuil) Improved infrastructure for password handling. in http://bzr.arbash-meinel.com/branches/bzr/jam-integration

John Arbash Meinel john at arbash-meinel.com
Thu Mar 8 22:08:16 GMT 2007


At http://bzr.arbash-meinel.com/branches/bzr/jam-integration

------------------------------------------------------------
revno: 2327
revision-id: john at arbash-meinel.com-20070308220808-3gdl6wjgje0ydbv5
parent: pqm at pqm.ubuntu.com-20070308215322-265e2cf0b768f432
parent: v.ladeuil+lp at free.fr-20070221144606-qoy01pue1p187j3b
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: jam-integration
timestamp: Thu 2007-03-08 16:08:08 -0600
message:
  (Vincent Ladeuil) Improved infrastructure for password handling.
modified:
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/blackbox/__init__.py __init__.py-20051128053524-eba30d8255e08dc3
  bzrlib/tests/blackbox/test_reconcile.py test_fix.py-20060223013051-9a188e15a5ee9451
  bzrlib/tests/blackbox/test_selftest.py test_selftest.py-20060123024542-01c5f1bbcb596d78
  bzrlib/tests/blackbox/test_upgrade.py test_upgrade.py-20060120060132-b41e5ed2f886ad28
  bzrlib/tests/test_ui.py        test_ui.py-20051130162854-458e667a7414af09
  bzrlib/ui/__init__.py          ui.py-20050824083933-8cf663c763ba53a9
  bzrlib/ui/text.py              text.py-20051130153916-2e438cffc8afc478
    ------------------------------------------------------------
    revno: 2294.4.4
    merged: v.ladeuil+lp at free.fr-20070221144606-qoy01pue1p187j3b
    parent: v.ladeuil+lp at free.fr-20070220080240-ti00ro17rxfqe86z
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: fakestdin
    timestamp: Wed 2007-02-21 15:46:06 +0100
    message:
      Provide a better implementation for testing passwords.
      
      * bzrlib/ui/__init__.py:
      (UIFactory.get_login): Deleted.
      (CLIUIFactory.get_non_echoed_password): New method allowing
      overriding.
      
      * bzrlib/tests/__init__.py:
      (TestUIFactory.get_non_echoed_password): Allows password testing
       without worrying about echo echo.
      
      * bzrlib/tests/__init__.py:
      (TestUIFactory): Moved from bzrlib/tests/blackbox/__init__.py
      (FakeStdin): Deleted.
      (TestCase.run_bzr_captured): Set and reuse ui.ui_factory.stdin.
      
      * bzrlib/ui/text.py:
      (TextUIFactory.get_login): Deleted.
      (TextUIFactory.get_password): Moved to CLIUIFactory.
      
      * bzrlib/tests/test_ui.py:
      (UITests): Delete get_login tests.
      (FakeTextUIFactory): Deleted. Better implementation in
      TestUIFactory.
      
      * bzrlib/tests/blackbox/__init__.py:
      (TestUIFactory): Moved to bzrlib/tests/__init__.py.
    ------------------------------------------------------------
    revno: 2294.4.3
    merged: v.ladeuil+lp at free.fr-20070220080240-ti00ro17rxfqe86z
    parent: v.ladeuil+lp at free.fr-20070220075141-92289xpd1mcfmjaq
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: fakestdin
    timestamp: Tue 2007-02-20 09:02:40 +0100
    message:
      Fix typo.
    ------------------------------------------------------------
    revno: 2294.4.2
    merged: v.ladeuil+lp at free.fr-20070220075141-92289xpd1mcfmjaq
    parent: v.ladeuil+lp at free.fr-20070219210827-qcz46is44zdpfjxr
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: fakestdin
    timestamp: Tue 2007-02-20 08:51:41 +0100
    message:
      Take Martin's review comments into account.
    ------------------------------------------------------------
    revno: 2294.4.1
    merged: v.ladeuil+lp at free.fr-20070219210827-qcz46is44zdpfjxr
    parent: pqm at pqm.ubuntu.com-20070217025822-306d98c244b53b08
    committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
    branch nick: fakestdin
    timestamp: Mon 2007-02-19 22:08:27 +0100
    message:
      Add a UIFactory.get_login method, fix tests.
      
      * bzrlib/ui/text.py:
      (TextUIFactory.prompt): Made usable for more than get_boolean by
      deleting the [y/n] part.
      (TextUIFactory.get_login): New method.
      
      * bzrlib/ui/__init__.py:
      (UIFactory.get_login): New method.
      (CLIUIFactory.get_boolean): Add the [y/n] part deleted from
      prompt().
      
      * bzrlib/tests/test_ui.py:
      (FakeTextUIFactory): New test class.
      (UITests.test_silent_factory): Add no output testing.
      (UITests.test_text_factory_ascii_password,
      UITests.test_text_factory_utf8_password,
      UITests.test_text_factory_ascii_password,
      UITests.test_text_factory_utf8_password): New tests replacing
      test_text_factory.
      (UITests.test_text_factory_prompts_and_clears): Use a
      DotsProgressBar so that we can run the tests from a dumb terminal
      too.
      
      * bzrlib/tests/__init__.py:
      (FakeStdin): New test class.
-------------- next part --------------
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2007-03-07 11:05:38 +0000
+++ b/bzrlib/tests/__init__.py	2007-03-08 22:08:08 +0000
@@ -51,6 +51,7 @@
     memorytree,
     osutils,
     progress,
+    ui,
     urlutils,
     )
 import bzrlib.branch
@@ -173,7 +174,7 @@
                 revision_id = ''
             bench_history.write("--date %s %s\n" % (time.time(), revision_id))
         self._bench_history = bench_history
-        self.ui = bzrlib.ui.ui_factory
+        self.ui = ui.ui_factory
         self.num_tests = num_tests
         self.error_count = 0
         self.failure_count = 0
@@ -571,6 +572,71 @@
             return setattr(self._cstring, name, val)
 
 
+class TestUIFactory(ui.CLIUIFactory):
+    """A UI Factory for testing.
+
+    Hide the progress bar but emit note()s.
+    Redirect stdin.
+    Allows get_password to be tested without real tty attached.
+    """
+
+    def __init__(self,
+                 stdout=None,
+                 stderr=None,
+                 stdin=None):
+        super(TestUIFactory, self).__init__()
+        if stdin is not None:
+            # We use a StringIOWrapper to be able to test various
+            # encodings, but the user is still responsible to
+            # encode the string and to set the encoding attribute
+            # of StringIOWrapper.
+            self.stdin = StringIOWrapper(stdin)
+        if stdout is None:
+            self.stdout = sys.stdout
+        else:
+            self.stdout = stdout
+        if stderr is None:
+            self.stderr = sys.stderr
+        else:
+            self.stderr = stderr
+
+    def clear(self):
+        """See progress.ProgressBar.clear()."""
+
+    def clear_term(self):
+        """See progress.ProgressBar.clear_term()."""
+
+    def clear_term(self):
+        """See progress.ProgressBar.clear_term()."""
+
+    def finished(self):
+        """See progress.ProgressBar.finished()."""
+
+    def note(self, fmt_string, *args, **kwargs):
+        """See progress.ProgressBar.note()."""
+        self.stdout.write((fmt_string + "\n") % args)
+
+    def progress_bar(self):
+        return self
+
+    def nested_progress_bar(self):
+        return self
+
+    def update(self, message, count=None, total=None):
+        """See progress.ProgressBar.update()."""
+
+    def get_non_echoed_password(self, prompt):
+        """Get password from stdin without trying to handle the echo mode"""
+        if prompt:
+            self.stdout.write(prompt)
+        password = self.stdin.readline()
+        if not password:
+            raise EOFError
+        if password[-1] == '\n':
+            password = password[:-1]
+        return password
+
+
 class TestCase(unittest.TestCase):
     """Base class for bzr unit tests.
     
@@ -620,10 +686,10 @@
     def _silenceUI(self):
         """Turn off UI for duration of test"""
         # by default the UI is off; tests can turn it on if they want it.
-        saved = bzrlib.ui.ui_factory
+        saved = ui.ui_factory
         def _restore():
-            bzrlib.ui.ui_factory = saved
-        bzrlib.ui.ui_factory = bzrlib.ui.SilentUIFactory()
+            ui.ui_factory = saved
+        ui.ui_factory = ui.SilentUIFactory()
         self.addCleanup(_restore)
 
     def _ndiff_strings(self, a, b):
@@ -994,8 +1060,6 @@
         """
         if encoding is None:
             encoding = bzrlib.user_encoding
-        if stdin is not None:
-            stdin = StringIO(stdin)
         stdout = StringIOWrapper()
         stderr = StringIOWrapper()
         stdout.encoding = encoding
@@ -1007,11 +1071,8 @@
         handler.setLevel(logging.INFO)
         logger = logging.getLogger('')
         logger.addHandler(handler)
-        old_ui_factory = bzrlib.ui.ui_factory
-        bzrlib.ui.ui_factory = bzrlib.tests.blackbox.TestUIFactory(
-            stdout=stdout,
-            stderr=stderr)
-        bzrlib.ui.ui_factory.stdin = stdin
+        old_ui_factory = ui.ui_factory
+        ui.ui_factory = TestUIFactory(stdin=stdin, stdout=stdout, stderr=stderr)
 
         cwd = None
         if working_dir is not None:
@@ -1022,14 +1083,15 @@
             saved_debug_flags = frozenset(debug.debug_flags)
             debug.debug_flags.clear()
             try:
-                result = self.apply_redirected(stdin, stdout, stderr,
+                result = self.apply_redirected(ui.ui_factory.stdin,
+                                               stdout, stderr,
                                                bzrlib.commands.run_bzr_catch_errors,
                                                argv)
             finally:
                 debug.debug_flags.update(saved_debug_flags)
         finally:
             logger.removeHandler(handler)
-            bzrlib.ui.ui_factory = old_ui_factory
+            ui.ui_factory = old_ui_factory
             if cwd is not None:
                 os.chdir(cwd)
 

=== modified file 'bzrlib/tests/blackbox/__init__.py'
--- a/bzrlib/tests/blackbox/__init__.py	2007-03-05 04:55:34 +0000
+++ b/bzrlib/tests/blackbox/__init__.py	2007-03-08 22:08:08 +0000
@@ -121,45 +121,3 @@
             return self.run_bzr_captured(args, retcode=retcode)[0]
         else:
             return self.run_bzr_captured(args, retcode=retcode)
-
-
-class TestUIFactory(ui.CLIUIFactory):
-    """A UI Factory for testing - hide the progress bar but emit note()s."""
-
-    def __init__(self,
-                 stdout=None,
-                 stderr=None):
-        super(TestUIFactory, self).__init__()
-        if stdout is None:
-            self.stdout = sys.stdout
-        else:
-            self.stdout = stdout
-        if stderr is None:
-            self.stderr = sys.stderr
-        else:
-            self.stderr = stderr
-
-    def clear(self):
-        """See progress.ProgressBar.clear()."""
-
-    def clear_term(self):
-        """See progress.ProgressBar.clear_term()."""
-
-    def clear_term(self):
-        """See progress.ProgressBar.clear_term()."""
-
-    def finished(self):
-        """See progress.ProgressBar.finished()."""
-
-    def note(self, fmt_string, *args, **kwargs):
-        """See progress.ProgressBar.note()."""
-        self.stdout.write((fmt_string + "\n") % args)
-
-    def progress_bar(self):
-        return self
-    
-    def nested_progress_bar(self):
-        return self
-
-    def update(self, message, count=None, total=None):
-        """See progress.ProgressBar.update()."""

=== modified file 'bzrlib/tests/blackbox/test_reconcile.py'
--- a/bzrlib/tests/blackbox/test_reconcile.py	2006-10-16 01:50:48 +0000
+++ b/bzrlib/tests/blackbox/test_reconcile.py	2007-02-21 14:46:06 +0000
@@ -22,7 +22,7 @@
 from bzrlib.inventory import Inventory
 import bzrlib.repository as repository
 from bzrlib.tests import TestCaseWithTransport
-from bzrlib.tests.blackbox import TestUIFactory
+from bzrlib.tests import TestUIFactory
 from bzrlib.transport import get_transport
 import bzrlib.ui as ui
 

=== modified file 'bzrlib/tests/blackbox/test_selftest.py'
--- a/bzrlib/tests/blackbox/test_selftest.py	2007-02-27 22:08:46 +0000
+++ b/bzrlib/tests/blackbox/test_selftest.py	2007-03-08 22:08:08 +0000
@@ -30,6 +30,7 @@
                           TestCaseInTempDir,
                           TestCaseWithMemoryTransport,
                           TestCaseWithTransport,
+                          TestUIFactory,
                           TestSkipped,
                           )
 from bzrlib.tests.blackbox import ExternalBase
@@ -211,10 +212,10 @@
         self.assertTrue(self.stdin is self.factory_stdin)
 
     def test_ui_factory(self):
-        # each invocation of self.run_bzr_captured should get its own UI
-        # factory, which is an instance of TestUIFactory, with stdout and
-        # stderr attached to the stdout and stderr of the invoked
-        # run_bzr_captured
+        # each invocation of self.run_bzr_captured should get its
+        # own UI factory, which is an instance of TestUIFactory,
+        # with stdin, stdout and stderr attached to the stdin,
+        # stdout and stderr of the invoked run_bzr_captured
         current_factory = bzrlib.ui.ui_factory
         self.run_bzr_captured(['foo'])
         self.failIf(current_factory is self.factory)
@@ -222,7 +223,7 @@
         self.assertNotEqual(sys.stderr, self.factory.stderr)
         self.assertEqual('foo\n', self.factory.stdout.getvalue())
         self.assertEqual('bar\n', self.factory.stderr.getvalue())
-        self.assertIsInstance(self.factory, bzrlib.tests.blackbox.TestUIFactory)
+        self.assertIsInstance(self.factory, TestUIFactory)
 
     def test_working_dir(self):
         self.build_tree(['one/', 'two/'])

=== modified file 'bzrlib/tests/blackbox/test_upgrade.py'
--- a/bzrlib/tests/blackbox/test_upgrade.py	2007-03-06 06:35:00 +0000
+++ b/bzrlib/tests/blackbox/test_upgrade.py	2007-03-08 22:08:08 +0000
@@ -24,7 +24,7 @@
 import bzrlib.bzrdir as bzrdir
 import bzrlib.repository as repository
 from bzrlib.tests import TestCaseWithTransport
-from bzrlib.tests.blackbox import TestUIFactory
+from bzrlib.tests import TestUIFactory
 from bzrlib.tests.test_sftp_transport import TestCaseWithSFTPServer
 from bzrlib.transport import get_transport
 import bzrlib.ui as ui

=== modified file 'bzrlib/tests/test_ui.py'
--- a/bzrlib/tests/test_ui.py	2006-11-02 10:20:19 +0000
+++ b/bzrlib/tests/test_ui.py	2007-02-21 14:46:06 +0000
@@ -24,8 +24,12 @@
 
 import bzrlib
 import bzrlib.errors as errors
-from bzrlib.progress import TTYProgressBar, ProgressBarStack
-from bzrlib.tests import TestCase
+from bzrlib.progress import DotsProgressBar, TTYProgressBar, ProgressBarStack
+from bzrlib.tests import (
+    TestUIFactory,
+    StringIOWrapper,
+    TestCase,
+    )
 from bzrlib.tests.test_progress import _TTYStringIO
 from bzrlib.ui import SilentUIFactory
 from bzrlib.ui.text import TextUIFactory
@@ -35,31 +39,53 @@
 
     def test_silent_factory(self):
         ui = SilentUIFactory()
-        pb = ui.nested_progress_bar()
-        try:
-            # TODO: Test that there is no output from SilentUIFactory
-    
-            self.assertEquals(ui.get_password(), None)
-            self.assertEquals(ui.get_password(u'Hello There \u1234 %(user)s',
-                                              user=u'some\u1234')
-                             , None)
-        finally:
-            pb.finished()
-
-    def test_text_factory(self):
-        ui = TextUIFactory()
-        pb = ui.nested_progress_bar()
-        pb.finished()
-        # TODO: Test the output from TextUIFactory, perhaps by overriding sys.stdout
-
-        # Unfortunately we can't actually test the ui.get_password() because 
-        # that would actually prompt the user for a password during the test suite
-        # This has been tested manually with both LANG=en_US.utf-8 and LANG=C
-        # print
-        # self.assertEquals(ui.get_password(u"%(user)s please type 'bogus'",
-        #                                   user=u'some\u1234')
-        #                  , 'bogus')
-
+        stdout = StringIO()
+        self.assertEqual(None,
+                         self.apply_redirected(None, stdout, stdout,
+                                               ui.get_password))
+        self.assertEqual('', stdout.getvalue())
+        self.assertEqual(None,
+                         self.apply_redirected(None, stdout, stdout,
+                                               ui.get_password,
+                                               u'Hello\u1234 %(user)s',
+                                               user=u'some\u1234'))
+        self.assertEqual('', stdout.getvalue())
+
+    def test_text_factory_ascii_password(self):
+        ui = TestUIFactory(stdin='secret\n', stdout=StringIOWrapper())
+        pb = ui.nested_progress_bar()
+        try:
+            self.assertEqual('secret',
+                             self.apply_redirected(ui.stdin, ui.stdout,
+                                                   ui.stdout,
+                                                   ui.get_password))
+            # ': ' is appended to prompt
+            self.assertEqual(': ', ui.stdout.getvalue())
+        finally:
+            pb.finished()
+
+    def test_text_factory_utf8_password(self):
+        """Test an utf8 password.
+
+        We can't predict what encoding users will have for stdin, so we force
+        it to utf8 to test that we transport the password correctly.
+        """
+        ui = TestUIFactory(stdin=u'baz\u1234'.encode('utf8'),
+                           stdout=StringIOWrapper())
+        ui.stdin.encoding = 'utf8'
+        ui.stdout.encoding = ui.stdin.encoding
+        pb = ui.nested_progress_bar()
+        try:
+            password = self.apply_redirected(ui.stdin, ui.stdout, ui.stdout,
+                                             ui.get_password,
+                                             u'Hello \u1234 %(user)s',
+                                             user=u'some\u1234')
+            # We use StringIO objects, we need to decode them
+            self.assertEqual(u'baz\u1234', password.decode('utf8'))
+            self.assertEqual(u'Hello \u1234 some\u1234: ',
+                             ui.stdout.getvalue().decode('utf8'))
+        finally:
+            pb.finished()
 
     def test_progress_note(self):
         stderr = StringIO()
@@ -136,10 +162,10 @@
     def test_text_factory_setting_progress_bar(self):
         # we should be able to choose the progress bar type used.
         factory = bzrlib.ui.text.TextUIFactory(
-            bar_type=bzrlib.progress.DotsProgressBar)
+            bar_type=DotsProgressBar)
         bar = factory.nested_progress_bar()
         bar.finished()
-        self.assertIsInstance(bar, bzrlib.progress.DotsProgressBar)
+        self.assertIsInstance(bar, DotsProgressBar)
 
     def test_cli_stdin_is_default_stdin(self):
         factory = bzrlib.ui.CLIUIFactory()
@@ -176,7 +202,7 @@
 
     def test_text_factory_prompts_and_clears(self):
         # a get_boolean call should clear the pb before prompting
-        factory = bzrlib.ui.text.TextUIFactory()
+        factory = bzrlib.ui.text.TextUIFactory(bar_type=DotsProgressBar)
         factory.stdout = _TTYStringIO()
         factory.stdin = StringIO("yada\ny\n")
         pb = self.apply_redirected(
@@ -189,17 +215,7 @@
             self.apply_redirected(
                 None, factory.stdout, factory.stdout, factory.get_boolean, "what do you want")
             )
-        # FIXME: This assumes the factory's going to produce a spinner-style
-        # progress bar, but it won't if this is run from a dumb terminal (e.g.
-        # from inside gvim.) -- mbp 20061014
-        #
-        # use a regular expression so that we don't depend on the particular
-        # screen width - could also set and restore $COLUMN if that has
-        # priority on all platforms, but it doesn't at present.
         output = factory.stdout.getvalue()
-        if not re.match(
-            "\r/ \\[    *\\] foo 0/1"
-            "\r   *" 
-            "\rwhat do you want\\? \\[y/n\\]:what do you want\\? \\[y/n\\]:", 
-            output):
-            self.fail("didn't match factory output %r, %r" % (factory, output))
+        self.assertEqual("foo: .\n"
+                         "what do you want? [y/n]: what do you want? [y/n]: ",
+                         factory.stdout.getvalue())

=== modified file 'bzrlib/ui/__init__.py'
--- a/bzrlib/ui/__init__.py	2006-10-31 01:30:31 +0000
+++ b/bzrlib/ui/__init__.py	2007-02-21 14:46:06 +0000
@@ -32,6 +32,8 @@
 
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
+import getpass
+
 from bzrlib import (
     progress,
     )
@@ -63,11 +65,14 @@
         :param kwargs: Arguments which will be expanded into the prompt.
                        This lets front ends display different things if
                        they so choose.
-        :return: The password string, return None if the user 
-                 canceled the request.
+
+        :return: The password string, return None if the user canceled the
+                 request. Note that we do not touch the encoding, users may
+                 have whatever they see fit and the password should be
+                 transported as is.
         """
         raise NotImplementedError(self.get_password)
-        
+
     def nested_progress_bar(self):
         """Return a nested progress bar.
 
@@ -104,13 +109,32 @@
         self.clear_term()
         # FIXME: make a regexp and handle case variations as well.
         while True:
-            self.prompt(prompt)
+            self.prompt(prompt + "? [y/n]: ")
             line = self.stdin.readline()
             if line in ('y\n', 'yes\n'):
                 return True
             if line in ('n\n', 'no\n'):
                 return False
 
+    def get_non_echoed_password(self, prompt):
+        return getpass.getpass(prompt)
+
+    def get_password(self, prompt='', **kwargs):
+        """Prompt the user for a password.
+
+        :param prompt: The prompt to present the user
+        :param kwargs: Arguments which will be expanded into the prompt.
+                       This lets front ends display different things if
+                       they so choose.
+        :return: The password string, return None if the user 
+                 canceled the request.
+        """
+        prompt += ': '
+        prompt = (prompt % kwargs).encode(sys.stdout.encoding, 'replace')
+        # There's currently no way to say 'i decline to enter a password'
+        # as opposed to 'my password is empty' -- does it matter?
+        return self.get_non_echoed_password(prompt)
+
     def prompt(self, prompt):
         """Emit prompt on the CLI."""
 

=== modified file 'bzrlib/ui/text.py'
--- a/bzrlib/ui/text.py	2006-10-31 01:30:31 +0000
+++ b/bzrlib/ui/text.py	2007-02-21 14:46:06 +0000
@@ -27,6 +27,7 @@
 
 from bzrlib import (
     progress,
+    osutils,
     )
 """)
 
@@ -63,7 +64,7 @@
 
     def prompt(self, prompt):
         """Emit prompt on the CLI."""
-        self.stdout.write(prompt + "? [y/n]:")
+        self.stdout.write(prompt)
         
     @deprecated_method(zero_eight)
     def progress_bar(self):
@@ -72,22 +73,6 @@
         # bar depending on what we think of the terminal
         return progress.ProgressBar()
 
-    def get_password(self, prompt='', **kwargs):
-        """Prompt the user for a password.
-
-        :param prompt: The prompt to present the user
-        :param kwargs: Arguments which will be expanded into the prompt.
-                       This lets front ends display different things if
-                       they so choose.
-        :return: The password string, return None if the user 
-                 canceled the request.
-        """
-        prompt = (prompt % kwargs).encode(sys.stdout.encoding, 'replace')
-        prompt += ': '
-        # There's currently no way to say 'i decline to enter a password'
-        # as opposed to 'my password is empty' -- does it matter?
-        return getpass.getpass(prompt)
-
     def nested_progress_bar(self):
         """Return a nested progress bar.
         



More information about the bazaar-commits mailing list