From dd0b823fd94e5ce48e6f5fac5338b31ea45fc5c4 Mon Sep 17 00:00:00 2001 From: pvizeli Date: Wed, 9 Nov 2016 15:31:14 +0100 Subject: [PATCH 01/10] Synology SSL fix & Error handling --- homeassistant/components/camera/synology.py | 69 +++++++++++++++------ 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index d6cc0a3e506d9c..92533c33bdfe3d 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -9,13 +9,15 @@ import voluptuous as vol +import aiohttp from aiohttp import web from aiohttp.web_exceptions import HTTPGatewayTimeout import async_timeout +from homeassistant.core import callback from homeassistant.const import ( CONF_NAME, CONF_USERNAME, CONF_PASSWORD, - CONF_URL, CONF_WHITELIST, CONF_VERIFY_SSL) + CONF_URL, CONF_WHITELIST, CONF_VERIFY_SSL, EVENT_HOMEASSISTANT_STOP) from homeassistant.components.camera import ( Camera, PLATFORM_SCHEMA) import homeassistant.helpers.config_validation as cv @@ -44,6 +46,8 @@ SYNO_API_URL = '{0}{1}{2}' +DATA_WEBSESSION = 'synology_websession' + PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, vol.Required(CONF_USERNAME): cv.string, @@ -57,9 +61,26 @@ @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Setup a Synology IP Camera.""" - if not config.get(CONF_VERIFY_SSL): - _LOGGER.warning('SSL verification currently cannot be disabled. ' - 'See https://goo.gl/1h1119') + if config.get(CONF_VERIFY_SSL): + # init websession + if not hass.data.get(DATA_WEBSESSION): + conn = aiohttp.TCPConnector(verify_ssl=False) + hass.data[DATA_WEBSESSION] = aiohttp.ClientSession( + connector=conn, + loop=hass.loop + ) + + @callback + def _async_close_synlogy_websession(event): + """Close websession on shutdown.""" + hass.data[DATA_WEBSESSION].close() + + hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STOP, _async_close_synlogy_websession) + + websession = hass.data.get(DATA_WEBSESSION) + else: + websession = hass.websession # Determine API to use for authentication syno_api_url = SYNO_API_URL.format( @@ -73,12 +94,12 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - query_req = yield from hass.websession.get( + query_req = yield from websession.get( syno_api_url, params=query_payload, ) - except asyncio.TimeoutError: - _LOGGER.error("Timeout on %s", syno_api_url) + except (asyncio.TimeoutError, aiohttp.errors.ClientError): + _LOGGER.exception("Error on %s", syno_api_url) return False query_resp = yield from query_req.json() @@ -95,6 +116,7 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): config.get(CONF_URL), WEBAPI_PATH, auth_path) session_id = yield from get_session_id( + websession, hass, config.get(CONF_USERNAME), config.get(CONF_PASSWORD), @@ -113,13 +135,13 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - camera_req = yield from hass.websession.get( + camera_req = yield from websession.get( syno_camera_url, params=camera_payload, cookies={'id': session_id} ) - except asyncio.TimeoutError: - _LOGGER.error("Timeout on %s", syno_camera_url) + except (asyncio.TimeoutError, aiohttp.errors.ClientError): + _LOGGER.exception("Error on %s", syno_camera_url) return False camera_resp = yield from camera_req.json() @@ -135,6 +157,8 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): snapshot_path = camera['snapshot_path'] device = SynologyCamera( + hass, + websession, config, camera_id, camera['name'], @@ -151,7 +175,8 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): @asyncio.coroutine -def get_session_id(hass, username, password, login_url, valid_cert): +def get_session_id(websession, hass, username, password, login_url, + valid_cert): """Get a session id.""" auth_payload = { 'api': AUTH_API, @@ -164,12 +189,12 @@ def get_session_id(hass, username, password, login_url, valid_cert): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - auth_req = yield from hass.websession.get( + auth_req = yield from websession.get( login_url, params=auth_payload, ) - except asyncio.TimeoutError: - _LOGGER.error("Timeout on %s", login_url) + except (asyncio.TimeoutError, aiohttp.errors.ClientError): + _LOGGER.exception("Error on %s", login_url) return False auth_resp = yield from auth_req.json() @@ -181,10 +206,12 @@ def get_session_id(hass, username, password, login_url, valid_cert): class SynologyCamera(Camera): """An implementation of a Synology NAS based IP camera.""" - def __init__(self, config, camera_id, camera_name, + def __init__(self, hass, websession, config, camera_id, camera_name, snapshot_path, streaming_path, camera_path, auth_path): """Initialize a Synology Surveillance Station camera.""" super().__init__() + self.hass = hass + self._websession = websession self._name = camera_name self._username = config.get(CONF_USERNAME) self._password = config.get(CONF_PASSWORD) @@ -205,6 +232,7 @@ def __init__(self, config, camera_id, camera_name, def async_read_sid(self): """Get a session id.""" self._session_id = yield from get_session_id( + self._websession, self.hass, self._username, self._password, @@ -231,13 +259,13 @@ def async_camera_image(self): } try: with async_timeout.timeout(TIMEOUT, loop=self.hass.loop): - response = yield from self.hass.websession.get( + response = yield from self._websession.get( image_url, params=image_payload, cookies={'id': self._session_id} ) - except asyncio.TimeoutError: - _LOGGER.error("Timeout on %s", image_url) + except (asyncio.TimeoutError, aiohttp.errors.ClientError): + _LOGGER.exception("Error on %s", image_url) return None image = yield from response.read() @@ -260,12 +288,13 @@ def handle_async_mjpeg_stream(self, request): } try: with async_timeout.timeout(TIMEOUT, loop=self.hass.loop): - stream = yield from self.hass.websession.get( + stream = yield from self._websession.get( streaming_url, payload=streaming_payload, cookies={'id': self._session_id} ) - except asyncio.TimeoutError: + except (asyncio.TimeoutError, aiohttp.errors.ClientError): + _LOGGER.exception("Error on %s", streaming_url) raise HTTPGatewayTimeout() response = web.StreamResponse() From 5830796a8de364c1a9bcbc5392b74fdc94fd3fa1 Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:07:53 +0100 Subject: [PATCH 02/10] change handling for cookies/ssl --- homeassistant/components/camera/synology.py | 83 ++++++++------------- 1 file changed, 33 insertions(+), 50 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 92533c33bdfe3d..465f28ee657c3d 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -46,8 +46,6 @@ SYNO_API_URL = '{0}{1}{2}' -DATA_WEBSESSION = 'synology_websession' - PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, vol.Required(CONF_USERNAME): cv.string, @@ -62,25 +60,17 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Setup a Synology IP Camera.""" if config.get(CONF_VERIFY_SSL): - # init websession - if not hass.data.get(DATA_WEBSESSION): - conn = aiohttp.TCPConnector(verify_ssl=False) - hass.data[DATA_WEBSESSION] = aiohttp.ClientSession( - connector=conn, - loop=hass.loop - ) - - @callback - def _async_close_synlogy_websession(event): - """Close websession on shutdown.""" - hass.data[DATA_WEBSESSION].close() + connector = aiohttp.TCPConnector(verify_ssl=False) - hass.bus.async_listen_once( - EVENT_HOMEASSISTANT_STOP, _async_close_synlogy_websession) + @callback + def _async_close_connector(event): + """Close websession on shutdown.""" + connector.close() - websession = hass.data.get(DATA_WEBSESSION) + hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STOP, _async_close_connector) else: - websession = hass.websession + connector = None # Determine API to use for authentication syno_api_url = SYNO_API_URL.format( @@ -94,9 +84,11 @@ def _async_close_synlogy_websession(event): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - query_req = yield from websession.get( + query_req = yield from asyncio.get( syno_api_url, params=query_payload, + loop=hass.loop, + connector=connector ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", syno_api_url) @@ -116,14 +108,24 @@ def _async_close_synlogy_websession(event): config.get(CONF_URL), WEBAPI_PATH, auth_path) session_id = yield from get_session_id( - websession, hass, + connector, config.get(CONF_USERNAME), config.get(CONF_PASSWORD), - syno_auth_url, - config.get(CONF_VERIFY_SSL) + syno_auth_url ) + # init websession + websession = asyncio.ClientSession( + loop=hass.loop, connector=connector, cookies={'id': session_id}) + + @asyncio.coroutine + def _async_close_websession(event): + yield from websession.close() + + hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_STOP, _async_close_websession) + # Use SessionID to get cameras in system syno_camera_url = SYNO_API_URL.format( config.get(CONF_URL), WEBAPI_PATH, camera_api) @@ -138,7 +140,6 @@ def _async_close_synlogy_websession(event): camera_req = yield from websession.get( syno_camera_url, params=camera_payload, - cookies={'id': session_id} ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", syno_camera_url) @@ -175,8 +176,7 @@ def _async_close_synlogy_websession(event): @asyncio.coroutine -def get_session_id(websession, hass, username, password, login_url, - valid_cert): +def get_session_id(hass, connector, username, password, login_url): """Get a session id.""" auth_payload = { 'api': AUTH_API, @@ -189,9 +189,11 @@ def get_session_id(websession, hass, username, password, login_url, } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - auth_req = yield from websession.get( + auth_req = yield from asyncio.get( login_url, params=auth_payload, + loop=hass.loop, + connector=connector ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", login_url) @@ -206,39 +208,22 @@ def get_session_id(websession, hass, username, password, login_url, class SynologyCamera(Camera): """An implementation of a Synology NAS based IP camera.""" - def __init__(self, hass, websession, config, camera_id, camera_name, - snapshot_path, streaming_path, camera_path, auth_path): + def __init__(self, hass, websession, config, camera_id, + camera_name, snapshot_path, streaming_path, camera_path, + auth_path): """Initialize a Synology Surveillance Station camera.""" super().__init__() self.hass = hass self._websession = websession self._name = camera_name - self._username = config.get(CONF_USERNAME) - self._password = config.get(CONF_PASSWORD) self._synology_url = config.get(CONF_URL) - self._api_url = config.get(CONF_URL) + 'webapi/' - self._login_url = config.get(CONF_URL) + '/webapi/' + 'auth.cgi' self._camera_name = config.get(CONF_CAMERA_NAME) self._stream_id = config.get(CONF_STREAM_ID) - self._valid_cert = config.get(CONF_VERIFY_SSL) self._camera_id = camera_id self._snapshot_path = snapshot_path self._streaming_path = streaming_path self._camera_path = camera_path self._auth_path = auth_path - self._session_id = None - - @asyncio.coroutine - def async_read_sid(self): - """Get a session id.""" - self._session_id = yield from get_session_id( - self._websession, - self.hass, - self._username, - self._password, - self._login_url, - self._valid_cert - ) def camera_image(self): """Return bytes of camera image.""" @@ -261,8 +246,7 @@ def async_camera_image(self): with async_timeout.timeout(TIMEOUT, loop=self.hass.loop): response = yield from self._websession.get( image_url, - params=image_payload, - cookies={'id': self._session_id} + params=image_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", image_url) @@ -290,8 +274,7 @@ def handle_async_mjpeg_stream(self, request): with async_timeout.timeout(TIMEOUT, loop=self.hass.loop): stream = yield from self._websession.get( streaming_url, - payload=streaming_payload, - cookies={'id': self._session_id} + payload=streaming_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", streaming_url) From 1a0d6b05219ac26b43347eadab574b9d00af23b6 Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:26:27 +0100 Subject: [PATCH 03/10] fix use not deprecated functions --- homeassistant/components/camera/synology.py | 24 +++++++++++---------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 465f28ee657c3d..8651a95a0e4294 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -72,6 +72,11 @@ def _async_close_connector(event): else: connector = None + websession_init = aiohttp.ClientSession( + loop=hass.loop, + connector=connector + ) + # Determine API to use for authentication syno_api_url = SYNO_API_URL.format( config.get(CONF_URL), WEBAPI_PATH, QUERY_CGI) @@ -84,11 +89,9 @@ def _async_close_connector(event): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - query_req = yield from asyncio.get( + query_req = yield from websession_init.get( syno_api_url, - params=query_payload, - loop=hass.loop, - connector=connector + params=query_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", syno_api_url) @@ -108,13 +111,14 @@ def _async_close_connector(event): config.get(CONF_URL), WEBAPI_PATH, auth_path) session_id = yield from get_session_id( - hass, - connector, + websession_init, config.get(CONF_USERNAME), config.get(CONF_PASSWORD), syno_auth_url ) + yield from websession_init.close() + # init websession websession = asyncio.ClientSession( loop=hass.loop, connector=connector, cookies={'id': session_id}) @@ -176,7 +180,7 @@ def _async_close_websession(event): @asyncio.coroutine -def get_session_id(hass, connector, username, password, login_url): +def get_session_id(websession, username, password, login_url): """Get a session id.""" auth_payload = { 'api': AUTH_API, @@ -189,11 +193,9 @@ def get_session_id(hass, connector, username, password, login_url): } try: with async_timeout.timeout(TIMEOUT, loop=hass.loop): - auth_req = yield from asyncio.get( + auth_req = yield from websession.get( login_url, - params=auth_payload, - loop=hass.loop, - connector=connector + params=auth_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", login_url) From c47e02eeea184d213b677fc85eb2eadc1c608af4 Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:32:56 +0100 Subject: [PATCH 04/10] fix lint --- homeassistant/components/camera/synology.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 8651a95a0e4294..ca3848318222ce 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -111,6 +111,7 @@ def _async_close_connector(event): config.get(CONF_URL), WEBAPI_PATH, auth_path) session_id = yield from get_session_id( + hass, websession_init, config.get(CONF_USERNAME), config.get(CONF_PASSWORD), @@ -120,7 +121,7 @@ def _async_close_connector(event): yield from websession_init.close() # init websession - websession = asyncio.ClientSession( + websession = aiohttp.ClientSession( loop=hass.loop, connector=connector, cookies={'id': session_id}) @asyncio.coroutine @@ -155,7 +156,6 @@ def _async_close_websession(event): # add cameras devices = [] - tasks = [] for camera in cameras: if not config.get(CONF_WHITELIST): camera_id = camera['id'] @@ -172,15 +172,13 @@ def _async_close_websession(event): camera_path, auth_path ) - tasks.append(device.async_read_sid()) devices.append(device) - yield from asyncio.wait(tasks, loop=hass.loop) yield from async_add_devices(devices) @asyncio.coroutine -def get_session_id(websession, username, password, login_url): +def get_session_id(hass, websession, username, password, login_url): """Get a session id.""" auth_payload = { 'api': AUTH_API, From 31a2fe14e18ba1e431fc98cf728400c3f2513a6f Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:38:55 +0100 Subject: [PATCH 05/10] change verify --- homeassistant/components/camera/synology.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index ca3848318222ce..04e2a240452f03 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -59,7 +59,7 @@ @asyncio.coroutine def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Setup a Synology IP Camera.""" - if config.get(CONF_VERIFY_SSL): + if not config.get(CONF_VERIFY_SSL): connector = aiohttp.TCPConnector(verify_ssl=False) @callback From 57579b1fbcfe459ed546b1956a0991a33edfba9e Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:44:00 +0100 Subject: [PATCH 06/10] fix connector close to coro --- homeassistant/components/camera/synology.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 04e2a240452f03..6229f1ef262691 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -62,10 +62,10 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): if not config.get(CONF_VERIFY_SSL): connector = aiohttp.TCPConnector(verify_ssl=False) - @callback + @asyncio.coroutine def _async_close_connector(event): """Close websession on shutdown.""" - connector.close() + yield from connector.close() hass.bus.async_listen_once( EVENT_HOMEASSISTANT_STOP, _async_close_connector) From 4bb295a9c447d3ea7386d06522e2cffab0531368 Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:56:27 +0100 Subject: [PATCH 07/10] fix force close --- homeassistant/components/camera/synology.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 6229f1ef262691..a80a5c1a4b2538 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -118,7 +118,7 @@ def _async_close_connector(event): syno_auth_url ) - yield from websession_init.close() + websession_init.detach() # init websession websession = aiohttp.ClientSession( @@ -126,6 +126,7 @@ def _async_close_connector(event): @asyncio.coroutine def _async_close_websession(event): + """Close webssesion on shutdown.""" yield from websession.close() hass.bus.async_listen_once( @@ -144,7 +145,7 @@ def _async_close_websession(event): with async_timeout.timeout(TIMEOUT, loop=hass.loop): camera_req = yield from websession.get( syno_camera_url, - params=camera_payload, + params=camera_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", syno_camera_url) From 453747bc8f853e3bfa3306b850c24535e7eff66b Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 10 Nov 2016 23:59:42 +0100 Subject: [PATCH 08/10] not needed since websession close connector too --- homeassistant/components/camera/synology.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index a80a5c1a4b2538..e14f7d524b5eb4 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -61,14 +61,6 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): """Setup a Synology IP Camera.""" if not config.get(CONF_VERIFY_SSL): connector = aiohttp.TCPConnector(verify_ssl=False) - - @asyncio.coroutine - def _async_close_connector(event): - """Close websession on shutdown.""" - yield from connector.close() - - hass.bus.async_listen_once( - EVENT_HOMEASSISTANT_STOP, _async_close_connector) else: connector = None From fdebcd7105cfac6e2d57a57779fdd4ad6ea2728d Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Fri, 11 Nov 2016 00:03:19 +0100 Subject: [PATCH 09/10] fix params --- homeassistant/components/camera/synology.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index e14f7d524b5eb4..51ac573e998339 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -267,7 +267,7 @@ def handle_async_mjpeg_stream(self, request): with async_timeout.timeout(TIMEOUT, loop=self.hass.loop): stream = yield from self._websession.get( streaming_url, - payload=streaming_payload + params=streaming_payload ) except (asyncio.TimeoutError, aiohttp.errors.ClientError): _LOGGER.exception("Error on %s", streaming_url) From d30ab3914d3636ab43200b356fe1344b3408da8d Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Fri, 11 Nov 2016 00:11:05 +0100 Subject: [PATCH 10/10] fix lint --- homeassistant/components/camera/synology.py | 1 - 1 file changed, 1 deletion(-) diff --git a/homeassistant/components/camera/synology.py b/homeassistant/components/camera/synology.py index 51ac573e998339..bbca25fd6b688f 100644 --- a/homeassistant/components/camera/synology.py +++ b/homeassistant/components/camera/synology.py @@ -14,7 +14,6 @@ from aiohttp.web_exceptions import HTTPGatewayTimeout import async_timeout -from homeassistant.core import callback from homeassistant.const import ( CONF_NAME, CONF_USERNAME, CONF_PASSWORD, CONF_URL, CONF_WHITELIST, CONF_VERIFY_SSL, EVENT_HOMEASSISTANT_STOP)