Skip to content

fix(kvstore): percent-encode keys and list items in request paths - #63

Open
Mohith26 wants to merge 1 commit into
quicknode:mainfrom
Mohith26:fix/kvstore-encode-path-segments
Open

Mohith26 wants to merge 1 commit into
quicknode:mainfrom
Mohith26:fix/kvstore-encode-path-segments

Conversation

@Mohith26

@Mohith26 Mohith26 commented Nov 3, 2026

Copy link
Copy Markdown

The KV store client builds request paths with base_url.join(&format!("lists/{key}/contains/{item}")), so keys and list items are parsed as URL syntax instead of being sent as values:

  • list_contains_item("my-list", "https://example.com/a?b=1#top") requests /lists/my-list/contains/https:/example.com/a with query b=1, and the #top part is dropped.
  • get_set("user:1/profile") requests /sets/user:1/profile.
  • delete_list_item("my-list", "../../sets/x") sends DELETE /sets/x, a different endpoint.

This change adds a small url(&[segments]) helper on ResolvedKvStoreConfig that appends segments with path_segments_mut(), the same approach admin::transactions uses for get_transaction, and uses it for every KV endpoint that takes a key or item. Plain keys like my-key or user:1 produce exactly the same URLs as before. One behavior note: a caller who was already percent-encoding keys themselves would now get them encoded twice.

Testing:

  • Added three wiremock tests (get_set_key_is_one_encoded_path_segment, list_contains_item_encodes_key_and_item, delete_list_item_cannot_escape_the_list) that expect the encoded path. All three fail on main and pass with this change.
  • cargo test -p quicknode-sdk --lib: 289 passed. cargo fmt --all -- --check and cargo clippy -p quicknode-sdk --lib --tests -- -D warnings are clean. I didn't build the Python/Node/Ruby bindings locally; the public method signatures are unchanged.
  • I don't have a KV Store account, so I haven't checked this against the live API. It assumes the API decodes percent-encoded path segments, as standard HTTP routers do.

I used an AI assistant while working on this, and reviewed and tested the change myself.

The KV store client built URLs with format!("lists/{key}/contains/{item}")
and Url::join, so a key or item containing '/', '?', '#' or '..' was
parsed as URL syntax: a '?' started a query string, a '#' cut the rest
off as a fragment, and '../' segments sent the request to a different
endpoint (delete_list_item("my-list", "../../sets/x") issued
DELETE /sets/x). Build these paths with path_segments_mut() instead, as
the admin client already does, so each key and item is one encoded
segment.
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.

1 participant