Browse Source

Raise BadCommand when the command is empty. Fixes #171

Mike Naberezny 12 years ago
parent
commit
5aa3e31ed7
2 changed files with 56 additions and 41 deletions
  1. 24 21
      supervisor/process.py
  2. 32 20
      supervisor/tests/test_process.py

+ 24 - 21
supervisor/process.py

@@ -49,7 +49,7 @@ class Subprocess:
     exitstatus = None # status attached to dead process by finsh()
     spawnerr = None # error message attached by spawn() if any
     group = None # ProcessGroup instance if process is in the group
-    
+
     def __init__(self, config):
         """Constructor.
 
@@ -80,7 +80,7 @@ class Subprocess:
                 dispatcher.handle_read_event()
             if dispatcher.writable():
                 dispatcher.handle_write_event()
-                
+
     def write(self, chars):
         if not self.pid or self.killing:
             raise OSError(errno.EPIPE, "Process already closed")
@@ -92,7 +92,7 @@ class Subprocess:
         dispatcher = self.dispatchers[stdin_fd]
         if dispatcher.closed:
             raise OSError(errno.EPIPE, "Process' stdin channel is closed")
-            
+
         dispatcher.input_buffer += chars
         dispatcher.flush() # this must raise EPIPE if the pipe is closed
 
@@ -106,7 +106,10 @@ class Subprocess:
             raise BadCommand("can't parse command %r: %s" % \
                 (self.config.command, str(e)))
 
-        program = commandargs[0]
+        if commandargs:
+            program = commandargs[0]
+        else:
+            raise BadCommand("command is empty")
 
         if "/" in program:
             filename = program
@@ -114,7 +117,7 @@ class Subprocess:
                 st = self.config.options.stat(filename)
             except OSError:
                 st = None
-            
+
         else:
             path = self.config.options.get_path()
             found = None
@@ -196,7 +199,7 @@ class Subprocess:
         self.exitstatus = None
         self.system_stop = 0
         self.administrative_stop = 0
-        
+
         self.laststart = time.time()
 
         self._assertInState(ProcessStates.EXITED, ProcessStates.FATAL,
@@ -225,7 +228,7 @@ class Subprocess:
             self._assertInState(ProcessStates.STARTING)
             self.change_state(ProcessStates.BACKOFF)
             return
-        
+
         try:
             pid = options.fork()
         except OSError, why:
@@ -246,7 +249,7 @@ class Subprocess:
 
         if pid != 0:
             return self._spawn_as_parent(pid)
-        
+
         else:
             return self._spawn_as_child(filename, argv)
 
@@ -270,7 +273,7 @@ class Subprocess:
         else:
             options.dup2(self.pipes['child_stderr'], 2)
         for i in range(3, options.minfds):
-            options.close_fd(i)        
+            options.close_fd(i)
 
     def _spawn_as_child(self, filename, argv):
         options = self.config.options
@@ -401,7 +404,7 @@ class Subprocess:
             self.killing = 0
             self.delay = 0
             return msg
-            
+
         return None
 
     def finish(self, pid, sts):
@@ -590,14 +593,14 @@ class FastCGISubprocess(Subprocess):
             #Remove object reference to decrement the reference count on error
             self.fcgi_sock = None
         return pid
-        
+
     def after_finish(self):
         """
         Releases reference to FastCGI socket when process is reaped
         """
         #Remove object reference to decrement the reference count
         self.fcgi_sock = None
-        
+
     def finish(self, pid, sts):
         """
         Overrides Subprocess.finish() so we can hook in after it happens
@@ -612,7 +615,7 @@ class FastCGISubprocess(Subprocess):
         The FastCGI socket needs to be set to file descriptor 0 in the child
         """
         sock_fd = self.fcgi_sock.fileno()
-        
+
         options = self.config.options
         options.dup2(sock_fd, 0)
         options.dup2(self.pipes['child_stdout'], 1)
@@ -622,14 +625,14 @@ class FastCGISubprocess(Subprocess):
             options.dup2(self.pipes['child_stderr'], 2)
         for i in range(3, options.minfds):
             options.close_fd(i)
-                
+
 class ProcessGroupBase:
     def __init__(self, config):
         self.config = config
         self.processes = {}
         for pconfig in self.config.process_configs:
             self.processes[pconfig.name] = pconfig.make_process(self)
-        
+
 
     def __cmp__(self, other):
         return cmp(self.config.priority, other.config.priority)
@@ -678,13 +681,13 @@ class ProcessGroup(ProcessGroupBase):
     def transition(self):
         for proc in self.processes.values():
             proc.transition()
-            
+
 class FastCGIProcessGroup(ProcessGroup):
 
     def __init__(self, config, **kwargs):
         ProcessGroup.__init__(self, config)
         sockManagerKlass = kwargs.get('socketManager', SocketManager)
-        self.socket_manager = sockManagerKlass(config.socket_config, 
+        self.socket_manager = sockManagerKlass(config.socket_config,
                                                logger=config.options.logger)
         #It's not required to call get_socket() here but we want
         #to fail early during start up if there is a config error
@@ -768,7 +771,7 @@ class EventListenerPool(ProcessGroupBase):
 
     def _dispatchEvent(self, event):
         pool_serial = event.pool_serials[self.config.name]
-            
+
         for process in self.processes.values():
             if process.state != ProcessStates.RUNNING:
                 continue
@@ -784,7 +787,7 @@ class EventListenerPool(ProcessGroupBase):
                     if why[0] != errno.EPIPE:
                         raise
                     continue
-                
+
                 process.listener_state = EventListenerStates.BUSY
                 process.event = event
                 self.config.options.logger.debug(
@@ -823,5 +826,5 @@ def new_serial(inst):
     inst.serial += 1
     return inst.serial
 
-            
-    
+
+

+ 32 - 20
supervisor/tests/test_process.py

@@ -76,7 +76,7 @@ class SubprocessTests(unittest.TestCase):
         instance.reopenlogs()
         self.assertEqual(instance.dispatchers[0].logs_reopened, True)
         self.assertEqual(instance.dispatchers[1].logs_reopened, False)
-        
+
     def test_removelogs(self):
         options = DummyOptions()
         config = DummyPConfig(options, 'test', '/test')
@@ -99,12 +99,24 @@ class SubprocessTests(unittest.TestCase):
         self.assertTrue(instance.dispatchers[0].read_event_handled)
         self.assertTrue(instance.dispatchers[1].write_event_handled)
 
-    def test_get_execv_args_bad_command(self):
+    def test_get_execv_args_bad_command_extraquote(self):
         options = DummyOptions()
         config = DummyPConfig(options, 'extraquote', 'extraquote"')
         instance = self._makeOne(config)
         self.assertRaises(BadCommand, instance.get_execv_args)
-        
+
+    def test_get_execv_args_bad_command_empty(self):
+        options = DummyOptions()
+        config = DummyPConfig(options, 'empty', '')
+        instance = self._makeOne(config)
+        self.assertRaises(BadCommand, instance.get_execv_args)
+
+    def test_get_execv_args_bad_command_whitespaceonly(self):
+        options = DummyOptions()
+        config = DummyPConfig(options, 'whitespaceonly', ' \t ')
+        instance = self._makeOne(config)
+        self.assertRaises(BadCommand, instance.get_execv_args)
+
     def test_get_execv_args_abs_missing(self):
         options = DummyOptions()
         config = DummyPConfig(options, 'notthere', '/notthere')
@@ -1228,14 +1240,14 @@ class FastCGISubprocessTests(unittest.TestCase):
         instance = self._makeOne(config)
         instance.group = DummyProcessGroup(DummyPGroupConfig(options))
         self.assertRaises(NotImplementedError, instance.spawn)
-        
+
     def test_prepare_child_fds(self):
         options = DummyOptions()
         options.forkpid = 0
         config = DummyPConfig(options, 'good', '/good/filename', uid=1)
         instance = self._makeOne(config)
         sock_config = DummySocketConfig(7)
-        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None, 
+        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None,
                                        sock_config)
         instance.group = DummyFCGIProcessGroup(gconfig)
         result = instance.spawn()
@@ -1253,7 +1265,7 @@ class FastCGISubprocessTests(unittest.TestCase):
         config.redirect_stderr = True
         instance = self._makeOne(config)
         sock_config = DummySocketConfig(13)
-        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None, 
+        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None,
                                        sock_config)
         instance.group = DummyFCGIProcessGroup(gconfig)
         result = instance.spawn()
@@ -1261,19 +1273,19 @@ class FastCGISubprocessTests(unittest.TestCase):
         self.assertEqual(len(options.duped), 2)
         self.assertEqual(options.duped[13], 0)
         self.assertEqual(len(options.fds_closed), options.minfds - 3)
-        
+
     def test_before_spawn_gets_socket_ref(self):
         options = DummyOptions()
         config = DummyPConfig(options, 'good', '/good/filename', uid=1)
         instance = self._makeOne(config)
         sock_config = DummySocketConfig(7)
-        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None, 
+        gconfig = DummyFCGIGroupConfig(options, 'whatever', 999, None,
                                        sock_config)
         instance.group = DummyFCGIProcessGroup(gconfig)
         self.assertTrue(instance.fcgi_sock is None)
         instance.before_spawn()
         self.assertFalse(instance.fcgi_sock is None)
-        
+
     def test_after_finish_removes_socket_ref(self):
         options = DummyOptions()
         config = DummyPConfig(options, 'good', '/good/filename', uid=1)
@@ -1281,7 +1293,7 @@ class FastCGISubprocessTests(unittest.TestCase):
         instance.fcgi_sock = 'hello'
         instance.after_finish()
         self.assertTrue(instance.fcgi_sock is None)
-    
+
     #Patch Subprocess.finish() method for this test to verify override
     @patch.object(Subprocess, 'finish', Mock(return_value=sentinel.finish_result))
     def test_finish_override(self):
@@ -1303,7 +1315,7 @@ class FastCGISubprocessTests(unittest.TestCase):
                             'Subprocess.finish() pid arg was not passed')
         self.assertEqual(sentinel.sts, sts_arg,
                             'Subprocess.finish() sts arg was not passed')
-                            
+
     #Patch Subprocess.spawn() method for this test to verify override
     @patch.object(Subprocess, 'spawn', Mock(return_value=sentinel.ppid))
     def test_spawn_override_success(self):
@@ -1334,7 +1346,7 @@ class FastCGISubprocessTests(unittest.TestCase):
         self.assertEqual(1, instance.before_spawn.call_count,
                             'FastCGISubprocess.before_spawn() not called once')
         self.assertEqual(None, instance.fcgi_sock,
-                'FastCGISubprocess.spawn() did not remove sock ref on error')    
+                'FastCGISubprocess.spawn() did not remove sock ref on error')
 
 class ProcessGroupBaseTests(unittest.TestCase):
     def _getTargetClass(self):
@@ -1399,7 +1411,7 @@ class ProcessGroupBaseTests(unittest.TestCase):
         group.processes = { 'process1': process1, 'process2': process2 }
         result= group.get_dispatchers()
         self.assertEqual(result, {4:None, 5:None})
-        
+
     def test_reopenlogs(self):
         options = DummyOptions()
         from supervisor.states import ProcessStates
@@ -1426,7 +1438,7 @@ class ProcessGroupBaseTests(unittest.TestCase):
         options = DummyOptions()
         gconfig1 = DummyPGroupConfig(options)
         group1 = self._makeOne(gconfig1)
-        
+
         gconfig2 = DummyPGroupConfig(options)
         group2 = self._makeOne(gconfig2)
 
@@ -1462,7 +1474,7 @@ class ProcessGroupTests(ProcessGroupBaseTests):
         group.processes = {'process1': process1}
         group.transition()
         self.assertEqual(process1.transitioned, True)
-                
+
 class EventListenerPoolTests(ProcessGroupBaseTests):
     def setUp(self):
         from supervisor.events import clear
@@ -1471,7 +1483,7 @@ class EventListenerPoolTests(ProcessGroupBaseTests):
     def tearDown(self):
         from supervisor.events import clear
         clear()
-        
+
     def _getTargetClass(self):
         from supervisor.process import EventListenerPool
         return EventListenerPool
@@ -1485,9 +1497,9 @@ class EventListenerPoolTests(ProcessGroupBaseTests):
         pool = self._makeOne(gconfig)
         from supervisor import events
         self.assertEqual(len(events.callbacks), 2)
-        self.assertEqual(events.callbacks[0], 
+        self.assertEqual(events.callbacks[0],
             (EventType, pool._acceptEvent))
-        self.assertEqual(events.callbacks[1], 
+        self.assertEqual(events.callbacks[1],
             (events.EventRejectedEvent, pool.handle_rejected))
         self.assertEqual(pool.serial, -1)
 
@@ -1529,7 +1541,7 @@ class EventListenerPoolTests(ProcessGroupBaseTests):
         dummyevent.serial = 1
         pool.handle_rejected(dummyevent)
         self.assertEqual(pool.event_buffer, [dummyevent.event, None, None])
-        
+
     def test_handle_rejected_event_buffer_overflowed(self):
         options = DummyOptions()
         gconfig = DummyPGroupConfig(options)
@@ -1618,7 +1630,7 @@ class EventListenerPoolTests(ProcessGroupBaseTests):
         self.assertEqual(process1.transitioned, True)
         self.assertEqual(pool.event_buffer, [event])
         data = pool.config.options.logger.data
-    
+
     def test_transition_event_proc_not_running(self):
         options = DummyOptions()
         from supervisor.states import ProcessStates