Skip to content

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

Merged
andrii-novikov merged 6 commits into
developmentfrom
fix/124-encode-storage-resource-paths
Aug 26, 2026
Merged

fix: percent-encode storage resource paths on the wire (#124)#125
andrii-novikov merged 6 commits into
developmentfrom
fix/124-encode-storage-resource-paths

Conversation

@andrii-novikov

@andrii-novikov andrii-novikov commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Applicable issues

Description of changes

DIAL Core parses every storage resource path — both in the request URL (upload/download/delete/get_metadata) and in JSON body fields (sourceUrl/destinationUrl for move/copy, url for permission grants) — as a percent-encoded path (each segment is URL-decoded via UrlUtil.decodePath, which runs new URI(segment)).

A raw reserved character in a filename therefore broke things:

  • # / ? were truncated by the client's urlparse in get_api_path before the request was even built → wrong path → 404.
  • [ / ] / space and friends reached Core unencoded → new URI(...) throws URISyntaxException, which Core wraps in a bare RuntimeException500.

The URL-slot ops only worked by accident (httpx encodes the URL path on the wire); the body-slot ops (move_to/copy_to) had nothing encoding them at all.

Fix:

  • Add percent_encode_resource_url() (stateless helper) and DialStorageResourceMixin.get_encoded_api_path() in helpers/storage_resource.py.
  • Each path segment is URL-decoded then re-encoded, so a decoded path (my file.txt) and an already-encoded one (my%20file.txt, as returned by the API) converge to the same wire form without double-encoding. Absolute URLs (always encoded by the API) pass through untouched.
  • Applied across:
    • files.py — upload, download, delete, move_to, copy_to, get_metadata
    • metadata.py — the shared get (covers conversations and direct low-level client.metadata.get(...) calls)
    • prompts.py — save, get, delete, get_metadata
    • resource_permissions.pygrant body urls

Verification:

  • New regression tests for reserved-character encoding + already-encoded round-trip across move/copy, download (URL + decoded filename), and both files.get_metadata and low-level metadata.get.
  • Full unit suite: 279 passed; pyright clean; nox -s format clean.
  • Differential wire test (new vs old code): previously-working inputs are byte-identical (no double-encoding, no regression); only the previously-broken reserved-char cases changed.

Notes:

  • Identifier endpoints (application/deployments/model/toolset) were left unchanged — they interpolate deployment/model names, not storage paths, and conventionally contain no reserved characters.
  • Caveat of the normalize-both contract: a filename containing a literal % sequence can't be expressed from a decoded path (extremely rare).

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@andrii-novikov
andrii-novikov requested a review from adubovik as a code owner July 27, 2026 14:23
DIAL Core parses every resource path (in the URL and in JSON bodies such as
sourceUrl/destinationUrl) as a percent-encoded URL. A raw reserved character
(space, #, ?, [, ...) either got truncated by urlparse or reached Core
unencoded, making 'new URI(...)' throw and Core answer 404/500.

Add percent_encode_resource_url() plus DialStorageResourceMixin
.get_encoded_api_path(), which decode-then-encode each segment so decoded
('my file.txt') and already-encoded ('my%20file.txt', as returned by the API)
inputs converge without double-encoding. Apply it across files (upload,
download, delete, move_to, copy_to, get_metadata), the shared metadata.get
(also covers conversations and direct low-level calls), prompts, and
resource_permissions.grant.
Drop the _prepare_file_download wrapper; encode the url and decode the
returned filename directly in the shared (files-only) _prepare_download_request.
Comment thread aidial_client/helpers/storage_resource.py Outdated
Comment thread aidial_client/helpers/storage_resource.py Outdated
Comment thread aidial_client/resources/resource_permissions.py Outdated
Comment thread aidial_client/helpers/storage_resource.py Outdated
Move URL encoding into safe_parse_storage_resource so get_api_path
encodes automatically, drop the now-redundant get_encoded_api_path,
rename percent_encode_resource_url to the private
_percent_encode_relative_url, widen mixin helpers to accept
str | PurePosixPath, and extract a shared _grant_body helper for
ResourcePermissions.
@andrii-novikov
andrii-novikov merged commit 96a4ea6 into development Aug 26, 2026
10 checks passed
@andrii-novikov
andrii-novikov deleted the fix/124-encode-storage-resource-paths branch August 26, 2026 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

files.move_to / copy_to 500 on paths with spaces or reserved characters (raw path sent in JSON body)

2 participants