From b77ee2d1ada81a5229370b823970fe2d722a1bd6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 13:41:00 -1000 Subject: [PATCH 1/7] Log failing snapshot URI to improve debug --- onvif/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/onvif/client.py b/onvif/client.py index 84ffde5..cf95df4 100644 --- a/onvif/client.py +++ b/onvif/client.py @@ -576,9 +576,9 @@ async def get_snapshot( try: response = await self._snapshot_client.get(uri, auth=auth) except httpx.TimeoutException as error: - raise ONVIFTimeoutError(error) from error + raise ONVIFTimeoutError(f"Timed out fetching {uri}: {error}") from error except httpx.RequestError as error: - raise ONVIFError(error) from error + raise ONVIFError(f"Error fetching {uri}: {error}") from error if response.status_code == 401: raise ONVIFAuthError(f"Failed to authenticate to {uri}") From a0afb31b06b160259cb57704ccb86a6b017a7da0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:19:41 -1000 Subject: [PATCH 2/7] Try fetching the snapshot URI without user/pass if it fails --- onvif/client.py | 63 ++++++++++++++++++++++++++++++++++++++++++------ requirements.txt | 1 + 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/onvif/client.py b/onvif/client.py index cf95df4..443ba60 100644 --- a/onvif/client.py +++ b/onvif/client.py @@ -8,7 +8,8 @@ import os.path from typing import Any from collections.abc import Callable - +from yarl import URL +from multidict import CIMultiDict import httpx from httpx import AsyncClient, BasicAuth, DigestAuth from zeep.cache import SqliteCache @@ -44,6 +45,36 @@ _WRITE_TIMEOUT = 90 _HTTPX_LIMITS = httpx.Limits(keepalive_expiry=KEEPALIVE_EXPIRY) _NO_VERIFY_SSL_CONTEXT = create_no_verify_ssl_context() +_CREDENTIAL_KEYS = ("username", "user", "pass", "password") + + +def strip_user_pass_url(url: str) -> str: + """Strip password from URL.""" + parsed_url = URL(url) + query = parsed_url.query + new_query: CIMultiDict | None = None + for key in _CREDENTIAL_KEYS: + if key in query: + if new_query is None: + new_query = CIMultiDict(parsed_url.query) + new_query.popall(key) + parsed_url = parsed_url.with_query(new_query) + return str(parsed_url) + + +def obscure_user_pass_url(url: str) -> str: + """Obscure user and password from URL.""" + parsed_url = URL(url) + query = parsed_url.query + new_query: CIMultiDict | None = None + for key in _CREDENTIAL_KEYS: + if key in query: + if new_query is None: + new_query = CIMultiDict(parsed_url.query) + new_query.popall(key) + new_query[key] = "********" + parsed_url = parsed_url.with_query(new_query) + return str(parsed_url) def safe_func(func): @@ -573,12 +604,16 @@ async def get_snapshot( else: auth = DigestAuth(self.user, self.passwd) - try: - response = await self._snapshot_client.get(uri, auth=auth) - except httpx.TimeoutException as error: - raise ONVIFTimeoutError(f"Timed out fetching {uri}: {error}") from error - except httpx.RequestError as error: - raise ONVIFError(f"Error fetching {uri}: {error}") from error + response = await self._try_snapshot_uri(uri, auth) + + # If the request fails with a 401, make sure to strip any + # sample user/pass from the URL and try again + if ( + response.status_code == 401 + and (stripped_uri := strip_user_pass_url(uri)) + and stripped_uri != uri + ): + response = await self._try_snapshot_uri(stripped_uri, auth) if response.status_code == 401: raise ONVIFAuthError(f"Failed to authenticate to {uri}") @@ -588,6 +623,20 @@ async def get_snapshot( return None + async def _try_snapshot_uri( + self, uri: str, auth: BasicAuth | DigestAuth | None + ) -> httpx.Response: + try: + return await self._snapshot_client.get(uri, auth=auth) + except httpx.TimeoutException as error: + raise ONVIFTimeoutError( + f"Timed out fetching {obscure_user_pass_url(uri)}: {error}" + ) from error + except httpx.RequestError as error: + raise ONVIFError( + f"Error fetching {obscure_user_pass_url(uri)}: {error}" + ) from error + def get_definition( self, name: str, port_type: str | None = None ) -> tuple[str, str, str]: diff --git a/requirements.txt b/requirements.txt index a93b1aa..e89dead 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ ciso8601==2.3.2 httpx==0.28.1 zeep[async]==4.3.1 +yarl>=1.10.0 From 68f4c1cae2434b56eee9ce4aa4940442b168a6f8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:22:46 -1000 Subject: [PATCH 3/7] add cover --- tests/test_client.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/test_client.py diff --git a/tests/test_client.py b/tests/test_client.py new file mode 100644 index 0000000..80c8cca --- /dev/null +++ b/tests/test_client.py @@ -0,0 +1,12 @@ +from onvif.client import strip_user_pass_url, obscure_user_pass_url + + +def test_strip_user_pass_url(): + assert strip_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") == "http://1.2.3.4/" + + +def test_obscure_user_pass_url(): + assert ( + obscure_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") + == "http://1.2.3.4/?user=********&pass=********" + ) From a175540d70d2f071dd3701e8bee8f739c82a58b0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:24:22 -1000 Subject: [PATCH 4/7] add cover --- onvif/client.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/onvif/client.py b/onvif/client.py index 443ba60..bc2f8e6 100644 --- a/onvif/client.py +++ b/onvif/client.py @@ -58,8 +58,9 @@ def strip_user_pass_url(url: str) -> str: if new_query is None: new_query = CIMultiDict(parsed_url.query) new_query.popall(key) - parsed_url = parsed_url.with_query(new_query) - return str(parsed_url) + if new_query: + return str(parsed_url.with_query(new_query)) + return url def obscure_user_pass_url(url: str) -> str: @@ -73,8 +74,9 @@ def obscure_user_pass_url(url: str) -> str: new_query = CIMultiDict(parsed_url.query) new_query.popall(key) new_query[key] = "********" - parsed_url = parsed_url.with_query(new_query) - return str(parsed_url) + if new_query: + return str(parsed_url.with_query(new_query)) + return url def safe_func(func): From 94f004f19ce55be959a1d35f8d6270a4311d1265 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:24:57 -1000 Subject: [PATCH 5/7] add cover --- onvif/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/onvif/client.py b/onvif/client.py index bc2f8e6..bdaf472 100644 --- a/onvif/client.py +++ b/onvif/client.py @@ -58,7 +58,7 @@ def strip_user_pass_url(url: str) -> str: if new_query is None: new_query = CIMultiDict(parsed_url.query) new_query.popall(key) - if new_query: + if new_query is not None: return str(parsed_url.with_query(new_query)) return url @@ -74,7 +74,7 @@ def obscure_user_pass_url(url: str) -> str: new_query = CIMultiDict(parsed_url.query) new_query.popall(key) new_query[key] = "********" - if new_query: + if new_query is not None: return str(parsed_url.with_query(new_query)) return url From 2c2f32f92f1103f33c6ae111bd4e26f9f83a14ec Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:26:16 -1000 Subject: [PATCH 6/7] add cover --- tests/test_client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_client.py b/tests/test_client.py index 80c8cca..2668918 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -3,6 +3,7 @@ def test_strip_user_pass_url(): assert strip_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") == "http://1.2.3.4/" + assert strip_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/" def test_obscure_user_pass_url(): @@ -10,3 +11,4 @@ def test_obscure_user_pass_url(): obscure_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") == "http://1.2.3.4/?user=********&pass=********" ) + assert obscure_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/" From f942fc393010fe9df6aa60a49355ef233a1c750d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 22 Jan 2025 14:28:52 -1000 Subject: [PATCH 7/7] move to util --- onvif/client.py | 43 ++++++++----------------------------------- onvif/util.py | 36 +++++++++++++++++++++++++++++++++++- tests/test_client.py | 14 -------------- tests/test_util.py | 14 ++++++++++++++ 4 files changed, 57 insertions(+), 50 deletions(-) delete mode 100644 tests/test_client.py diff --git a/onvif/client.py b/onvif/client.py index bdaf472..5f4f7b3 100644 --- a/onvif/client.py +++ b/onvif/client.py @@ -8,8 +8,6 @@ import os.path from typing import Any from collections.abc import Callable -from yarl import URL -from multidict import CIMultiDict import httpx from httpx import AsyncClient, BasicAuth, DigestAuth from zeep.cache import SqliteCache @@ -28,7 +26,14 @@ from .settings import DEFAULT_SETTINGS from .transport import ASYNC_TRANSPORT from .types import FastDateTime, ForgivingTime -from .util import create_no_verify_ssl_context, normalize_url, path_isfile, utcnow +from .util import ( + create_no_verify_ssl_context, + normalize_url, + path_isfile, + utcnow, + strip_user_pass_url, + obscure_user_pass_url, +) from .wrappers import retry_connection_error # noqa: F401 from .wsa import WsAddressingIfMissingPlugin @@ -45,38 +50,6 @@ _WRITE_TIMEOUT = 90 _HTTPX_LIMITS = httpx.Limits(keepalive_expiry=KEEPALIVE_EXPIRY) _NO_VERIFY_SSL_CONTEXT = create_no_verify_ssl_context() -_CREDENTIAL_KEYS = ("username", "user", "pass", "password") - - -def strip_user_pass_url(url: str) -> str: - """Strip password from URL.""" - parsed_url = URL(url) - query = parsed_url.query - new_query: CIMultiDict | None = None - for key in _CREDENTIAL_KEYS: - if key in query: - if new_query is None: - new_query = CIMultiDict(parsed_url.query) - new_query.popall(key) - if new_query is not None: - return str(parsed_url.with_query(new_query)) - return url - - -def obscure_user_pass_url(url: str) -> str: - """Obscure user and password from URL.""" - parsed_url = URL(url) - query = parsed_url.query - new_query: CIMultiDict | None = None - for key in _CREDENTIAL_KEYS: - if key in query: - if new_query is None: - new_query = CIMultiDict(parsed_url.query) - new_query.popall(key) - new_query[key] = "********" - if new_query is not None: - return str(parsed_url.with_query(new_query)) - return url def safe_func(func): diff --git a/onvif/util.py b/onvif/util.py index 794c030..c6d0b17 100644 --- a/onvif/util.py +++ b/onvif/util.py @@ -9,7 +9,8 @@ import ssl from typing import Any from urllib.parse import ParseResultBytes, urlparse, urlunparse - +from yarl import URL +from multidict import CIMultiDict from zeep.exceptions import Fault utcnow: partial[dt.datetime] = partial(dt.datetime.now, dt.timezone.utc) @@ -18,6 +19,8 @@ # to minimize the impact of the blocking I/O. path_isfile = lru_cache(maxsize=128)(os.path.isfile) +_CREDENTIAL_KEYS = ("username", "password", "user", "pass") + def normalize_url(url: bytes | str | None) -> str | None: """Normalize URL. @@ -105,3 +108,34 @@ def create_no_verify_ssl_context() -> ssl.SSLContext: # ssl.OP_LEGACY_SERVER_CONNECT is only available in Python 3.12a4+ sslcontext.options |= getattr(ssl, "OP_LEGACY_SERVER_CONNECT", 0x4) return sslcontext + + +def strip_user_pass_url(url: str) -> str: + """Strip password from URL.""" + parsed_url = URL(url) + query = parsed_url.query + new_query: CIMultiDict | None = None + for key in _CREDENTIAL_KEYS: + if key in query: + if new_query is None: + new_query = CIMultiDict(parsed_url.query) + new_query.popall(key) + if new_query is not None: + return str(parsed_url.with_query(new_query)) + return url + + +def obscure_user_pass_url(url: str) -> str: + """Obscure user and password from URL.""" + parsed_url = URL(url) + query = parsed_url.query + new_query: CIMultiDict | None = None + for key in _CREDENTIAL_KEYS: + if key in query: + if new_query is None: + new_query = CIMultiDict(parsed_url.query) + new_query.popall(key) + new_query[key] = "********" + if new_query is not None: + return str(parsed_url.with_query(new_query)) + return url diff --git a/tests/test_client.py b/tests/test_client.py deleted file mode 100644 index 2668918..0000000 --- a/tests/test_client.py +++ /dev/null @@ -1,14 +0,0 @@ -from onvif.client import strip_user_pass_url, obscure_user_pass_url - - -def test_strip_user_pass_url(): - assert strip_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") == "http://1.2.3.4/" - assert strip_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/" - - -def test_obscure_user_pass_url(): - assert ( - obscure_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") - == "http://1.2.3.4/?user=********&pass=********" - ) - assert obscure_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/" diff --git a/tests/test_util.py b/tests/test_util.py index e3eaa71..c3d36aa 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -4,6 +4,7 @@ import pytest from zeep.loader import parse_xml +from onvif.util import strip_user_pass_url, obscure_user_pass_url from onvif.client import ONVIFCamera from onvif.settings import DEFAULT_SETTINGS @@ -40,3 +41,16 @@ async def test_normalize_url_with_missing_url(): ) result = operation.process_reply(envelope) assert normalize_url(result.SubscriptionReference.Address._value_1) is None + + +def test_strip_user_pass_url(): + assert strip_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") == "http://1.2.3.4/" + assert strip_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/" + + +def test_obscure_user_pass_url(): + assert ( + obscure_user_pass_url("http://1.2.3.4/?user=foo&pass=bar") + == "http://1.2.3.4/?user=********&pass=********" + ) + assert obscure_user_pass_url("http://1.2.3.4/") == "http://1.2.3.4/"