From a4eb975b595a5bf0a41f6a39b0cd6b9c01970e06 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Fri, 28 Nov 2014 22:27:44 -0800 Subject: [PATCH] HTTP more robust and increased test coverage --- homeassistant/components/http/__init__.py | 117 +++++++++--------- homeassistant/remote.py | 62 +++++++--- test/test_component_http.py | 139 ++++++++++++++++++++-- test/test_remote.py | 2 +- 4 files changed, 232 insertions(+), 88 deletions(-) diff --git a/homeassistant/components/http/__init__.py b/homeassistant/components/http/__init__.py index 7f054190128..79bd17044f4 100644 --- a/homeassistant/components/http/__init__.py +++ b/homeassistant/components/http/__init__.py @@ -108,6 +108,8 @@ CONF_SERVER_HOST = "server_host" CONF_SERVER_PORT = "server_port" CONF_DEVELOPMENT = "development" +DATA_API_PASSWORD = 'api_password' + _LOGGER = logging.getLogger(__name__) @@ -118,7 +120,7 @@ def setup(hass, config): _LOGGER): return False - api_password = config[DOMAIN]['api_password'] + api_password = config[DOMAIN][CONF_API_PASSWORD] # If no server host is given, accept all incoming requests server_host = config[DOMAIN].get(CONF_SERVER_HOST, '0.0.0.0') @@ -192,7 +194,6 @@ class RequestHandler(SimpleHTTPRequestHandler): PATHS = [ # debug interface ('GET', URL_ROOT, '_handle_get_root'), - ('POST', URL_ROOT, '_handle_get_root'), # /api - for validation purposes ('GET', rem.URL_API, '_handle_get_api'), @@ -228,8 +229,10 @@ class RequestHandler(SimpleHTTPRequestHandler): ('DELETE', rem.URL_API_EVENT_FORWARD, '_handle_delete_api_event_forward'), - # Statis files + # Static files ('GET', re.compile(r'/static/(?P[a-zA-Z\._\-0-9/]+)'), + '_handle_get_static'), + ('HEAD', re.compile(r'/static/(?P[a-zA-Z\._\-0-9/]+)'), '_handle_get_static') ] @@ -255,24 +258,22 @@ class RequestHandler(SimpleHTTPRequestHandler): if content_length: body_content = self.rfile.read(content_length).decode("UTF-8") - if self.use_json: - try: - data.update(json.loads(body_content)) - except ValueError: - _LOGGER.exception("Exception parsing JSON: %s", - body_content) + try: + data.update(json.loads(body_content)) + except (TypeError, ValueError): + # TypeError is JSON object is not a dict + # ValueError if we could not parse JSON + _LOGGER.exception("Exception parsing JSON: %s", + body_content) - self._message( - "Error parsing JSON", HTTP_UNPROCESSABLE_ENTITY) - return - else: - data.update({key: value[-1] for key, value in - parse_qs(body_content).items()}) + self._json_message( + "Error parsing JSON", HTTP_UNPROCESSABLE_ENTITY) + return api_password = self.headers.get(rem.AUTH_HEADER) - if not api_password and 'api_password' in data: - api_password = data['api_password'] + if not api_password and DATA_API_PASSWORD in data: + api_password = data[DATA_API_PASSWORD] if '_METHOD' in data: method = data.pop('_METHOD') @@ -307,7 +308,7 @@ class RequestHandler(SimpleHTTPRequestHandler): # For API calls we need a valid password if self.use_json and api_password != self.server.api_password: - self._message( + self._json_message( "API password missing or incorrect.", HTTP_UNAUTHORIZED) else: @@ -315,9 +316,11 @@ class RequestHandler(SimpleHTTPRequestHandler): elif path_matched_but_not_method: self.send_response(HTTP_METHOD_NOT_ALLOWED) + self.end_headers() else: self.send_response(HTTP_NOT_FOUND) + self.end_headers() def do_HEAD(self): # pylint: disable=invalid-name """ HEAD request handler. """ @@ -377,7 +380,7 @@ class RequestHandler(SimpleHTTPRequestHandler): # pylint: disable=unused-argument def _handle_get_api(self, path_match, data): """ Renders the debug interface. """ - self._message("API running.") + self._json_message("API running.") # pylint: disable=unused-argument def _handle_get_api_states(self, path_match, data): @@ -394,7 +397,7 @@ class RequestHandler(SimpleHTTPRequestHandler): if state: self._write_json(state) else: - self._message("State does not exist.", HTTP_NOT_FOUND) + self._json_message("State does not exist.", HTTP_NOT_FOUND) def _handle_post_state_entity(self, path_match, data): """ Handles updating the state of an entity. @@ -407,7 +410,7 @@ class RequestHandler(SimpleHTTPRequestHandler): try: new_state = data['state'] except KeyError: - self._message("state not specified", HTTP_BAD_REQUEST) + self._json_message("state not specified", HTTP_BAD_REQUEST) return attributes = data['attributes'] if 'attributes' in data else None @@ -417,19 +420,14 @@ class RequestHandler(SimpleHTTPRequestHandler): # Write state self.server.hass.states.set(entity_id, new_state, attributes) - # Return state if json, else redirect to main page - if self.use_json: - state = self.server.hass.states.get(entity_id) + state = self.server.hass.states.get(entity_id) - status_code = HTTP_CREATED if is_new_state else HTTP_OK + status_code = HTTP_CREATED if is_new_state else HTTP_OK - self._write_json( - state.as_dict(), - status_code=status_code, - location=rem.URL_API_STATES_ENTITY.format(entity_id)) - else: - self._message( - "State of {} changed to {}".format(entity_id, new_state)) + self._write_json( + state.as_dict(), + status_code=status_code, + location=rem.URL_API_STATES_ENTITY.format(entity_id)) def _handle_get_api_events(self, path_match, data): """ Handles getting overview of event listeners. """ @@ -448,8 +446,8 @@ class RequestHandler(SimpleHTTPRequestHandler): event_type = path_match.group('event_type') if event_data is not None and not isinstance(event_data, dict): - self._message("event_data should be an object", - HTTP_UNPROCESSABLE_ENTITY) + self._json_message("event_data should be an object", + HTTP_UNPROCESSABLE_ENTITY) event_origin = ha.EventOrigin.remote @@ -464,7 +462,7 @@ class RequestHandler(SimpleHTTPRequestHandler): self.server.hass.bus.fire(event_type, event_data, event_origin) - self._message("Event {} fired.".format(event_type)) + self._json_message("Event {} fired.".format(event_type)) def _handle_get_api_services(self, path_match, data): """ Handles getting overview of services. """ @@ -485,7 +483,7 @@ class RequestHandler(SimpleHTTPRequestHandler): self.server.hass.call_service(domain, service, data) - self._message("Service {}/{} called.".format(domain, service)) + self._json_message("Service {}/{} called.".format(domain, service)) # pylint: disable=invalid-name def _handle_post_api_event_forward(self, path_match, data): @@ -495,26 +493,31 @@ class RequestHandler(SimpleHTTPRequestHandler): host = data['host'] api_password = data['api_password'] except KeyError: - self._message("No host or api_password received.", - HTTP_BAD_REQUEST) + self._json_message("No host or api_password received.", + HTTP_BAD_REQUEST) return try: port = int(data['port']) if 'port' in data else None except ValueError: - self._message( + self._json_message( "Invalid value received for port", HTTP_UNPROCESSABLE_ENTITY) return + api = rem.API(host, api_password, port) + + if not api.validate_api(): + self._json_message( + "Unable to validate API", HTTP_UNPROCESSABLE_ENTITY) + return + if self.server.event_forwarder is None: self.server.event_forwarder = \ rem.EventForwarder(self.server.hass) - api = rem.API(host, api_password, port) - self.server.event_forwarder.connect(api) - self._message("Event forwarding setup.") + self._json_message("Event forwarding setup.") def _handle_delete_api_event_forward(self, path_match, data): """ Handles deleting an event forwarding target. """ @@ -522,14 +525,14 @@ class RequestHandler(SimpleHTTPRequestHandler): try: host = data['host'] except KeyError: - self._message("No host received.", - HTTP_BAD_REQUEST) + self._json_message("No host received.", + HTTP_BAD_REQUEST) return try: port = int(data['port']) if 'port' in data else None except ValueError: - self._message( + self._json_message( "Invalid value received for port", HTTP_UNPROCESSABLE_ENTITY) return @@ -538,7 +541,7 @@ class RequestHandler(SimpleHTTPRequestHandler): self.server.event_forwarder.disconnect(api) - self._message("Event forwarding cancelled.") + self._json_message("Event forwarding cancelled.") def _handle_get_static(self, path_match, data): """ Returns a static file. """ @@ -585,7 +588,10 @@ class RequestHandler(SimpleHTTPRequestHandler): self.end_headers() - if do_gzip: + if self.command == 'HEAD': + return + + elif do_gzip: self.wfile.write(gzip_data) else: @@ -599,22 +605,9 @@ class RequestHandler(SimpleHTTPRequestHandler): if inp: inp.close() - def _message(self, message, status_code=HTTP_OK): + def _json_message(self, message, status_code=HTTP_OK): """ Helper method to return a message to the caller. """ - if self.use_json: - self._write_json({'message': message}, status_code=status_code) - else: - self.send_error(status_code, message) - - def _redirect(self, location): - """ Helper method to redirect caller. """ - self.send_response(HTTP_MOVED_PERMANENTLY) - - self.send_header( - "Location", "{}?api_password={}".format( - location, self.server.api_password)) - - self.end_headers() + self._write_json({'message': message}, status_code=status_code) def _write_json(self, data=None, status_code=HTTP_OK, location=None): """ Helper method to return JSON to the caller. """ diff --git a/homeassistant/remote.py b/homeassistant/remote.py index c0aca8700f3..bcfe6e68e65 100644 --- a/homeassistant/remote.py +++ b/homeassistant/remote.py @@ -34,6 +34,7 @@ URL_API_EVENT_FORWARD = "/api/event_forwarding" METHOD_GET = "get" METHOD_POST = "post" +METHOD_DELETE = "delete" _LOGGER = logging.getLogger(__name__) @@ -94,6 +95,10 @@ class API(object): _LOGGER.exception(error) raise ha.HomeAssistantError(error) + def __repr__(self): + return "API({}, {}, {})".format( + self.host, self.api_password, self.port) + class HomeAssistant(ha.HomeAssistant): """ Home Assistant that forwards work. """ @@ -122,19 +127,23 @@ class HomeAssistant(ha.HomeAssistant): import random # pylint: disable=too-many-format-args - random_password = '%030x'.format(random.randrange(16**30)) + random_password = '{:30}'.format(random.randrange(16**30)) http.setup( self, {http.DOMAIN: {http.CONF_API_PASSWORD: random_password}}) ha.Timer(self) - # Setup that events from remote_api get forwarded to local_api - connect_remote_events(self.remote_api, self.local_api) - self.bus.fire(ha.EVENT_HOMEASSISTANT_START, origin=ha.EventOrigin.remote) + # Setup that events from remote_api get forwarded to local_api + # Do this after we fire START, otherwise HTTP is not started + if not connect_remote_events(self.remote_api, self.local_api): + raise ha.HomeAssistantError(( + 'Could not setup event forwarding from api {} to ' + 'local api {}').format(self.remote_api, self.local_api)) + def stop(self): """ Stops Home Assistant and shuts down all threads. """ _LOGGER.info("Stopping") @@ -289,30 +298,51 @@ def validate_api(api): def connect_remote_events(from_api, to_api): """ Sets up from_api to forward all events to to_api. """ - data = {'host': to_api.host, 'api_password': to_api.api_password} - - if to_api.port is not None: - data['port'] = to_api.port + data = { + 'host': to_api.host, + 'api_password': to_api.api_password, + 'port': to_api.port + } try: - from_api(METHOD_POST, URL_API_EVENT_FORWARD, data) + req = from_api(METHOD_POST, URL_API_EVENT_FORWARD, data) + + if req.status_code == 200: + return True + else: + _LOGGER.error( + "Error settign up event forwarding: %s - %s", + req.status_code, req.text) + + return False except ha.HomeAssistantError: - pass + _LOGGER.exception("Error setting up event forwarding") + return False def disconnect_remote_events(from_api, to_api): """ Disconnects forwarding events from from_api to to_api. """ - data = {'host': to_api.host, '_METHOD': 'DELETE'} - - if to_api.port is not None: - data['port'] = to_api.port + data = { + 'host': to_api.host, + 'port': to_api.port + } try: - from_api(METHOD_POST, URL_API_EVENT_FORWARD, data) + req = from_api(METHOD_DELETE, URL_API_EVENT_FORWARD, data) + + if req.status_code == 200: + return True + else: + _LOGGER.error( + "Error removing event forwarding: %s - %s", + req.status_code, req.text) + + return False except ha.HomeAssistantError: - pass + _LOGGER.exception("Error removing an event forwarder") + return False def get_event_listeners(api): diff --git a/test/test_component_http.py b/test/test_component_http.py index 640600becca..f4080daaa8b 100644 --- a/test/test_component_http.py +++ b/test/test_component_http.py @@ -52,30 +52,50 @@ def setUpModule(): # pylint: disable=invalid-name def tearDownModule(): # pylint: disable=invalid-name """ Stops the Home Assistant server. """ - global hass - hass.stop() class TestHTTP(unittest.TestCase): """ Test the HTTP debug interface and API. """ - def test_get_frontend(self): + def test_setup(self): + """ Test http.setup. """ + self.assertFalse(http.setup(hass, {})) + self.assertFalse(http.setup(hass, {http.DOMAIN: {}})) + + def test_frontend_and_static(self): """ Tests if we can get the frontend. """ req = requests.get(_url("")) self.assertEqual(200, req.status_code) + # Test we can retrieve frontend.js frontendjs = re.search( r'(?P\/static\/frontend-[A-Za-z0-9]{32}.html)', - req.text).groups(0)[0] + req.text) self.assertIsNotNone(frontendjs) - req = requests.get(_url(frontendjs)) + req = requests.head(_url(frontendjs.groups(0)[0])) self.assertEqual(200, req.status_code) + # Test auto filling in api password + req = requests.get( + _url("?{}={}".format(http.DATA_API_PASSWORD, API_PASSWORD))) + + self.assertEqual(200, req.status_code) + + auth_text = re.search(r"auth='{}'".format(API_PASSWORD), req.text) + + self.assertIsNotNone(auth_text) + + # Test 404 + self.assertEqual(404, requests.get(_url("/not-existing")).status_code) + + # Test we cannot POST to / + self.assertEqual(405, requests.post(_url("")).status_code) + def test_api_password(self): """ Test if we get access denied if we omit or provide a wrong api password. """ @@ -127,8 +147,8 @@ class TestHTTP(unittest.TestCase): hass.states.set("test.test", "not_to_be_set") requests.post(_url(remote.URL_API_STATES_ENTITY.format("test.test")), - data=json.dumps({"state": "debug_state_change2", - "api_password": API_PASSWORD})) + data=json.dumps({"state": "debug_state_change2"}), + headers=HA_HEADERS) self.assertEqual("debug_state_change2", hass.states.get("test.test").state) @@ -143,8 +163,8 @@ class TestHTTP(unittest.TestCase): req = requests.post( _url(remote.URL_API_STATES_ENTITY.format( "test_entity.that_does_not_exist")), - data=json.dumps({"state": new_state, - "api_password": API_PASSWORD})) + data=json.dumps({'state': new_state}), + headers=HA_HEADERS) cur_state = (hass.states. get("test_entity.that_does_not_exist").state) @@ -152,6 +172,20 @@ class TestHTTP(unittest.TestCase): self.assertEqual(201, req.status_code) self.assertEqual(cur_state, new_state) + # pylint: disable=invalid-name + def test_api_state_change_with_bad_data(self): + """ Test if API sends appropriate error if we omit state. """ + + new_state = "debug_state_change" + + req = requests.post( + _url(remote.URL_API_STATES_ENTITY.format( + "test_entity.that_does_not_exist")), + data=json.dumps({}), + headers=HA_HEADERS) + + self.assertEqual(400, req.status_code) + # pylint: disable=invalid-name def test_api_fire_event_with_no_data(self): """ Test if the API allows us to fire an event. """ @@ -214,6 +248,17 @@ class TestHTTP(unittest.TestCase): self.assertEqual(422, req.status_code) self.assertEqual(0, len(test_value)) + # Try now with valid but unusable JSON + req = requests.post( + _url(remote.URL_API_EVENTS_EVENT.format("test_event_bad_data")), + data=json.dumps([1, 2, 3]), + headers=HA_HEADERS) + + hass._pool.block_till_done() + + self.assertEqual(422, req.status_code) + self.assertEqual(0, len(test_value)) + def test_api_get_event_listeners(self): """ Test if we can get the list of events being listened for. """ req = requests.get(_url(remote.URL_API_EVENTS), @@ -279,3 +324,79 @@ class TestHTTP(unittest.TestCase): hass._pool.block_till_done() self.assertEqual(1, len(test_value)) + + def test_api_event_forward(self): + """ Test setting up event forwarding. """ + + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + headers=HA_HEADERS) + self.assertEqual(400, req.status_code) + + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({'host': '127.0.0.1'}), + headers=HA_HEADERS) + self.assertEqual(400, req.status_code) + + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({'api_password': 'bla-di-bla'}), + headers=HA_HEADERS) + self.assertEqual(400, req.status_code) + + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({ + 'api_password': 'bla-di-bla', + 'host': '127.0.0.1', + 'port': 'abcd' + }), + headers=HA_HEADERS) + self.assertEqual(422, req.status_code) + + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({ + 'api_password': 'bla-di-bla', + 'host': '127.0.0.1', + 'port': '8125' + }), + headers=HA_HEADERS) + self.assertEqual(422, req.status_code) + + # Setup a real one + req = requests.post( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({ + 'api_password': API_PASSWORD, + 'host': '127.0.0.1', + 'port': SERVER_PORT + }), + headers=HA_HEADERS) + self.assertEqual(200, req.status_code) + + # Delete it again.. + req = requests.delete( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({}), + headers=HA_HEADERS) + self.assertEqual(400, req.status_code) + + req = requests.delete( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({ + 'host': '127.0.0.1', + 'port': 'abcd' + }), + headers=HA_HEADERS) + self.assertEqual(422, req.status_code) + + req = requests.delete( + _url(remote.URL_API_EVENT_FORWARD), + data=json.dumps({ + 'host': '127.0.0.1', + 'port': SERVER_PORT + }), + headers=HA_HEADERS) + self.assertEqual(200, req.status_code) diff --git a/test/test_remote.py b/test/test_remote.py index 07e7a23416d..948a84a7600 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -15,7 +15,7 @@ import homeassistant.components.http as http API_PASSWORD = "test1234" -HTTP_BASE_URL = "http://127.0.0.1:{}".format(remote.SERVER_PORT) +HTTP_BASE_URL = "http://127.0.0.1:8122" HA_HEADERS = {remote.AUTH_HEADER: API_PASSWORD}