diff --git a/homeassistant/components/command_line/__init__.py b/homeassistant/components/command_line/__init__.py index fe0640d3efa..92f219a13ea 100644 --- a/homeassistant/components/command_line/__init__.py +++ b/homeassistant/components/command_line/__init__.py @@ -1 +1,42 @@ """The command_line component.""" + +import logging +import subprocess + +_LOGGER = logging.getLogger(__name__) + + +def call_shell_with_timeout(command, timeout): + """Run a shell command with a timeout.""" + try: + _LOGGER.debug("Running command: %s", command) + subprocess.check_output( + command, shell=True, timeout=timeout # nosec # shell by design + ) + return 0 + except subprocess.CalledProcessError as proc_exception: + _LOGGER.error("Command failed: %s", command) + return proc_exception.returncode + except subprocess.TimeoutExpired: + _LOGGER.error("Timeout for command: %s", command) + return -1 + except subprocess.SubprocessError: + _LOGGER.error("Error trying to exec command: %s", command) + return -1 + + +def check_output_or_log(command, timeout): + """Run a shell command with a timeout and return the output.""" + try: + return_value = subprocess.check_output( + command, shell=True, timeout=timeout # nosec # shell by design + ) + return return_value.strip().decode("utf-8") + except subprocess.CalledProcessError: + _LOGGER.error("Command failed: %s", command) + except subprocess.TimeoutExpired: + _LOGGER.error("Timeout for command: %s", command) + except subprocess.SubprocessError: + _LOGGER.error("Error trying to exec command: %s", command) + + return None diff --git a/homeassistant/components/command_line/binary_sensor.py b/homeassistant/components/command_line/binary_sensor.py index dc62d8daa9d..86916e86a26 100644 --- a/homeassistant/components/command_line/binary_sensor.py +++ b/homeassistant/components/command_line/binary_sensor.py @@ -19,6 +19,7 @@ from homeassistant.const import ( ) import homeassistant.helpers.config_validation as cv +from .const import CONF_COMMAND_TIMEOUT, DEFAULT_TIMEOUT from .sensor import CommandSensorData _LOGGER = logging.getLogger(__name__) @@ -29,8 +30,6 @@ DEFAULT_PAYLOAD_OFF = "OFF" SCAN_INTERVAL = timedelta(seconds=60) -CONF_COMMAND_TIMEOUT = "command_timeout" -DEFAULT_TIMEOUT = 15 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { diff --git a/homeassistant/components/command_line/const.py b/homeassistant/components/command_line/const.py new file mode 100644 index 00000000000..8c5bc0b2967 --- /dev/null +++ b/homeassistant/components/command_line/const.py @@ -0,0 +1,4 @@ +"""Allows to configure custom shell commands to turn a value for a sensor.""" + +CONF_COMMAND_TIMEOUT = "command_timeout" +DEFAULT_TIMEOUT = 15 diff --git a/homeassistant/components/command_line/cover.py b/homeassistant/components/command_line/cover.py index 6f2a038d051..1fdcdf3b3e7 100644 --- a/homeassistant/components/command_line/cover.py +++ b/homeassistant/components/command_line/cover.py @@ -1,6 +1,5 @@ """Support for command line covers.""" import logging -import subprocess import voluptuous as vol @@ -16,6 +15,9 @@ from homeassistant.const import ( ) import homeassistant.helpers.config_validation as cv +from . import call_shell_with_timeout, check_output_or_log +from .const import CONF_COMMAND_TIMEOUT, DEFAULT_TIMEOUT + _LOGGER = logging.getLogger(__name__) COVER_SCHEMA = vol.Schema( @@ -26,6 +28,7 @@ COVER_SCHEMA = vol.Schema( vol.Optional(CONF_COMMAND_STOP, default="true"): cv.string, vol.Optional(CONF_FRIENDLY_NAME): cv.string, vol.Optional(CONF_VALUE_TEMPLATE): cv.template, + vol.Optional(CONF_COMMAND_TIMEOUT, default=DEFAULT_TIMEOUT): cv.positive_int, } ) @@ -48,11 +51,12 @@ def setup_platform(hass, config, add_entities, discovery_info=None): CommandCover( hass, device_config.get(CONF_FRIENDLY_NAME, device_name), - device_config.get(CONF_COMMAND_OPEN), - device_config.get(CONF_COMMAND_CLOSE), - device_config.get(CONF_COMMAND_STOP), + device_config[CONF_COMMAND_OPEN], + device_config[CONF_COMMAND_CLOSE], + device_config[CONF_COMMAND_STOP], device_config.get(CONF_COMMAND_STATE), value_template, + device_config[CONF_COMMAND_TIMEOUT], ) ) @@ -75,6 +79,7 @@ class CommandCover(CoverEntity): command_stop, command_state, value_template, + timeout, ): """Initialize the cover.""" self._hass = hass @@ -85,31 +90,23 @@ class CommandCover(CoverEntity): self._command_stop = command_stop self._command_state = command_state self._value_template = value_template + self._timeout = timeout - @staticmethod - def _move_cover(command): + def _move_cover(self, command): """Execute the actual commands.""" _LOGGER.info("Running command: %s", command) - success = subprocess.call(command, shell=True) == 0 # nosec # shell by design + success = call_shell_with_timeout(command, self._timeout) == 0 if not success: _LOGGER.error("Command failed: %s", command) return success - @staticmethod - def _query_state_value(command): + def _query_state_value(self, command): """Execute state command for return value.""" - _LOGGER.info("Running state command: %s", command) - - try: - return_value = subprocess.check_output( - command, shell=True # nosec # shell by design - ) - return return_value.strip().decode("utf-8") - except subprocess.CalledProcessError: - _LOGGER.error("Command failed: %s", command) + _LOGGER.info("Running state value command: %s", command) + return check_output_or_log(command, self._timeout) @property def should_poll(self): diff --git a/homeassistant/components/command_line/notify.py b/homeassistant/components/command_line/notify.py index 50b0bec74ee..948bda7e45a 100644 --- a/homeassistant/components/command_line/notify.py +++ b/homeassistant/components/command_line/notify.py @@ -8,26 +8,34 @@ from homeassistant.components.notify import PLATFORM_SCHEMA, BaseNotificationSer from homeassistant.const import CONF_COMMAND, CONF_NAME import homeassistant.helpers.config_validation as cv +from .const import CONF_COMMAND_TIMEOUT, DEFAULT_TIMEOUT + _LOGGER = logging.getLogger(__name__) PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( - {vol.Required(CONF_COMMAND): cv.string, vol.Optional(CONF_NAME): cv.string} + { + vol.Required(CONF_COMMAND): cv.string, + vol.Optional(CONF_NAME): cv.string, + vol.Optional(CONF_COMMAND_TIMEOUT, default=DEFAULT_TIMEOUT): cv.positive_int, + } ) def get_service(hass, config, discovery_info=None): """Get the Command Line notification service.""" command = config[CONF_COMMAND] + timeout = config[CONF_COMMAND_TIMEOUT] - return CommandLineNotificationService(command) + return CommandLineNotificationService(command, timeout) class CommandLineNotificationService(BaseNotificationService): """Implement the notification service for the Command Line service.""" - def __init__(self, command): + def __init__(self, command, timeout): """Initialize the service.""" self.command = command + self._timeout = timeout def send_message(self, message="", **kwargs): """Send a message to a command line.""" @@ -38,8 +46,10 @@ class CommandLineNotificationService(BaseNotificationService): stdin=subprocess.PIPE, shell=True, # nosec # shell by design ) - proc.communicate(input=message) + proc.communicate(input=message, timeout=self._timeout) if proc.returncode != 0: _LOGGER.error("Command failed: %s", self.command) + except subprocess.TimeoutExpired: + _LOGGER.error("Timeout for command: %s", self.command) except subprocess.SubprocessError: - _LOGGER.error("Error trying to exec Command: %s", self.command) + _LOGGER.error("Error trying to exec command: %s", self.command) diff --git a/homeassistant/components/command_line/sensor.py b/homeassistant/components/command_line/sensor.py index feb63e18443..778806099aa 100644 --- a/homeassistant/components/command_line/sensor.py +++ b/homeassistant/components/command_line/sensor.py @@ -3,7 +3,6 @@ from collections.abc import Mapping from datetime import timedelta import json import logging -import subprocess import voluptuous as vol @@ -20,13 +19,14 @@ from homeassistant.helpers import template import homeassistant.helpers.config_validation as cv from homeassistant.helpers.entity import Entity +from . import check_output_or_log +from .const import CONF_COMMAND_TIMEOUT, DEFAULT_TIMEOUT + _LOGGER = logging.getLogger(__name__) -CONF_COMMAND_TIMEOUT = "command_timeout" CONF_JSON_ATTRIBUTES = "json_attributes" DEFAULT_NAME = "Command Sensor" -DEFAULT_TIMEOUT = 15 SCAN_INTERVAL = timedelta(seconds=60) @@ -171,13 +171,6 @@ class CommandSensorData: else: # Template used. Construct the string used in the shell command = f"{prog} {rendered_args}" - try: - _LOGGER.debug("Running command: %s", command) - return_value = subprocess.check_output( - command, shell=True, timeout=self.timeout # nosec # shell by design - ) - self.value = return_value.strip().decode("utf-8") - except subprocess.CalledProcessError: - _LOGGER.error("Command failed: %s", command) - except subprocess.TimeoutExpired: - _LOGGER.error("Timeout for command: %s", command) + + _LOGGER.debug("Running command: %s", command) + self.value = check_output_or_log(command, self.timeout) diff --git a/homeassistant/components/command_line/switch.py b/homeassistant/components/command_line/switch.py index 7f62970b639..50cda31a537 100644 --- a/homeassistant/components/command_line/switch.py +++ b/homeassistant/components/command_line/switch.py @@ -1,6 +1,5 @@ """Support for custom shell commands to turn a switch on/off.""" import logging -import subprocess import voluptuous as vol @@ -19,6 +18,9 @@ from homeassistant.const import ( ) import homeassistant.helpers.config_validation as cv +from . import call_shell_with_timeout, check_output_or_log +from .const import CONF_COMMAND_TIMEOUT, DEFAULT_TIMEOUT + _LOGGER = logging.getLogger(__name__) SWITCH_SCHEMA = vol.Schema( @@ -28,6 +30,7 @@ SWITCH_SCHEMA = vol.Schema( vol.Optional(CONF_COMMAND_STATE): cv.string, vol.Optional(CONF_FRIENDLY_NAME): cv.string, vol.Optional(CONF_VALUE_TEMPLATE): cv.template, + vol.Optional(CONF_COMMAND_TIMEOUT, default=DEFAULT_TIMEOUT): cv.positive_int, } ) @@ -52,10 +55,11 @@ def setup_platform(hass, config, add_entities, discovery_info=None): hass, object_id, device_config.get(CONF_FRIENDLY_NAME, object_id), - device_config.get(CONF_COMMAND_ON), - device_config.get(CONF_COMMAND_OFF), + device_config[CONF_COMMAND_ON], + device_config[CONF_COMMAND_OFF], device_config.get(CONF_COMMAND_STATE), value_template, + device_config[CONF_COMMAND_TIMEOUT], ) ) @@ -78,6 +82,7 @@ class CommandSwitch(SwitchEntity): command_off, command_state, value_template, + timeout, ): """Initialize the switch.""" self._hass = hass @@ -88,37 +93,28 @@ class CommandSwitch(SwitchEntity): self._command_off = command_off self._command_state = command_state self._value_template = value_template + self._timeout = timeout - @staticmethod - def _switch(command): + def _switch(self, command): """Execute the actual commands.""" _LOGGER.info("Running command: %s", command) - success = subprocess.call(command, shell=True) == 0 # nosec # shell by design + success = call_shell_with_timeout(command, self._timeout) == 0 if not success: _LOGGER.error("Command failed: %s", command) return success - @staticmethod - def _query_state_value(command): + def _query_state_value(self, command): """Execute state command for return value.""" - _LOGGER.info("Running state command: %s", command) + _LOGGER.info("Running state value command: %s", command) + return check_output_or_log(command, self._timeout) - try: - return_value = subprocess.check_output( - command, shell=True # nosec # shell by design - ) - return return_value.strip().decode("utf-8") - except subprocess.CalledProcessError: - _LOGGER.error("Command failed: %s", command) - - @staticmethod - def _query_state_code(command): + def _query_state_code(self, command): """Execute state command for return code.""" - _LOGGER.info("Running state command: %s", command) - return subprocess.call(command, shell=True) == 0 # nosec # shell by design + _LOGGER.info("Running state code command: %s", command) + return call_shell_with_timeout(command, self._timeout) == 0 @property def should_poll(self): @@ -146,8 +142,8 @@ class CommandSwitch(SwitchEntity): _LOGGER.error("No state command specified") return if self._value_template: - return CommandSwitch._query_state_value(self._command_state) - return CommandSwitch._query_state_code(self._command_state) + return self._query_state_value(self._command_state) + return self._query_state_code(self._command_state) def update(self): """Update device state.""" @@ -159,12 +155,12 @@ class CommandSwitch(SwitchEntity): def turn_on(self, **kwargs): """Turn the device on.""" - if CommandSwitch._switch(self._command_on) and not self._command_state: + if self._switch(self._command_on) and not self._command_state: self._state = True self.schedule_update_ha_state() def turn_off(self, **kwargs): """Turn the device off.""" - if CommandSwitch._switch(self._command_off) and not self._command_state: + if self._switch(self._command_off) and not self._command_state: self._state = False self.schedule_update_ha_state() diff --git a/tests/components/command_line/test_cover.py b/tests/components/command_line/test_cover.py index c4f45ed9704..cc91e521d68 100644 --- a/tests/components/command_line/test_cover.py +++ b/tests/components/command_line/test_cover.py @@ -27,6 +27,7 @@ def rs(hass): "command_stop", "command_state", None, + 15, ) @@ -45,7 +46,7 @@ def test_query_state_value(rs): assert "foo bar" == result assert mock_run.call_count == 1 assert mock_run.call_args == mock.call( - "runme", shell=True, # nosec # shell by design + "runme", shell=True, timeout=15 # nosec # shell by design ) diff --git a/tests/components/command_line/test_notify.py b/tests/components/command_line/test_notify.py index ecdb5af91da..8509bc785da 100644 --- a/tests/components/command_line/test_notify.py +++ b/tests/components/command_line/test_notify.py @@ -4,7 +4,7 @@ import tempfile import unittest import homeassistant.components.notify as notify -from homeassistant.setup import setup_component +from homeassistant.setup import async_setup_component, setup_component from tests.async_mock import patch from tests.common import assert_setup_component, get_test_home_assistant @@ -93,3 +93,25 @@ class TestCommandLine(unittest.TestCase): "notify", "test", {"message": "error"}, blocking=True ) assert mock_error.call_count == 1 + + +async def test_timeout(hass, caplog): + """Test we do not block forever.""" + assert await async_setup_component( + hass, + notify.DOMAIN, + { + "notify": { + "name": "test", + "platform": "command_line", + "command": "sleep 10000", + "command_timeout": 0.0000001, + } + }, + ) + await hass.async_block_till_done() + assert await hass.services.async_call( + "notify", "test", {"message": "error"}, blocking=True + ) + await hass.async_block_till_done() + assert "Timeout" in caplog.text diff --git a/tests/components/command_line/test_sensor.py b/tests/components/command_line/test_sensor.py index 9d7e46002f6..623269b9c16 100644 --- a/tests/components/command_line/test_sensor.py +++ b/tests/components/command_line/test_sensor.py @@ -74,7 +74,7 @@ class TestCommandSensorSensor(unittest.TestCase): """Ensure command with templates and quotes get rendered properly.""" self.hass.states.set("sensor.test_state", "Works 2") with patch( - "homeassistant.components.command_line.sensor.subprocess.check_output", + "homeassistant.components.command_line.subprocess.check_output", return_value=b"Works\n", ) as check_output: data = command_line.CommandSensorData( diff --git a/tests/components/command_line/test_switch.py b/tests/components/command_line/test_switch.py index 5c4a1aa336f..a9d9c61444a 100644 --- a/tests/components/command_line/test_switch.py +++ b/tests/components/command_line/test_switch.py @@ -180,13 +180,14 @@ class TestCommandSwitch(unittest.TestCase): "echo 'off command'", None, None, + 15, ] no_state_device = command_line.CommandSwitch(*init_args) assert no_state_device.assumed_state # Set state command - init_args[-2] = "cat {}" + init_args[-3] = "cat {}" state_device = command_line.CommandSwitch(*init_args) assert not state_device.assumed_state @@ -201,6 +202,7 @@ class TestCommandSwitch(unittest.TestCase): "echo 'off command'", False, None, + 15, ] test_switch = command_line.CommandSwitch(*init_args)