From 9ff36a886badd2fa84ec3f179dcdc0e1a5edd244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Fri, 20 Dec 2024 16:35:41 -0600 Subject: [PATCH] fix: disallow empty hosts on `_UriData` and fetch-libs on build --- .jujuignore | 1 + charmcraft.yaml | 1 + .../filesystem_client/v0/filesystem_info.py | 25 +++++++++++++------ .../filesystem_client/v0/filesystem_info.py | 25 +++++++++++++------ 4 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 .jujuignore diff --git a/.jujuignore b/.jujuignore new file mode 100644 index 0000000..2bfa6a4 --- /dev/null +++ b/.jujuignore @@ -0,0 +1 @@ +tests/ diff --git a/charmcraft.yaml b/charmcraft.yaml index b628329..abe220d 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -27,6 +27,7 @@ parts: charm-requirements: ["requirements.txt"] override-build: | just requirements + charmcraft fetch-libs craftctl default charm-libs: diff --git a/lib/charms/filesystem_client/v0/filesystem_info.py b/lib/charms/filesystem_client/v0/filesystem_info.py index 22a33c4..aa0a903 100644 --- a/lib/charms/filesystem_client/v0/filesystem_info.py +++ b/lib/charms/filesystem_client/v0/filesystem_info.py @@ -138,7 +138,7 @@ def _on_start(self, event: ops.StartEvent) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 _logger = logging.getLogger(__name__) @@ -217,12 +217,17 @@ def __init__( ) -> None: if not scheme: raise FilesystemInfoError("scheme cannot be empty") - if len(hosts) == 0: + str_hosts = [] + for host in hosts: + if not host: + raise FilesystemInfoError(f"invalid host `{host}`") + str_hosts.append(str(host)) + if len(str_hosts) == 0: raise FilesystemInfoError("list of hosts cannot be empty") # Strictly convert to the required types to avoid passing through weird data. object.__setattr__(self, "scheme", str(scheme)) - object.__setattr__(self, "hosts", [str(host) for host in hosts]) + object.__setattr__(self, "hosts", str_hosts) object.__setattr__(self, "user", str(user) if user else "") object.__setattr__(self, "path", str(path) if path else "/") object.__setattr__( @@ -245,10 +250,14 @@ def from_uri(cls, uri: str) -> "_UriData": hosts = hostname[1:-1].split(",") path = unquote(result.path or "") try: - options = { - key: ",".join(values) - for key, values in parse_qs(result.query, strict_parsing=True).items() - } + options = ( + { + key: ",".join(values) + for key, values in parse_qs(result.query, strict_parsing=True).items() + } + if result.query + else {} + ) except ValueError: raise ParseUriError(f"invalid options for endpoint `{uri}`") try: @@ -378,7 +387,7 @@ def to_uri(self, _model: Model) -> str: except AddressValueError: host = self.hostname - hosts = [host + f":{self.port}" if self.port else ""] + hosts = [host + (f":{self.port}" if self.port else "")] return str(_UriData(scheme=self.filesystem_type(), hosts=hosts, path=self.path)) diff --git a/tests/integration/server/lib/charms/filesystem_client/v0/filesystem_info.py b/tests/integration/server/lib/charms/filesystem_client/v0/filesystem_info.py index 22a33c4..aa0a903 100644 --- a/tests/integration/server/lib/charms/filesystem_client/v0/filesystem_info.py +++ b/tests/integration/server/lib/charms/filesystem_client/v0/filesystem_info.py @@ -138,7 +138,7 @@ def _on_start(self, event: ops.StartEvent) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 _logger = logging.getLogger(__name__) @@ -217,12 +217,17 @@ def __init__( ) -> None: if not scheme: raise FilesystemInfoError("scheme cannot be empty") - if len(hosts) == 0: + str_hosts = [] + for host in hosts: + if not host: + raise FilesystemInfoError(f"invalid host `{host}`") + str_hosts.append(str(host)) + if len(str_hosts) == 0: raise FilesystemInfoError("list of hosts cannot be empty") # Strictly convert to the required types to avoid passing through weird data. object.__setattr__(self, "scheme", str(scheme)) - object.__setattr__(self, "hosts", [str(host) for host in hosts]) + object.__setattr__(self, "hosts", str_hosts) object.__setattr__(self, "user", str(user) if user else "") object.__setattr__(self, "path", str(path) if path else "/") object.__setattr__( @@ -245,10 +250,14 @@ def from_uri(cls, uri: str) -> "_UriData": hosts = hostname[1:-1].split(",") path = unquote(result.path or "") try: - options = { - key: ",".join(values) - for key, values in parse_qs(result.query, strict_parsing=True).items() - } + options = ( + { + key: ",".join(values) + for key, values in parse_qs(result.query, strict_parsing=True).items() + } + if result.query + else {} + ) except ValueError: raise ParseUriError(f"invalid options for endpoint `{uri}`") try: @@ -378,7 +387,7 @@ def to_uri(self, _model: Model) -> str: except AddressValueError: host = self.hostname - hosts = [host + f":{self.port}" if self.port else ""] + hosts = [host + (f":{self.port}" if self.port else "")] return str(_UriData(scheme=self.filesystem_type(), hosts=hosts, path=self.path))