fix: percent-encode storage resource paths on the wire (#124)

aidial_client/helpers/storage_resource.py

@@ -1,6 +1,6 @@
 from pathlib import PurePosixPath
 from typing import Literal, cast, get_args
-from urllib.parse import urljoin, urlparse
+from urllib.parse import quote, unquote, urljoin, urlparse, urlsplit
 
 from aidial_client._compatibility.pydantic_v1 import BaseModel
 from aidial_client._constants import API_PREFIX
@@ -12,6 +12,22 @@
 StorageResourceType = Literal["files", "conversations", "prompts"]
 
 
+def _percent_encode_relative_url(url: str) -> str:
+    """
+    Percent-encode each path segment so reserved characters (space, ``#``,
+    ``?``, ``[`` …) reach DIAL Core encoded instead of making it answer 500.
+    Segments are decoded first, so a decoded path (``my file.txt``) and an
+    already-encoded one (``my%20file.txt``, as returned by the API) converge
+    without double-encoding. Absolute URLs come from the API already encoded and
+    are returned untouched.
+    """
+    if urlsplit(url).netloc:
+        return url
+
+    segments = url.split("/")
+    return "/".join(quote(unquote(seg), safe="") for seg in segments)
+
+
 def _is_directory(s: str) -> bool:
     return s[-1] == "/"
 
@@ -60,7 +76,7 @@ def safe_parse_storage_resource(
             f"API prefix as relative part is not allowed: {url}"
         )
 
-    absolute_url = urljoin(dial_api_url, url)
+    absolute_url = urljoin(dial_api_url, _percent_encode_relative_url(url))
     url_parsed = urlparse(absolute_url)
     dial_api_parsed = urlparse(dial_api_url)
     if url_parsed.netloc != dial_api_parsed.netloc:
@@ -133,27 +149,30 @@ class DialStorageResourceMixin(BaseModel):
     resource_type: StorageResourceType
     dial_api_url: str
 
-    def get_storage_resource(self, url: str) -> DialStorageResource:
+    def get_storage_resource(
+        self, url: str | PurePosixPath
+    ) -> DialStorageResource:
         """
         Get the storage resource object from the URL
         Args:
-            url (str): The URL to be processed.
+            url (str | PurePosixPath): The URL to be processed.
         Returns:
             DialStorageResource: The storage resource object
         """
         return parse_storage_resource(
-            url=url,
+            url=str(url),
             dial_api_url=self.dial_api_url,
             expected_resource_type=self.resource_type,
         )
 
-    def get_api_path(self, url: str) -> str:
+    def get_api_path(self, url: str | PurePosixPath) -> str:
         """
-        Convert URL, that could relative or absolute, to relative URL
+        Convert URL, that could relative or absolute, to relative,
+        percent-encoded API path.
         """
         return self.get_storage_resource(url).api_path
 
-    def get_display_name(self, url: str) -> str:
+    def get_display_name(self, url: str | PurePosixPath) -> str:
         """
         Get the display name of the resource from the URL
         """
@@ -164,7 +183,7 @@ def _prepare_download_request(
         url: str | PurePosixPath,
         etag_if_match: str | None,
     ) -> tuple[FinalRequestOptions, str]:
-        storage_resource = self.get_storage_resource(str(url))
+        storage_resource = self.get_storage_resource(url)
 
         if storage_resource.filename is None:
             raise InvalidDialURLError("URL points to a directory, not a file")
@@ -179,4 +198,5 @@ def _prepare_download_request(
             ),
         )
 
-        return options, storage_resource.filename
+        # api_path is percent-encoded; return a human-readable filename.
+        return options, unquote(storage_resource.filename)

aidial_client/resources/files.py

@@ -25,6 +25,19 @@
 from aidial_client.types.metadata import FileItem, FileMetadata
 
 
+def _move_copy_body(
+    resource: DialStorageResourceMixin,
+    source: str | PurePosixPath,
+    destination: str | PurePosixPath,
+    overwrite: bool,
+) -> dict[str, object]:
+    return {
+        "sourceUrl": resource.get_api_path(source),
+        "destinationUrl": resource.get_api_path(destination),
+        "overwrite": overwrite,
+    }
+
+
 def _files_error_processor(
     http_status_error: httpx.HTTPStatusError,
 ) -> DialException | None:
@@ -54,7 +67,7 @@ def upload(
             cast_to=FileItem,
             options=FinalRequestOptions(
                 method="PUT",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 files={"file": file},
                 headers=remove_none(
                     {
@@ -88,7 +101,7 @@ def delete(
             cast_to=NoneType,
             options=FinalRequestOptions(
                 method="DELETE",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 headers=remove_none(
                     {
                         "If-Match": etag_if_match,
@@ -109,11 +122,7 @@ def move_to(
             options=FinalRequestOptions(
                 method="POST",
                 url=urljoin(API_PREFIX, "ops/resource/move"),
-                json_data={
-                    "sourceUrl": self.get_api_path(str(source)),
-                    "destinationUrl": self.get_api_path(str(destination)),
-                    "overwrite": overwrite,
-                },
+                json_data=_move_copy_body(self, source, destination, overwrite),
             ),
             on_http_error=_files_error_processor,
         )
@@ -129,11 +138,7 @@ def copy_to(
             options=FinalRequestOptions(
                 method="POST",
                 url=urljoin(API_PREFIX, "ops/resource/copy"),
-                json_data={
-                    "sourceUrl": self.get_api_path(str(source)),
-                    "destinationUrl": self.get_api_path(str(destination)),
-                    "overwrite": overwrite,
-                },
+                json_data=_move_copy_body(self, source, destination, overwrite),
             ),
             on_http_error=_files_error_processor,
         )
@@ -147,7 +152,7 @@ def get_metadata(
     ) -> FileMetadata:
         return self.metadata.get(
             resource="files",
-            relative_url=self.get_api_path(str(url)),
+            relative_url=self.get_api_path(url),
             limit=limit,
             token=token,
         )
@@ -168,7 +173,7 @@ async def upload(
             cast_to=FileItem,
             options=FinalRequestOptions(
                 method="PUT",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 files={"file": file},
                 headers=remove_none(
                     {
@@ -215,7 +220,7 @@ async def delete(
             cast_to=NoneType,
             options=FinalRequestOptions(
                 method="DELETE",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 headers=remove_none(
                     {
                         "If-Match": etag_if_match,
@@ -236,11 +241,7 @@ async def move_to(
             options=FinalRequestOptions(
                 method="POST",
                 url=urljoin(API_PREFIX, "ops/resource/move"),
-                json_data={
-                    "sourceUrl": self.get_api_path(str(source)),
-                    "destinationUrl": self.get_api_path(str(destination)),
-                    "overwrite": overwrite,
-                },
+                json_data=_move_copy_body(self, source, destination, overwrite),
             ),
             on_http_error=_files_error_processor,
         )
@@ -256,11 +257,7 @@ async def copy_to(
             options=FinalRequestOptions(
                 method="POST",
                 url=urljoin(API_PREFIX, "ops/resource/copy"),
-                json_data={
-                    "sourceUrl": self.get_api_path(str(source)),
-                    "destinationUrl": self.get_api_path(str(destination)),
-                    "overwrite": overwrite,
-                },
+                json_data=_move_copy_body(self, source, destination, overwrite),
             ),
             on_http_error=_files_error_processor,
         )
@@ -274,7 +271,7 @@ async def get_metadata(
     ) -> FileMetadata:
         return await self.metadata.get(
             resource="files",
-            relative_url=self.get_api_path(str(url)),
+            relative_url=self.get_api_path(url),
             limit=limit,
             token=token,
         )

aidial_client/resources/metadata.py

@@ -6,7 +6,10 @@
 from aidial_client._constants import METADATA_PREFIX
 from aidial_client._internal_types._http_request import FinalRequestOptions
 from aidial_client._utils._dict import remove_none
-from aidial_client.helpers.storage_resource import StorageResourceType
+from aidial_client.helpers.storage_resource import (
+    StorageResourceType,
+    _percent_encode_relative_url,
+)
 from aidial_client.resources.base import AsyncResource, Resource
 from aidial_client.types.metadata import (
     ConversationMetadata,
@@ -71,7 +74,10 @@ def get(
             cast_to=_get_cast_to(resource),
             options=FinalRequestOptions(
                 method="GET",
-                url=urljoin(METADATA_PREFIX, relative_url),
+                url=urljoin(
+                    METADATA_PREFIX,
+                    _percent_encode_relative_url(relative_url),
+                ),
                 params=remove_none({"limit": limit, "token": token}),
             ),
         )
@@ -120,7 +126,10 @@ async def get(
             cast_to=_get_cast_to(resource),
             options=FinalRequestOptions(
                 method="GET",
-                url=urljoin(METADATA_PREFIX, relative_url),
+                url=urljoin(
+                    METADATA_PREFIX,
+                    _percent_encode_relative_url(relative_url),
+                ),
                 params=remove_none({"limit": limit, "token": token}),
             ),
         )

aidial_client/resources/prompts.py

@@ -56,7 +56,7 @@ def save(
             cast_to=PromptItem,
             options=FinalRequestOptions(
                 method="PUT",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 json_data=_prompt_to_json(prompt),
                 headers=remove_none(
                     {
@@ -74,7 +74,7 @@ def get(self, url: str | PurePosixPath) -> Prompt:
             cast_to=Prompt,
             options=FinalRequestOptions(
                 method="GET",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
             ),
             on_http_error=_prompts_error_processor,
         )
@@ -88,7 +88,7 @@ def delete(
             cast_to=NoneType,
             options=FinalRequestOptions(
                 method="DELETE",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 headers=remove_none(
                     {
                         "If-Match": etag_if_match,
@@ -101,7 +101,7 @@ def delete(
     def get_metadata(self, url: str | PurePosixPath) -> PromptMetadata:
         return self.metadata.get(
             resource="prompts",
-            relative_url=self.get_api_path(str(url)),
+            relative_url=self.get_api_path(url),
         )
 
 
@@ -120,7 +120,7 @@ async def save(
             cast_to=PromptItem,
             options=FinalRequestOptions(
                 method="PUT",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 json_data=_prompt_to_json(prompt),
                 headers=remove_none(
                     {
@@ -138,7 +138,7 @@ async def get(self, url: str | PurePosixPath) -> Prompt:
             cast_to=Prompt,
             options=FinalRequestOptions(
                 method="GET",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
             ),
             on_http_error=_prompts_error_processor,
         )
@@ -152,7 +152,7 @@ async def delete(
             cast_to=NoneType,
             options=FinalRequestOptions(
                 method="DELETE",
-                url=urljoin(API_PREFIX, self.get_api_path(str(url))),
+                url=urljoin(API_PREFIX, self.get_api_path(url)),
                 headers=remove_none(
                     {
                         "If-Match": etag_if_match,
@@ -165,5 +165,5 @@ async def delete(
     async def get_metadata(self, url: str | PurePosixPath) -> PromptMetadata:
         return await self.metadata.get(
             resource="prompts",
-            relative_url=self.get_api_path(str(url)),
+            relative_url=self.get_api_path(url),
         )

aidial_client/resources/resource_permissions.py

@@ -1,10 +1,30 @@
 from aidial_client._internal_types._generic import NoneType
 from aidial_client._internal_types._http_request import FinalRequestOptions
+from aidial_client.helpers.storage_resource import (
+    _percent_encode_relative_url,
+)
 from aidial_client.resources.base import AsyncResource, Resource
 
 _GRANT_URL = "v1/ops/resource/per-request-permissions/grant"
 
 
+def _grant_body(
+    resources: list[str],
+    receiver: str,
+    permissions: list[str],
+) -> dict[str, object]:
+    return {
+        "resourcePermissions": [
+            {
+                "url": _percent_encode_relative_url(url),
+                "permissions": permissions,
+            }
+            for url in resources
+        ],
+        "receiver": receiver,
+    }
+
+
 class ResourcePermissions(Resource):
     def grant(
         self,
@@ -19,13 +39,7 @@ def grant(
             options=FinalRequestOptions(
                 method="POST",
                 url=_GRANT_URL,
-                json_data={
-                    "resourcePermissions": [
-                        {"url": url, "permissions": permissions}
-                        for url in resources
-                    ],
-                    "receiver": receiver,
-                },
+                json_data=_grant_body(resources, receiver, permissions),
             ),
         )
 
@@ -44,12 +58,6 @@ async def grant(
             options=FinalRequestOptions(
                 method="POST",
                 url=_GRANT_URL,
-                json_data={
-                    "resourcePermissions": [
-                        {"url": url, "permissions": permissions}
-                        for url in resources
-                    ],
-                    "receiver": receiver,
-                },
+                json_data=_grant_body(resources, receiver, permissions),
             ),
         )