Fix: trash-empty crash on invalid cmdline options.

HISTORY.txt

@@ -1,3 +1,7 @@
+after the 0.12.6:
+ - fixed trash-empty crashed with GetoptError in short_has_arg(): option -2 
+   not recognized (see 
+   https://bugs.launchpad.net/ubuntu/+source/trash-cli/+bug/1015877 )
 0.12.6:
  - add Donate button on README
 

TODO.txt

@@ -31,6 +31,7 @@ To be fixed:
  - get rid of TimeUtils.parse_iso8601 if not used
  - rename remove_file in to remove_existing_file
  - refactor TrashPutCmd for simplicity
+
 Test to be ported to nosetests:
  - trash-put:
     - trash in home trashcan
@@ -44,3 +45,10 @@ Test to be ported to nosetests:
  - should refuse to create the $topdir/.Trash/$uid directory if the 
    $topdir/.Trash is not sticky
 
+Bug reported on external trackers:
+ x trash-empty crashed with GetoptError in short_has_arg(): option -2 not 
+   recognized, 
+   url: https://bugs.launchpad.net/ubuntu/+source/trash-cli/+bug/1015877
+   status: solved on Thu Jun 21 12:50:03 CEST 2012
+
+

integration_tests/test_trash_empty.py

@@ -1,6 +1,8 @@
 # Copyright (C) 2011 Andrea Francia Trivolzio(PV) Italy
 
-from nose.tools import assert_equals, assert_items_equal, istest
+from nose.tools import (assert_equals,
+                        assert_items_equal,
+                        istest)
 from trashcli.trash import EmptyCmd
 
 from StringIO import StringIO
@@ -233,3 +235,36 @@ def test_it_print_version(self):
             trash-empty 1.2.3
             """))
 
+class describe_trash_empty_command_line__on_invalid_options():
+    def setUp(self):
+        self.err, self.out = StringIO(), StringIO()
+        self.cmd = EmptyCmd(
+                       err = self.err,
+                       out = self.out,
+                       environ = {})
+
+    def it_should_fail(self):
+
+        self.exit_code = self.cmd.run('trash-empty', '-2')
+
+        exit_code_for_command_line_usage = 64
+        assert_equals(exit_code_for_command_line_usage, self.exit_code)
+
+    def it_should_complain_to_the_standard_error(self):
+
+        self.exit_code = self.cmd.run('trash-empty', '-2')
+
+        assert_equals(self.err.getvalue(), dedent("""\
+                trash-empty: invalid option -- '2'
+                """))
+
+    def test_with_a_different_option(self):
+
+        self.cmd.run('trash-empty', '-3')
+
+        assert_equals(self.err.getvalue(), dedent("""\
+                trash-empty: invalid option -- '3'
+                """))
+
+
+

trashcli/cmds.py

@@ -22,7 +22,7 @@ def restore():
 
 def empty():
     from trashcli.trash import EmptyCmd
-    EmptyCmd(
+    return EmptyCmd(
         out=sys.stdout,
         err=sys.stderr,
         environ=os.environ,

trashcli/trash.py

@@ -876,20 +876,31 @@ def __init__(self):
         self.short_options = ''
         self.long_options = []
         self.actions = dict()
+        self._on_invalid_option = do_nothing
+
     def __call__(self, argv):
         program_name = argv[0]
-        from getopt import getopt
-        options, arguments = getopt(argv[1:],
-                                    self.short_options,
-                                    self.long_options)
-
-        for option, value in options:
-            if option in self.actions:
-                self.actions[option](program_name)
-                return
-        for argument in arguments:
-            self.argument_action(argument)
-        self.default_action()
+        from getopt import getopt, GetoptError
+
+        try:
+            options, arguments = getopt(argv[1:],
+                                        self.short_options,
+                                        self.long_options)
+        except GetoptError, e:
+            invalid_option = e.opt
+            self._on_invalid_option(program_name, invalid_option)
+        else:
+            for option, value in options:
+                if option in self.actions:
+                    self.actions[option](program_name)
+                    return
+            for argument in arguments:
+                self.argument_action(argument)
+            self.default_action()
+
+    def on_invalid_option(self, action):
+        self._on_invalid_option = action
+
     def on_help(self, action):
         self.add_option('help', action, 'h')
 
@@ -911,6 +922,10 @@ def on_argument(self, argument_action):
     def as_default(self, default_action):
         self.default_action = default_action
 
+# Error codes (from os on *nix, hard coded for Windows):
+EX_USAGE = getattr(os, 'EX_USAGE', 64)
+EX_OK    = getattr(os, 'EX_OK'   ,  0)
+
 class EmptyCmd():
     def __init__(self, out, err, environ,
                  now           = datetime.now,
@@ -940,12 +955,24 @@ def __init__(self):
 
     def run(self, *argv):
         self._maybe_delete = self._delete_both
+        self.exit_code     = EX_OK
+
         parse = Parser()
         parse.on_help(PrintHelp(self.description, self.println))
         parse.on_version(PrintVersion(self.println, self.version))
         parse.on_argument(self.set_deletion_date_criteria)
         parse.as_default(self._empty_all_trashdirs)
+        parse.on_invalid_option(self.report_invalid_option_usage)
+
         parse(argv)
+
+        return self.exit_code
+
+    def report_invalid_option_usage(self, program_name, option):
+        self.err.write(
+            "{program_name}: invalid option -- '{option}'\n".format(**locals()))
+        self.exit_code |= EX_USAGE
+
     def set_deletion_date_criteria(self, arg):
         self.max_age_in_days = int(arg)
         self._maybe_delete = self._delete_according_date

unit_tests/test_parser.py

@@ -13,3 +13,13 @@ def it_calls_the_actions_passing_the_program_name(self):
         parser(['trash-list', '--raw'])
 
         on_raw.assert_called_with('trash-list')
+
+    @istest
+    def how_getopt_works_with_an_invalid_option(self):
+        invalid_option_callback = MagicMock()
+        parser = Parser()
+        parser.on_invalid_option(invalid_option_callback)
+
+        parser(['command-name', '-x'])
+
+        invalid_option_callback.assert_called_with('command-name', 'x')