From 6749b7776bfa367c6ebafcddf21d8361b7f1fea1 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Mon, 10 Aug 2026 11:25:20 -0300 Subject: [PATCH 01/10] feat: add support to path addressed requests in sdk --- e2e/src/lib.rs | 4 + .../tests/storage/legacy_put_get_delete.rs | 21 ++--- pubky-sdk/README.md | 14 +-- pubky-sdk/bindings/js/pkg/README.md | 2 +- pubky-sdk/bindings/js/pkg/tests/http.ts | 34 ++++++++ pubky-sdk/bindings/js/pkg/tests/storage.ts | 4 +- pubky-sdk/bindings/js/src/actors/auth_flow.rs | 2 + .../bindings/js/src/actors/cookie_session.rs | 12 ++- pubky-sdk/bindings/js/src/actors/session.rs | 10 ++- pubky-sdk/bindings/js/src/actors/signer.rs | 5 ++ pubky-sdk/bindings/js/src/client/http.rs | 6 +- pubky-sdk/bindings/js/src/pubky.rs | 8 ++ pubky-sdk/src/actors/auth/cookie/builder.rs | 1 + .../src/actors/auth/cookie/credential.rs | 13 ++- .../src/actors/auth/cookie/legacy_api.rs | 7 +- pubky-sdk/src/actors/auth/cookie/mod.rs | 5 ++ pubky-sdk/src/actors/auth/cookie/view.rs | 8 ++ .../src/actors/auth/grant/grant_exchange.rs | 10 ++- pubky-sdk/src/actors/auth/grant/manager.rs | 17 ++-- pubky-sdk/src/actors/event_stream.rs | 4 + pubky-sdk/src/actors/mod.rs | 1 + pubky-sdk/src/actors/signer/session.rs | 8 ++ pubky-sdk/src/actors/storage/resource.rs | 85 +++++++++++++------ pubky-sdk/src/client/core.rs | 4 +- pubky-sdk/src/client/http_targets/mod.rs | 46 ++++++++++ pubky-sdk/src/client/http_targets/native.rs | 53 ++++++++++-- pubky-sdk/src/client/http_targets/wasm.rs | 73 +++++++++++++--- pubky-sdk/src/client/mod.rs | 2 + pubky-sdk/src/lib.rs | 5 +- pubky-sdk/src/pubky.rs | 19 ++++- 30 files changed, 380 insertions(+), 103 deletions(-) diff --git a/e2e/src/lib.rs b/e2e/src/lib.rs index 74c4763f3..88888ab97 100644 --- a/e2e/src/lib.rs +++ b/e2e/src/lib.rs @@ -1,4 +1,8 @@ #![warn(unused_crate_dependencies)] // E2E tests #[cfg(test)] +#[allow( + deprecated, + reason = "E2E tests intentionally cover legacy cookie compatibility alongside grant flows" +)] mod tests; diff --git a/e2e/src/tests/storage/legacy_put_get_delete.rs b/e2e/src/tests/storage/legacy_put_get_delete.rs index 0d8b85b60..6a89955c6 100644 --- a/e2e/src/tests/storage/legacy_put_get_delete.rs +++ b/e2e/src/tests/storage/legacy_put_get_delete.rs @@ -2,6 +2,10 @@ use super::*; #[tokio::test] #[pubky_testnet::test] +#[allow( + deprecated, + reason = "This test covers legacy cookie and routing compatibility" +)] async fn legacy_and_storage_routes_interoperate() { let testnet = build_full_testnet().await; let server = testnet.homeserver_app(); @@ -14,22 +18,16 @@ async fn legacy_and_storage_routes_interoperate() { let cookie_secret = session.as_cookie().unwrap().export_secret().unwrap(); let (cookie_name, cookie_value) = cookie_secret.split_once(':').unwrap(); let cookie = format!("{cookie_name}={cookie_value}"); - let legacy_url = format!( - "{}pub/foo.txt?pubky-host={}", - server.icann_http_url(), - session.public_key().z32() - ); - let storage_url = format!( - "{}storage/{}/pub/foo.txt", - server.icann_http_url(), - session.public_key().z32() - ); + let owner = session.public_key().z32(); + let legacy_url = format!("{}pub/foo.txt", server.icann_http_url()); + let storage_url = format!("{}storage/{}/pub/foo.txt", server.icann_http_url(), owner); // A legacy write can be read and deleted through `/storage`. let response = session .client() .request(Method::PUT, &legacy_url) .header("Host", "non.pubky.host") + .header("pubky-host", &owner) .header("Cookie", &cookie) .body(vec![0, 1, 2, 3, 4]) .send() @@ -66,6 +64,7 @@ async fn legacy_and_storage_routes_interoperate() { .client() .request(Method::GET, &legacy_url) .header("Host", "non.pubky.host") + .header("pubky-host", &owner) .send() .await .unwrap(); @@ -86,6 +85,7 @@ async fn legacy_and_storage_routes_interoperate() { .client() .request(Method::GET, &legacy_url) .header("Host", "non.pubky.host") + .header("pubky-host", &owner) .send() .await .unwrap(); @@ -99,6 +99,7 @@ async fn legacy_and_storage_routes_interoperate() { .client() .request(Method::DELETE, &legacy_url) .header("Host", "non.pubky.host") + .header("pubky-host", &owner) .header("Cookie", &cookie) .send() .await diff --git a/pubky-sdk/README.md b/pubky-sdk/README.md index 0281d4e5e..79d5ff105 100644 --- a/pubky-sdk/README.md +++ b/pubky-sdk/README.md @@ -54,7 +54,11 @@ let caps = Capabilities::builder() .write("/pub/example.com/") .expect("static scope is canonical") .finish(); -let flow = pubky.start_cookie_auth_flow(&caps, AuthFlowKind::signin())?; +let flow = pubky.start_grant_auth_flow( + &caps, + AuthFlowKind::signin(), + ClientId::new("my-cool-app").unwrap(), +)?; println!("Scan to sign in: {}", flow.authorization_url()); let app_session = flow.await_approval().await?; @@ -71,9 +75,9 @@ println!("Your current homeserver: {:?}", resolved); `PublicKey` has two string representations: - **Display format**: `pubky` (used for logs/UI and human-facing identifiers). -- **Transport/storage format**: raw `z32` (used for hostnames, headers, query params, serde/JSON, and database storage). +- **Transport/storage format**: raw `z32` (used for hostnames, storage owner path segments, legacy headers, query params, serde/JSON, and database storage). -Use `.z32()` whenever you are building hostnames or transport values (for example `_pubky.` or the `pubky-host` header). Use `Display`/`.to_string()` when you want the prefixed identifier for people. +Use `.z32()` whenever you are building hostnames or transport values (for example `_pubky.`, `/storage//...`, or the legacy `pubky-host` header). Use `Display`/`.to_string()` when you want the prefixed identifier for people. ### Reuse a single facade across your app @@ -168,7 +172,7 @@ Need to feed a public resource into a raw HTTP client? Use [`resolve_pubky`] to let url = resolve_pubky("pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG")?; assert_eq!( url.as_str(), - "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" + "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/storage/operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" ); # Ok(()) # } @@ -203,7 +207,7 @@ Request an authorization URL and await approval. **Typical usage:** -1. Start an auth flow with `pubky.start_grant_auth_flow(&caps, ..)` (or `pubky.start_cookie_auth_flow(&caps, ..)` for the cookie variant). You can also use `PubkyGrantAuthFlow::builder()` / `PubkyCookieAuthFlow::builder()` to set a custom relay. +1. Start an auth flow with `pubky.start_grant_auth_flow(&caps, ..)`. You can also use `PubkyGrantAuthFlow::builder()` to set a custom relay. The deprecated cookie flow remains available only for compatibility. 2. Show `authorization_url()` (QR/deeplink) to the signing device (e.g., [Pubky Ring](https://github.com/pubky/pubky-ring) — [iOS](https://apps.apple.com/om/app/pubky-ring/id6739356756) / [Android](https://play.google.com/store/apps/details?id=to.pubky.ring)). 3. Await `await_approval()` to obtain a session-bound `PubkySession`, or `await_credential()` for a raw `GrantCredential`/`CookieCredential` that you can persist, inspect, or lift into a session later via `PubkySession::from_{grant,cookie}_credential`. diff --git a/pubky-sdk/bindings/js/pkg/README.md b/pubky-sdk/bindings/js/pkg/README.md index 8e21c541c..14babb9b9 100644 --- a/pubky-sdk/bindings/js/pkg/README.md +++ b/pubky-sdk/bindings/js/pkg/README.md @@ -467,7 +467,7 @@ import { resolvePubky } from "@synonymdev/pubky"; const identifier = "pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG"; const url = resolvePubky(identifier); -// -> "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" +// -> "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/storage/operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" ``` Both `pubky/…` (preferred) and `pubky:///…` resolve to the same HTTPS endpoint. diff --git a/pubky-sdk/bindings/js/pkg/tests/http.ts b/pubky-sdk/bindings/js/pkg/tests/http.ts index bc36ee8df..afd0c433b 100644 --- a/pubky-sdk/bindings/js/pkg/tests/http.ts +++ b/pubky-sdk/bindings/js/pkg/tests/http.ts @@ -138,6 +138,40 @@ test("fetch merges plain object headers", async (t) => { t.end(); }); +test("path-addressed storage omits pubky-host", async (t) => { + const client = Client.testnet(); + const originalFetch = globalThis.fetch; + let seenRequest: Request | undefined; + + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + + if (new URL(request.url).pathname.startsWith("/storage/")) { + seenRequest = request; + } + + return originalFetch(input as RequestInfo | URL, init); + }) as typeof fetch; + + try { + await client.fetch( + `https://${TLD}/storage/${TLD}/pub/missing.txt?cursor=hello%20world`, + ); + } finally { + globalThis.fetch = originalFetch; + } + + t.ok(seenRequest, "fetch preserved the path-addressed storage request"); + t.equal(seenRequest!.headers.get("pubky-host"), null, "pubky-host omitted"); + t.equal(seenRequest!.credentials, "include", "cookie credentials preserved"); + t.equal( + new URL(seenRequest!.url).search, + "?cursor=hello%20world", + "query parameters preserved", + ); + t.end(); +}); + test("fetch failed", async (t) => { const client = Client.testnet(); diff --git a/pubky-sdk/bindings/js/pkg/tests/storage.ts b/pubky-sdk/bindings/js/pkg/tests/storage.ts index 2ac91519d..4f0756974 100644 --- a/pubky-sdk/bindings/js/pkg/tests/storage.ts +++ b/pubky-sdk/bindings/js/pkg/tests/storage.ts @@ -38,7 +38,7 @@ test("resolvePubky helper", (t) => { const pk = HOMESERVER_PUBLICKEY.z32(); const preferred = `pubky${pk}/pub/example.com/data.txt`; const deeplink = `pubky://${pk}/pub/example.com/data.txt`; - const expected = `https://_pubky.${pk}/pub/example.com/data.txt`; + const expected = `https://_pubky.${pk}/storage/${pk}/pub/example.com/data.txt`; t.equal(resolvePubky(preferred), expected, "preferred format resolves"); t.equal(resolvePubky(deeplink), expected, "deeplink format resolves"); @@ -496,7 +496,7 @@ test("unauthorized (no cookie) PUT returns 401", async (t) => { const session = await signer.signin("storage.test"); const userPk = session.info.publicKey.z32(); - const url = `https://_pubky.${userPk}/pub/example.com/unauth.json`; + const url = `https://_pubky.${userPk}/storage/${userPk}/pub/example.com/unauth.json`; await session.signout(); diff --git a/pubky-sdk/bindings/js/src/actors/auth_flow.rs b/pubky-sdk/bindings/js/src/actors/auth_flow.rs index e0a9fb451..c56e5baf8 100644 --- a/pubky-sdk/bindings/js/src/actors/auth_flow.rs +++ b/pubky-sdk/bindings/js/src/actors/auth_flow.rs @@ -121,6 +121,8 @@ impl AuthFlow { /// @returns {AuthFlow} A flow reconnected to the original relay channel. /// @throws {PubkyError} /// - `{ name: "AuthenticationError" }` if the URL is invalid or not a signin/signup link + /// + /// @deprecated Use `GrantAuthFlow.resume(...)` instead. #[wasm_bindgen(js_name = "resume")] pub fn resume(authorization_url: String) -> JsResult { Self::resume_with_client(authorization_url, None) diff --git a/pubky-sdk/bindings/js/src/actors/cookie_session.rs b/pubky-sdk/bindings/js/src/actors/cookie_session.rs index a84f5a536..daca6cb19 100644 --- a/pubky-sdk/bindings/js/src/actors/cookie_session.rs +++ b/pubky-sdk/bindings/js/src/actors/cookie_session.rs @@ -1,11 +1,17 @@ +#![allow( + deprecated, + reason = "JS bindings preserve deprecated cookie compatibility APIs" +)] + use wasm_bindgen::prelude::*; use crate::js_error::{JsResult, PubkyError, PubkyErrorName}; /// Cookie-only view over a cookie-backed `Session`. /// -/// Grant-backed sessions do not expose this view; use `session.cookie` and -/// check for `undefined` before calling cookie-specific methods. +/// New applications should use `session.grant` and `GrantSession`. +/// +/// @deprecated Use `GrantSession` instead. #[wasm_bindgen] pub struct CookieSession(pub(crate) pubky::PubkySession); @@ -17,6 +23,7 @@ impl CookieSession { /// still hold the HTTP-only cookie. /// /// @returns {string} + /// @deprecated Use `GrantSession.exportLocalSecret()` instead. pub fn export(&self) -> JsResult { let cookie = self.as_cookie()?; Ok(cookie.export()) @@ -28,6 +35,7 @@ impl CookieSession { /// Node.js. Browser sessions cannot read HTTP-only Set-Cookie values. /// /// @returns {Promise} + /// @deprecated Use `GrantSession.exportLocalSecret()` instead. #[wasm_bindgen(js_name = "exportSecret")] pub async fn export_secret(&self) -> JsResult { let cookie = self.as_cookie()?; diff --git a/pubky-sdk/bindings/js/src/actors/session.rs b/pubky-sdk/bindings/js/src/actors/session.rs index 47430365d..c2e4f2dbc 100644 --- a/pubky-sdk/bindings/js/src/actors/session.rs +++ b/pubky-sdk/bindings/js/src/actors/session.rs @@ -1,4 +1,9 @@ // js/src/wrappers/session.rs +#![allow( + deprecated, + reason = "JS bindings preserve deprecated cookie compatibility APIs" +)] + use wasm_bindgen::prelude::*; use super::{cookie_session::CookieSession, grant_session::GrantSession, storage::SessionStorage}; @@ -45,6 +50,7 @@ impl Session { /// Grant-backed sessions return `undefined`. /// /// @returns {CookieSession|undefined} + /// @deprecated Use `session.grant` and `GrantSession` instead. #[wasm_bindgen(getter)] pub fn cookie(&self) -> Option { self.0.as_cookie().map(|_| CookieSession(self.0.clone())) @@ -70,7 +76,7 @@ impl Session { /// @returns {string} /// A base64 string to store (e.g. in `localStorage`). /// - /// @deprecated Prefer `exportLocalSecret()` for grant sessions. + /// @deprecated Use `GrantSession.exportLocalSecret()` instead. #[wasm_bindgen] pub fn export(&self) -> String { self.0 @@ -128,7 +134,7 @@ impl Session { /// Optional client to reuse transport configuration. /// @returns {Promise} /// - /// @deprecated Prefer `Pubky.restoreSession(...)`. + /// @deprecated Use `Pubky.restoreSession(...)` with a grant secret. #[wasm_bindgen(js_name = "restore")] pub async fn restore(exported: String, client: Option) -> JsResult { let session = match client { diff --git a/pubky-sdk/bindings/js/src/actors/signer.rs b/pubky-sdk/bindings/js/src/actors/signer.rs index 084a50e20..929d56865 100644 --- a/pubky-sdk/bindings/js/src/actors/signer.rs +++ b/pubky-sdk/bindings/js/src/actors/signer.rs @@ -1,3 +1,8 @@ +#![allow( + deprecated, + reason = "JS bindings preserve deprecated cookie compatibility APIs" +)] + use wasm_bindgen::prelude::*; use super::{pkdns::Pkdns, session::Session}; diff --git a/pubky-sdk/bindings/js/src/client/http.rs b/pubky-sdk/bindings/js/src/client/http.rs index edefe2aa9..e7ebb45d1 100644 --- a/pubky-sdk/bindings/js/src/client/http.rs +++ b/pubky-sdk/bindings/js/src/client/http.rs @@ -21,7 +21,7 @@ impl Client { /// /// @example /// const client = pubky.client; - /// const res = await client.fetch(`https://_pubky.${user}/pub/app/file.txt`, { method: "PUT", body: "hi", credentials: "include" }); + /// const res = await client.fetch(`https://_pubky.${user}/storage/${user}/pub/app/file.txt`, { method: "PUT", body: "hi", credentials: "include" }); pub async fn fetch(&self, url: &str, init: Option) -> JsResult { // 1) Parse URL let mut url = Url::parse(url)?; @@ -43,7 +43,9 @@ impl Client { .unwrap_or_default(); // 3a) If needed, ensure `pubky-host` is present in *init.headers* BEFORE Request creation. - if let Some(host) = pubky_host.as_deref() { + if let Some(host) = pubky_host.as_deref() + && !url.path().starts_with("/storage/") + { // Try to read any existing headers off RequestInit via reflection. // This value can be: undefined/null (no headers), a real `Headers`, or // a plain object/array. We handle those cases explicitly. diff --git a/pubky-sdk/bindings/js/src/pubky.rs b/pubky-sdk/bindings/js/src/pubky.rs index c669c75d1..c623f5a37 100644 --- a/pubky-sdk/bindings/js/src/pubky.rs +++ b/pubky-sdk/bindings/js/src/pubky.rs @@ -1,3 +1,8 @@ +#![allow( + deprecated, + reason = "JS bindings preserve deprecated cookie compatibility APIs" +)] + use wasm_bindgen::prelude::*; use crate::actors::{ @@ -101,6 +106,7 @@ impl Pubky { /// renderQr(flow.authorizationUrl); /// const session = await flow.awaitApproval(); /// + /// @deprecated Use `startGrantAuthFlow(...)` instead. #[wasm_bindgen(js_name = "startCookieAuthFlow")] pub fn start_cookie_auth_flow( &self, @@ -198,6 +204,8 @@ impl Pubky { /// sessionStorage.removeItem("pubky-auth-url"); /// } /// } + /// + /// @deprecated Use `resumeGrantAuthFlow(...)` instead. #[wasm_bindgen(js_name = "resumeCookieAuthFlow")] pub fn resume_cookie_auth_flow(&self, authorization_url: String) -> JsResult { AuthFlow::resume_with_client(authorization_url, Some(self.0.client().clone())) diff --git a/pubky-sdk/src/actors/auth/cookie/builder.rs b/pubky-sdk/src/actors/auth/cookie/builder.rs index 0e683af3b..439c014b6 100644 --- a/pubky-sdk/src/actors/auth/cookie/builder.rs +++ b/pubky-sdk/src/actors/auth/cookie/builder.rs @@ -20,6 +20,7 @@ use crate::{Capabilities, PubkyHttpClient}; /// exchanges for a session cookie. For long-lived, mirror-friendly /// sessions, prefer [`crate::PubkyGrantAuthFlow`]. #[derive(Debug, Clone)] +#[deprecated(note = "Use PubkyGrantAuthFlow::builder instead.")] pub struct CookieAuthFlowBuilder { caps: Capabilities, base_relay: Url, diff --git a/pubky-sdk/src/actors/auth/cookie/credential.rs b/pubky-sdk/src/actors/auth/cookie/credential.rs index b42e06714..fce7b4f27 100644 --- a/pubky-sdk/src/actors/auth/cookie/credential.rs +++ b/pubky-sdk/src/actors/auth/cookie/credential.rs @@ -34,7 +34,7 @@ use crate::actors::session::credential::{SessionCredential, credential_session_m use crate::{ Error, PubkyHttpClient, actors::session::SessionInfo, - actors::storage::resource::resolve_pubky, + client::user_endpoint_url, cross_log, errors::{PkarrError, Result}, util::check_http_status, @@ -52,6 +52,7 @@ const SESSION_PATH: &str = "/session"; /// inaccessible (the fetch spec hides `Set-Cookie`) and the browser cookie /// jar handles attachment automatically — `cookie` is `None`. #[derive(Clone, Debug)] +#[deprecated(note = "Use GrantCredential instead.")] pub struct CookieCredential { /// User public key — used to name the `Cookie` header. user: PublicKey, @@ -207,10 +208,6 @@ impl CookieCredential { } } -fn session_resource(user: &PublicKey) -> String { - format!("pubky{}{}", user.z32(), SESSION_PATH) -} - async fn session_request( client: &PubkyHttpClient, method: Method, @@ -223,8 +220,9 @@ async fn session_request( .await; } - let resolved = resolve_pubky(session_resource(user))?; - client.cross_request(method, resolved).await + client + .cross_request(method, user_endpoint_url(user, SESSION_PATH)?) + .await } /// Cross-target reader for `Set-Cookie` response header values. @@ -337,6 +335,7 @@ impl PubkySession { /// returns a credential you want to hold separately, this lifts it into /// a full session bound to the given HTTP client. #[must_use] + #[deprecated(note = "Use PubkySession::from_grant_credential instead.")] pub fn from_cookie_credential(client: PubkyHttpClient, credential: CookieCredential) -> Self { Self::from_credential(client, Arc::new(credential)) } diff --git a/pubky-sdk/src/actors/auth/cookie/legacy_api.rs b/pubky-sdk/src/actors/auth/cookie/legacy_api.rs index 4fcf6e3fa..c2096b977 100644 --- a/pubky-sdk/src/actors/auth/cookie/legacy_api.rs +++ b/pubky-sdk/src/actors/auth/cookie/legacy_api.rs @@ -18,7 +18,7 @@ impl PubkySession { /// # Panics /// Panics if the session is not cookie-backed. #[must_use] - #[deprecated(note = "Use `session.as_cookie().map(|cookie| cookie.export())` instead")] + #[deprecated(note = "Use GrantSessionView::export_local_secret instead.")] pub fn export(&self) -> String { self.as_cookie() .expect("export() is only valid for cookie sessions") @@ -32,6 +32,7 @@ impl PubkySession { /// # Errors /// - Returns [`crate::errors::RequestError::Validation`] if the export string is malformed. /// - On native, returns an error because exports are only supported on WASM. + #[deprecated(note = "Use Pubky::restore_session with a grant secret instead.")] pub async fn import(export: &str, client: Option) -> Result { super::secret::import_session(export, client).await } @@ -43,6 +44,7 @@ impl PubkySession { /// # Errors /// - Returns [`crate::errors::RequestError::Validation`] if the token is malformed. /// - Propagates transport failures while validating the session. + #[deprecated(note = "Use Pubky::restore_session with a grant secret instead.")] pub async fn import_secret(token: &str, client: Option) -> Result { super::secret::import_session_secret(token, client).await } @@ -55,6 +57,7 @@ impl PubkySession { /// - Returns [`crate::errors::RequestError::Validation`] when the file extension is not `.sess`. /// - Propagates errors from the stored token validation. #[cfg(not(target_arch = "wasm32"))] + #[deprecated(note = "Read the grant secret and pass it to Pubky::restore_session instead.")] pub async fn from_secret_file( path: &std::path::Path, client: Option, @@ -74,7 +77,7 @@ impl PubkySession { /// permissions cannot be set. #[cfg(not(target_arch = "wasm32"))] #[deprecated( - note = "Use `session.as_cookie().map(|cookie| cookie.write_secret_file(path))` instead" + note = "Use GrantSessionView::export_local_secret and persist that grant secret instead." )] pub fn write_secret_file>( &self, diff --git a/pubky-sdk/src/actors/auth/cookie/mod.rs b/pubky-sdk/src/actors/auth/cookie/mod.rs index 96ae4b442..66d6452b1 100644 --- a/pubky-sdk/src/actors/auth/cookie/mod.rs +++ b/pubky-sdk/src/actors/auth/cookie/mod.rs @@ -4,6 +4,11 @@ //! Cookie-backed sessions lack the self-refreshing, mirror-friendly properties //! of grant-backed sessions. +#![allow( + deprecated, + reason = "This module implements the deprecated cookie compatibility API" +)] + pub(crate) mod approval; pub(crate) mod builder; pub(crate) mod credential; diff --git a/pubky-sdk/src/actors/auth/cookie/view.rs b/pubky-sdk/src/actors/auth/cookie/view.rs index 966b5075a..4232abd5c 100644 --- a/pubky-sdk/src/actors/auth/cookie/view.rs +++ b/pubky-sdk/src/actors/auth/cookie/view.rs @@ -18,6 +18,7 @@ use crate::actors::session::core::PubkySession; /// Cookie-only operations on a [`PubkySession`]. #[derive(Debug)] +#[deprecated(note = "Use GrantSessionView instead.")] pub struct CookieSessionView<'a> { session: &'a PubkySession, credential: &'a CookieCredential, @@ -31,6 +32,7 @@ impl PubkySession { /// runtime is whether the cookie secret is *capturable*: see /// [`CookieSessionView::export_secret`]. #[must_use] + #[deprecated(note = "Use PubkySession::as_grant instead.")] pub fn as_cookie(&self) -> Option> { self.try_downcast_credential::() .map(|c| CookieSessionView::new(self, c)) @@ -53,6 +55,7 @@ impl<'a> CookieSessionView<'a> { /// [`PubkySession::info`](crate::actors::session::core::PubkySession::info) /// accessor. #[must_use] + #[deprecated(note = "Use GrantSessionView::session_info instead.")] pub fn session_info(&self) -> CookieSessionRecord { self.credential.cookie_record() } @@ -64,6 +67,7 @@ impl<'a> CookieSessionView<'a> { /// HTTP-only session cookie; `export()` merely captures the metadata needed to /// reconstruct a `PubkySession` handle. #[must_use] + #[deprecated(note = "Use GrantSessionView::export_local_secret instead.")] pub fn export(&self) -> String { let record = self.session_info(); crate::cross_log!(info, "Exporting session for {}", record.public_key()); @@ -86,6 +90,7 @@ impl<'a> CookieSessionView<'a> { /// JavaScript by the WHATWG fetch spec — only the browser cookie /// jar holds the value. #[must_use] + #[deprecated(note = "Use GrantSessionView::export_local_secret instead.")] pub fn export_secret(&self) -> Option { let public_key = self.session.info().public_key().z32(); let cookie = self.credential.cookie_secret()?; @@ -108,6 +113,9 @@ impl<'a> CookieSessionView<'a> { /// permissions cannot be set. On native the secret is always /// present, so this never errors with `NotFound` for that reason. #[cfg(not(target_arch = "wasm32"))] + #[deprecated( + note = "Use GrantSessionView::export_local_secret and persist that grant secret instead." + )] pub fn write_secret_file>( &self, secret_file_path: P, diff --git a/pubky-sdk/src/actors/auth/grant/grant_exchange.rs b/pubky-sdk/src/actors/auth/grant/grant_exchange.rs index 245534739..4a6744ad7 100644 --- a/pubky-sdk/src/actors/auth/grant/grant_exchange.rs +++ b/pubky-sdk/src/actors/auth/grant/grant_exchange.rs @@ -19,7 +19,6 @@ use super::{ credential::{GrantCredential, sign_pop_for_grant}, pop_signer::GrantPopSigner, }; -use crate::actors::storage::resource::resolve_pubky; use crate::errors::{RequestError, Result}; use crate::util::check_http_status; use crate::{PubkyHttpClient, cross_log}; @@ -106,10 +105,13 @@ async fn post_grant_session( let pop_jws = sign_pop_for_grant(client_signer, homeserver_pk, &grant_claims.jti).await?; let body = serde_json::json!({ "grant": grant_jws, "pop": pop_jws }); - let url = format!("pubky://{}/auth/grant/session", grant_claims.iss.z32()); - let resolved = resolve_pubky(&url)?; let resp = client - .cross_request(Method::POST, resolved) + .cross_request_via_homeserver( + Method::POST, + homeserver_pk, + &grant_claims.iss, + "/auth/grant/session", + ) .await? .json(&body) .send() diff --git a/pubky-sdk/src/actors/auth/grant/manager.rs b/pubky-sdk/src/actors/auth/grant/manager.rs index f06f06425..01431a5ef 100644 --- a/pubky-sdk/src/actors/auth/grant/manager.rs +++ b/pubky-sdk/src/actors/auth/grant/manager.rs @@ -10,7 +10,7 @@ use reqwest::Method; use std::sync::Arc; use crate::actors::session::credential::SessionCredential; -use crate::actors::storage::resource::resolve_pubky; +use crate::client::user_endpoint_url; use crate::errors::{RequestError, Result}; use crate::util::check_http_status; use crate::{PubkyHttpClient, PubkySession}; @@ -68,9 +68,8 @@ impl GrantManager { /// - Propagates HTTP errors from the homeserver (`401`/`403` for invalid /// auth or missing root capability). pub async fn list(&self) -> Result> { - let url = format!("pubky://{}/auth/grant/sessions", self.user.z32()); - let resolved = resolve_pubky(&url)?; - let rb = self.client.cross_request(Method::GET, resolved).await?; + let url = user_endpoint_url(&self.user, "/auth/grant/sessions")?; + let rb = self.client.cross_request(Method::GET, url).await?; let resp = self .credential .attach(rb, &self.client) @@ -93,13 +92,9 @@ impl GrantManager { /// - Propagates HTTP errors from the homeserver (`401`/`403` for invalid /// auth or missing root capability). pub async fn revoke(&self, grant_id: &GrantId) -> Result<()> { - let url = format!( - "pubky://{}/auth/grant/session/{}", - self.user.z32(), - grant_id.as_str() - ); - let resolved = resolve_pubky(&url)?; - let rb = self.client.cross_request(Method::DELETE, resolved).await?; + let path = format!("/auth/grant/session/{}", grant_id.as_str()); + let url = user_endpoint_url(&self.user, &path)?; + let rb = self.client.cross_request(Method::DELETE, url).await?; let resp = self .credential .attach(rb, &self.client) diff --git a/pubky-sdk/src/actors/event_stream.rs b/pubky-sdk/src/actors/event_stream.rs index 650804257..3efcef929 100644 --- a/pubky-sdk/src/actors/event_stream.rs +++ b/pubky-sdk/src/actors/event_stream.rs @@ -651,6 +651,10 @@ fn decode_content_hash(content_hash_base64: Option<&str>) -> Result { } #[cfg(test)] +#[allow( + deprecated, + reason = "These tests verify legacy cookie session event-stream compatibility" +)] mod tests { use super::*; use crate::actors::auth::cookie::credential::CookieCredential; diff --git a/pubky-sdk/src/actors/mod.rs b/pubky-sdk/src/actors/mod.rs index 4aa3a53ea..a7150478d 100644 --- a/pubky-sdk/src/actors/mod.rs +++ b/pubky-sdk/src/actors/mod.rs @@ -8,6 +8,7 @@ pub mod storage; pub use auth::AuthFlowKind; #[allow(deprecated, reason = "Re-exporting deprecated public API")] pub use auth::cookie::PubkyCookieAuthFlow; +#[allow(deprecated, reason = "Re-exporting deprecated public API")] pub use auth::cookie::{CookieCredential, CookieSessionView}; pub use auth::deep_links; #[doc(hidden)] diff --git a/pubky-sdk/src/actors/signer/session.rs b/pubky-sdk/src/actors/signer/session.rs index 84ae694d3..932f1e79a 100644 --- a/pubky-sdk/src/actors/signer/session.rs +++ b/pubky-sdk/src/actors/signer/session.rs @@ -1,3 +1,8 @@ +#![allow( + deprecated, + reason = "This module preserves deprecated cookie signer compatibility methods" +)] + use std::sync::Arc; use pubky_common::auth::{ @@ -156,6 +161,7 @@ impl PubkySigner { /// - Returns [`crate::errors::Error::Parse`] if the homeserver URL cannot be constructed. /// - Propagates transport failures while creating the account or publishing the homeserver record. /// - Propagates validation errors while hydrating the cookie session. + #[deprecated(note = "Use PubkySigner::signup followed by PubkySigner::signin instead.")] pub async fn signup_cookie( &self, homeserver: &PublicKey, @@ -181,6 +187,7 @@ impl PubkySigner { /// # Errors /// - Propagates transport failures during the session exchange. /// - Propagates validation errors while creating the cookie credential. + #[deprecated(note = "Use PubkySigner::signin instead.")] pub async fn signin_cookie(&self) -> Result { self.signin_cookie_with_publish(PublishMode::Background) .await @@ -191,6 +198,7 @@ impl PubkySigner { /// # Errors /// - Propagates transport failures during the session exchange. /// - Propagates failures while refreshing the homeserver record. + #[deprecated(note = "Use PubkySigner::signin_blocking instead.")] pub async fn signin_cookie_blocking(&self) -> Result { self.signin_cookie_with_publish(PublishMode::Blocking).await } diff --git a/pubky-sdk/src/actors/storage/resource.rs b/pubky-sdk/src/actors/storage/resource.rs index 2eb764371..4cda934b7 100644 --- a/pubky-sdk/src/actors/storage/resource.rs +++ b/pubky-sdk/src/actors/storage/resource.rs @@ -21,6 +21,7 @@ use std::{fmt, str::FromStr}; use crate::PublicKey; +use percent_encoding::percent_decode_str; use url::Url; use crate::{Error, errors::RequestError}; @@ -210,7 +211,7 @@ impl PubkyResource { format!("pubky://{}/{}", self.owner.z32(), rel) } - /// Render as `https://_pubky./` for transport. + /// Render as `https://_pubky./storage//` for transport. /// /// This converts the addressed resource into the actual homeserver URL used /// by the transport layer. It is the same mapping performed by @@ -220,39 +221,35 @@ impl PubkyResource { /// - Returns [`Error::Request`] if the constructed transport URL is invalid. pub fn to_transport_url(&self) -> Result { let rel = self.path.as_str().trim_start_matches('/'); - let https = format!("https://_pubky.{}/{}", self.owner.z32(), rel); + let owner = self.owner.z32(); + let https = format!("https://_pubky.{owner}/storage/{owner}/{rel}"); Ok(Url::parse(&https)?) } - /// Construct a [`PubkyResource`] from a homeserver transport URL. - /// - /// Accepts either `https://_pubky./...` or `http://_pubky./...` - /// (the latter is mainly useful in local testnets). + /// Parse canonical `/storage//...` and legacy `_pubky./...` URLs. + /// Canonical URLs take the owner from the path; legacy URLs use the host. /// /// # Errors - /// - Returns [`Error::Request`] if the URL is missing the expected `_pubky.` host. - /// - Returns [`Error::Request`] if the host does not contain a valid public key. + /// Returns [`Error::Request`] when the owner or path is malformed. pub fn from_transport_url(url: &Url) -> Result { - let host = url - .host_str() - .ok_or_else(|| invalid("transport URL missing host"))?; - let owner = host - .strip_prefix("_pubky.") - .ok_or_else(|| invalid("transport URL host must start with '_pubky.'"))?; - if PublicKey::is_pubky_prefixed(owner) { - return Err(invalid( - "transport URL host must use raw z32 without `pubky` prefix", - )); - } - let public_key = PublicKey::try_from_z32(owner) - .map_err(|_err| invalid("transport URL host does not contain a valid public key"))?; - - let path = if url.path().is_empty() { - "/" + let (owner, path) = if let Some(path) = url.path().strip_prefix("/storage/") { + path.split_once('/') + .ok_or_else(|| invalid("storage transport URL is missing a resource path"))? } else { - url.path() + let owner = url + .host_str() + .and_then(|host| host.strip_prefix("_pubky.")) + .ok_or_else(|| invalid("legacy transport URL is missing a `_pubky` host"))?; + (owner, url.path()) }; - Self::new(public_key, path) + + let owner = PublicKey::try_from_z32(owner) + .map_err(|_err| invalid("transport URL contains an invalid owner"))?; + let path = if path.is_empty() { "/" } else { path }; + let decoded_path = percent_decode_str(path) + .decode_utf8() + .map_err(|_err| invalid("transport URL path is not valid UTF-8"))?; + Self::new(owner, decoded_path.as_ref()) } /// Render as the identifier form `pubky/`. @@ -576,11 +573,12 @@ mod tests { fn resolve_identifiers() { let kp = Keypair::random(); let user = kp.public_key(); + let user_raw = user.z32(); let base = format!("pubky://{}/pub/site/index.html", user.z32()); let resolved = resolve_pubky(&base).unwrap(); assert_eq!( resolved.as_str(), - format!("https://_pubky.{}/pub/site/index.html", user.z32()) + format!("https://_pubky.{user_raw}/storage/{user_raw}/pub/site/index.html") ); let prefixed = format!("pubky{}/pub/site/index.html", user.z32()); @@ -593,9 +591,42 @@ mod tests { let parsed = PubkyResource::from_transport_url(&resolved).unwrap(); assert_eq!(parsed, resource); + let homeserver = Keypair::random().public_key(); + let explicit_homeserver = Url::parse(&format!( + "https://{}/storage/{user_raw}/pub/site/index.html", + homeserver.z32() + )) + .unwrap(); + assert_eq!( + PubkyResource::from_transport_url(&explicit_homeserver).unwrap(), + resource + ); + + let legacy = Url::parse(&format!("https://_pubky.{user_raw}/pub/site/index.html")).unwrap(); + assert_eq!( + PubkyResource::from_transport_url(&legacy).unwrap(), + resource + ); + let http_url = Url::parse(&format!("http://_pubky.{}/pub/site/index.html", user.z32())).unwrap(); let parsed_http = PubkyResource::from_transport_url(&http_url).unwrap(); assert_eq!(parsed_http, resource); } + + #[test] + fn canonical_transport_path_owner_is_authoritative() { + let host_owner = Keypair::random().public_key(); + let path_owner = Keypair::random().public_key(); + let url = Url::parse(&format!( + "https://_pubky.{}/storage/{}/pub/My%20File.txt", + host_owner.z32(), + path_owner.z32() + )) + .unwrap(); + + let resource = PubkyResource::from_transport_url(&url).unwrap(); + assert_eq!(resource.owner, path_owner); + assert_eq!(resource.path.as_str(), "/pub/My%20File.txt"); + } } diff --git a/pubky-sdk/src/client/core.rs b/pubky-sdk/src/client/core.rs index 18b410847..7ffb26111 100644 --- a/pubky-sdk/src/client/core.rs +++ b/pubky-sdk/src/client/core.rs @@ -406,14 +406,14 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// let client = PubkyHttpClient::new()?; /// // Pubky App profile of user Pubky https://pubky.app/profile/ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y /// let user = "ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y"; -/// let url = format!("https://_pubky.{user}/pub/pubky.app/profile.json"); +/// let url = format!("https://_pubky.{user}/storage/{user}/pub/pubky.app/profile.json"); /// let resp = client.request(Method::GET, &url).send().await?; /// let info = resp.text().await?; /// # Ok(()) } /// ``` /// /// > Tip: For authenticated reads/writes, prefer `session.storage().get(...)`, which -/// > automatically scopes paths and attaches the right session cookie. +/// > automatically scopes paths and attaches the right session credential. #[derive(Clone, Debug)] pub struct PubkyHttpClient { pub(crate) http: reqwest::Client, diff --git a/pubky-sdk/src/client/http_targets/mod.rs b/pubky-sdk/src/client/http_targets/mod.rs index 6cce63399..8349d5d60 100644 --- a/pubky-sdk/src/client/http_targets/mod.rs +++ b/pubky-sdk/src/client/http_targets/mod.rs @@ -18,3 +18,49 @@ fn homeserver_url(homeserver: &PublicKey, path: &str) -> Result { path ))?) } + +pub(crate) fn user_endpoint_url(user: &PublicKey, path: &str) -> Result { + let path = if path.starts_with('/') { + path.to_string() + } else { + format!("/{path}") + }; + Ok(Url::parse(&format!( + "https://_pubky.{}{}", + user.z32(), + path + ))?) +} + +#[inline] +fn is_path_addressed_storage(url: &Url) -> bool { + url.path().starts_with("/storage/") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_only_path_addressed_storage_routes() { + assert!(is_path_addressed_storage( + &Url::parse("https://example.com/storage/user/pub/file.txt").unwrap() + )); + assert!(!is_path_addressed_storage( + &Url::parse("https://example.com/storage-user/pub/file.txt").unwrap() + )); + assert!(!is_path_addressed_storage( + &Url::parse("https://example.com/session").unwrap() + )); + } + + #[test] + fn user_endpoints_keep_authority_addressing() { + let user = crate::Keypair::random().public_key(); + let url = user_endpoint_url(&user, "/auth/grant/session").unwrap(); + let expected_host = format!("_pubky.{}", user.z32()); + + assert_eq!(url.host_str(), Some(expected_host.as_str())); + assert_eq!(url.path(), "/auth/grant/session"); + } +} diff --git a/pubky-sdk/src/client/http_targets/native.rs b/pubky-sdk/src/client/http_targets/native.rs index 3f11298bd..e699da269 100644 --- a/pubky-sdk/src/client/http_targets/native.rs +++ b/pubky-sdk/src/client/http_targets/native.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, PoisonError, RwLock}; use std::time::{Duration, Instant}; -use super::homeserver_url; +use super::{homeserver_url, is_path_addressed_storage}; use futures_util::StreamExt; use tokio::net::TcpStream; @@ -201,10 +201,14 @@ impl PubkyHttpClient { let transport = self.transport.resolve(&homeserver_z32, &self.pkarr).await; match transport { - ResolvedTransport::PubkyTls => Ok(self - .http - .request(method, url.as_str()) - .header("pubky-host", pubky_host_z32)), + ResolvedTransport::PubkyTls => { + let request = self.http.request(method, url.as_str()); + if is_path_addressed_storage(&url) { + Ok(request) + } else { + Ok(request.header("pubky-host", pubky_host_z32)) + } + } ResolvedTransport::Icann { .. } => { self.build_pubky_request(method, &url, &pubky_host_z32, &transport) } @@ -230,10 +234,12 @@ impl PubkyHttpClient { .map_err(|_err| url::ParseError::InvalidPort)?; } cross_log!(debug, "ICANN fallback for {pk} via {domain}"); - Ok(self - .icann_http - .request(method, icann_url.as_str()) - .header("pubky-host", pk)) + let request = self.icann_http.request(method, icann_url.as_str()); + if is_path_addressed_storage(url) { + Ok(request) + } else { + Ok(request.header("pubky-host", pk)) + } } } } @@ -383,6 +389,35 @@ mod tests { assert_eq!(req.headers().get("pubky-host").unwrap(), z32); } + #[test] + fn build_pubky_storage_request_icann_retains_owner_path_without_header() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let z32 = "o4dksfbqk85ogzdb5osziw6befigbuxmuxkuxq8434q89uj56uyy"; + let url = Url::parse(&format!( + "https://_pubky.{z32}/storage/{z32}/pub/app/file.txt?cursor=hello%20world" + )) + .unwrap(); + let transport = ResolvedTransport::Icann { + domain: "example.com".to_string(), + port: Some(8443), + }; + + let req = client + .build_pubky_request(Method::GET, &url, z32, &transport) + .unwrap() + .build() + .unwrap(); + + assert_eq!(req.url().host_str(), Some("example.com")); + assert_eq!(req.url().port(), Some(8443)); + assert_eq!(req.url().path(), format!("/storage/{z32}/pub/app/file.txt")); + assert_eq!(req.url().query(), Some("cursor=hello%20world")); + assert!(!req.headers().contains_key("pubky-host")); + } + #[tokio::test] async fn resolve_transport_direct_only() { let kp = Keypair::random(); diff --git a/pubky-sdk/src/client/http_targets/wasm.rs b/pubky-sdk/src/client/http_targets/wasm.rs index f3f670767..51d3a770a 100644 --- a/pubky-sdk/src/client/http_targets/wasm.rs +++ b/pubky-sdk/src/client/http_targets/wasm.rs @@ -1,6 +1,6 @@ //! HTTP methods that support `https://` with Pkarr domains, including `_pubky.` URLs -use super::homeserver_url; +use super::{homeserver_url, is_path_addressed_storage}; use crate::PublicKey; use crate::errors::{PkarrError, RequestError, Result}; use crate::{PubkyHttpClient, cross_log}; @@ -17,6 +17,20 @@ enum AmbientCredentials { } impl PubkyHttpClient { + fn attach_pubky_host( + request: RequestBuilder, + url: &Url, + pubky_host: Option, + ) -> RequestBuilder { + if let Some(pubky_host) = pubky_host + && !is_path_addressed_storage(url) + { + request.header("pubky-host", pubky_host) + } else { + request + } + } + /// A wrapper around [`PubkyHttpClient::request`], with the same signature between native and WASM. pub(crate) async fn cross_request( &self, @@ -48,11 +62,15 @@ impl PubkyHttpClient { let mut url = homeserver_url(homeserver, path)?; self.prepare_request(&mut url).await?; - Ok(self + let request = self .http - .request(method, url) - .fetch_credentials_include() - .header("pubky-host", pubky_host.z32())) + .request(method, url.clone()) + .fetch_credentials_include(); + Ok(Self::attach_pubky_host( + request, + &url, + Some(pubky_host.z32()), + )) } async fn cross_request_with_credentials( @@ -72,13 +90,7 @@ impl PubkyHttpClient { AmbientCredentials::Omit => request.fetch_credentials_omit(), }; - let builder = if let Some(pubky_host) = pubky_host { - builder.header("pubky-host", pubky_host) - } else { - builder - }; - - Ok(builder) + Ok(Self::attach_pubky_host(builder, &url, pubky_host)) } /// - Resolves a clearnet host to call with fetch @@ -231,6 +243,43 @@ mod tests { wasm_bindgen_test_configure!(run_in_browser); + #[wasm_bindgen_test] + fn storage_request_preserves_path_and_query_without_pubky_host() { + let client = PubkyHttpClient::new().unwrap(); + let owner = Keypair::random().public_key().z32(); + let url = Url::parse(&format!( + "https://example.com/storage/{owner}/pub/file.txt?cursor=hello%20world" + )) + .unwrap(); + let request = PubkyHttpClient::attach_pubky_host( + client.http.request(Method::GET, url.clone()), + &url, + Some(owner), + ) + .build() + .unwrap(); + + assert!(request.headers().get("pubky-host").is_none()); + assert!(request.url().path().starts_with("/storage/")); + assert_eq!(request.url().query(), Some("cursor=hello%20world")); + } + + #[wasm_bindgen_test] + fn cookie_session_request_keeps_pubky_host() { + let client = PubkyHttpClient::new().unwrap(); + let owner = Keypair::random().public_key().z32(); + let url = Url::parse("https://example.com/session").unwrap(); + let request = PubkyHttpClient::attach_pubky_host( + client.http.request(Method::POST, url.clone()), + &url, + Some(owner.clone()), + ) + .build() + .unwrap(); + + assert_eq!(request.headers().get("pubky-host").unwrap(), &owner); + } + #[wasm_bindgen_test(async)] async fn transform_url_errors_when_no_domain_is_found() { let client = PubkyHttpClient::new().unwrap(); diff --git a/pubky-sdk/src/client/mod.rs b/pubky-sdk/src/client/mod.rs index 57d03b599..15b262d9e 100644 --- a/pubky-sdk/src/client/mod.rs +++ b/pubky-sdk/src/client/mod.rs @@ -1,2 +1,4 @@ pub mod core; mod http_targets; + +pub(crate) use http_targets::user_endpoint_url; diff --git a/pubky-sdk/src/lib.rs b/pubky-sdk/src/lib.rs index 6d7290075..6f78b713c 100644 --- a/pubky-sdk/src/lib.rs +++ b/pubky-sdk/src/lib.rs @@ -46,6 +46,7 @@ pub use actors::SessionInfo; #[doc(inline)] pub use actors::deep_links; #[doc(inline)] +#[allow(deprecated, reason = "Re-exporting deprecated public API")] pub use actors::{ CookieCredential, CookieSessionView, DelegatedGrantCredentialState, GrantCredential, GrantManager, GrantSessionView, @@ -85,6 +86,9 @@ pub use pkarr; // Re-exports #[doc(inline)] +#[deprecated(note = "Use SessionInfo or GrantSessionInfo instead.")] +pub use pubky_common::session::CookieSessionRecord; +#[doc(inline)] pub use pubky_common::{ StoragePath, StoragePathError, auth::{ @@ -97,7 +101,6 @@ pub use pubky_common::{ capabilities::{Capabilities, Capability}, crypto::{Keypair, PublicKey}, recovery_file, - session::CookieSessionRecord, }; pub use reqwest::{Method, StatusCode}; diff --git a/pubky-sdk/src/pubky.rs b/pubky-sdk/src/pubky.rs index 202d98aa3..b8fbf0167 100644 --- a/pubky-sdk/src/pubky.rs +++ b/pubky-sdk/src/pubky.rs @@ -9,7 +9,7 @@ //! ## Quick starts //! ### 1) App sign-in via QR/deeplink (auth flow) //! ```no_run -//! use pubky::{Pubky, Capabilities, AuthFlowKind}; +//! use pubky::{AuthFlowKind, Capabilities, ClientId, Pubky}; //! //! # async fn run() -> pubky::Result<()> { //! let pubky = Pubky::new()?; // or Pubky::testnet() / Pubky::with_client(...) @@ -18,7 +18,11 @@ //! .write("/pub/demoapp/") //! .expect("static scope is canonical") //! .finish(); -//! let flow = pubky.start_cookie_auth_flow(&caps, AuthFlowKind::signin())?; +//! let flow = pubky.start_grant_auth_flow( +//! &caps, +//! AuthFlowKind::signin(), +//! ClientId::new("demo.app").unwrap(), +//! )?; //! println!("Scan to sign in: {}", flow.authorization_url()); //! //! let session = flow.await_approval().await?; @@ -56,6 +60,10 @@ use std::str::FromStr; use crate::PublicKey; +use crate::actors::auth::cookie::secret::import_session_secret; + +#[cfg(not(target_arch = "wasm32"))] +use crate::actors::auth::cookie::secret::session_from_secret_file; #[allow(deprecated, reason = "Internal use of deprecated public API")] use crate::PubkyCookieAuthFlow; @@ -173,6 +181,7 @@ impl Pubky { deprecated, reason = "Cookie flow is intentionally exposed via this facade while deprecated" )] + #[deprecated(note = "Use Pubky::start_grant_auth_flow instead.")] pub fn start_cookie_auth_flow( &self, caps: &Capabilities, @@ -226,6 +235,7 @@ impl Pubky { deprecated, reason = "Cookie flow is intentionally exposed via this facade while deprecated" )] + #[deprecated(note = "Use PubkyGrantAuthFlow::restore with GrantAuthFlowState instead.")] pub fn resume_cookie_auth_flow(&self, authorization_url: &str) -> Result { let (caps, relay, secret, auth_kind, x_callback) = parse_auth_deep_link(authorization_url)?; @@ -359,8 +369,9 @@ impl Pubky { /// - Propagates transport errors from [`PubkySession::from_secret_file`] if the client /// cannot be prepared. #[cfg(not(target_arch = "wasm32"))] + #[deprecated(note = "Read the grant secret and pass it to Pubky::restore_session instead.")] pub async fn session_from_file>(&self, path: P) -> Result { - PubkySession::from_secret_file(path.as_ref(), Some(self.client.clone())).await + session_from_secret_file(path.as_ref(), Some(self.client.clone())).await } /// Restore a session from an exported session secret token. @@ -384,7 +395,7 @@ impl Pubky { return PubkySession::import_grant_secret(token, Some(self.client.clone())).await; } - PubkySession::import_secret(token, Some(self.client.clone())).await + import_session_secret(token, Some(self.client.clone())).await } /// Restore an origin-bound delegated browser grant session. From 4228f4d4064e20fe2fce7284a54cf895a767a4af Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Mon, 10 Aug 2026 17:29:27 -0300 Subject: [PATCH 02/10] test: added regression check --- pubky-common/src/auth/jws.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pubky-common/src/auth/jws.rs b/pubky-common/src/auth/jws.rs index 82a0a8a21..4c4a7ab70 100644 --- a/pubky-common/src/auth/jws.rs +++ b/pubky-common/src/auth/jws.rs @@ -114,7 +114,7 @@ impl RandomId { /// Parse and validate an existing ID string. /// - /// Must be non-empty and at most 22 characters. + /// Must be non-empty, at most 22 characters, and contain only base64url characters. pub fn parse(s: &str) -> Result { if s.is_empty() { return Err(Error::InvalidFormat("RandomId must not be empty")); @@ -124,6 +124,14 @@ impl RandomId { "RandomId must be at most 22 characters", )); } + if !s + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_')) + { + return Err(Error::InvalidFormat( + "RandomId must contain only base64url characters", + )); + } Ok(Self(s.to_string())) } @@ -262,9 +270,27 @@ mod tests { #[test] fn random_id_parse_valid() { RandomId::parse("abc123").unwrap(); + RandomId::parse("AZaz09-_").unwrap(); RandomId::parse("a").unwrap(); // min length } + #[test] + fn random_id_parse_rejects_non_base64url_characters() { + for value in [ + ".", + "..", + "../../../pub/a.txt", + "a/b", + "a?b", + "a#b", + "a+b", + "a=b", + "a b", + ] { + assert!(RandomId::parse(value).is_err(), "accepted {value:?}"); + } + } + #[test] fn random_id_parse_rejects_empty() { assert!(RandomId::parse("").is_err()); From 9ff374437fb0725f9090f75f4bb5822d451c9efc Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Tue, 11 Aug 2026 09:58:29 -0300 Subject: [PATCH 03/10] fix: minor fixes from feedback --- pubky-sdk/src/actors/storage/resource.rs | 26 ++++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/pubky-sdk/src/actors/storage/resource.rs b/pubky-sdk/src/actors/storage/resource.rs index 4cda934b7..87e3760db 100644 --- a/pubky-sdk/src/actors/storage/resource.rs +++ b/pubky-sdk/src/actors/storage/resource.rs @@ -134,6 +134,11 @@ impl ResourcePath { pub fn as_str(&self) -> &str { &self.0 } + + #[inline] + fn as_root_relative_str(&self) -> &str { + self.as_str().trim_start_matches('/') + } } impl FromStr for ResourcePath { @@ -207,22 +212,21 @@ impl PubkyResource { /// is always normalized. #[must_use] pub fn to_pubky_url(&self) -> String { - let rel = self.path.as_str().trim_start_matches('/'); - format!("pubky://{}/{}", self.owner.z32(), rel) + let path = self.path.as_root_relative_str(); + format!("pubky://{}/{path}", self.owner.z32()) } - /// Render as `https://_pubky./storage//` for transport. - /// - /// This converts the addressed resource into the actual homeserver URL used - /// by the transport layer. It is the same mapping performed by - /// [`resolve_pubky`]. + /// Render as the canonical path-addressed transport URL: + /// `https://_pubky./storage//`. + /// + /// It is the same mapping performed by [`resolve_pubky`]. /// /// # Errors /// - Returns [`Error::Request`] if the constructed transport URL is invalid. pub fn to_transport_url(&self) -> Result { - let rel = self.path.as_str().trim_start_matches('/'); let owner = self.owner.z32(); - let https = format!("https://_pubky.{owner}/storage/{owner}/{rel}"); + let path = self.path.as_root_relative_str(); + let https = format!("https://_pubky.{owner}/storage/{owner}/{path}"); Ok(Url::parse(&https)?) } @@ -254,8 +258,8 @@ impl PubkyResource { /// Render as the identifier form `pubky/`. pub(crate) fn to_identifier(&self) -> String { - let rel = self.path.as_str().trim_start_matches('/'); - format!("{}/{}", self.owner, rel) + let path = self.path.as_root_relative_str(); + format!("{}/{path}", self.owner) } } From 28a5e5ab1ac7a03aa8c54aa7fda5c87071fb1284 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Tue, 11 Aug 2026 10:03:10 -0300 Subject: [PATCH 04/10] fmt --- pubky-sdk/src/actors/storage/resource.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pubky-sdk/src/actors/storage/resource.rs b/pubky-sdk/src/actors/storage/resource.rs index 87e3760db..c150056d0 100644 --- a/pubky-sdk/src/actors/storage/resource.rs +++ b/pubky-sdk/src/actors/storage/resource.rs @@ -218,7 +218,7 @@ impl PubkyResource { /// Render as the canonical path-addressed transport URL: /// `https://_pubky./storage//`. - /// + /// /// It is the same mapping performed by [`resolve_pubky`]. /// /// # Errors From e50ce82e58d02b30b87ef95be8b600973eae4d7a Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Thu, 13 Aug 2026 11:46:29 -0300 Subject: [PATCH 05/10] feat: use features info endpoiint to signalize the storage path --- pubky-common/src/constants.rs | 6 + pubky-homeserver/README.md | 4 +- pubky-homeserver/openapi-client.yml | 3 +- pubky-homeserver/src/client_server/app.rs | 6 +- .../src/client_server/routes/info.rs | 3 +- pubky-sdk/bindings/js/pkg/tests/http.ts | 77 ++++++- pubky-sdk/bindings/js/src/utils.rs | 2 +- pubky-sdk/src/client/core.rs | 3 + pubky-sdk/src/client/http_targets/features.rs | 178 +++++++++++++++ pubky-sdk/src/client/http_targets/mod.rs | 11 + pubky-sdk/src/client/http_targets/native.rs | 205 +++++++++++++----- pubky-sdk/src/client/http_targets/storage.rs | 115 ++++++++++ pubky-sdk/src/client/http_targets/wasm.rs | 28 ++- 13 files changed, 576 insertions(+), 65 deletions(-) create mode 100644 pubky-sdk/src/client/http_targets/features.rs create mode 100644 pubky-sdk/src/client/http_targets/storage.rs diff --git a/pubky-common/src/constants.rs b/pubky-common/src/constants.rs index 7ce79c977..e13993738 100644 --- a/pubky-common/src/constants.rs +++ b/pubky-common/src/constants.rs @@ -16,6 +16,12 @@ pub mod storage { pub const PUBLIC_ROOT: &str = "/pub/"; } +/// Features advertised by the homeserver client API. +pub mod features { + /// Homeserver supports storage URLs containing the resource owner in the path. + pub const PATH_ADDRESSED_STORAGE: &str = "path-addressed-storage"; +} + /// Local test network's hardcoded port numbers for local development. pub mod testnet_ports { /// The local test network's hardcoded DHT bootstrapping node's port number. diff --git a/pubky-homeserver/README.md b/pubky-homeserver/README.md index 64a64cd15..9d189d2f3 100644 --- a/pubky-homeserver/README.md +++ b/pubky-homeserver/README.md @@ -18,8 +18,8 @@ See [config.sample.toml](config.sample.toml) for all configuration options. When an SDK change requires homeserver behavior that older versions do not support, such as a new endpoint, add a stable feature identifier to the client -`GET /info` response. SDKs must check that identifier before using the new -behavior. +`GET /info` response. SDKs must check it before using the new behavior and +ignore unknown identifiers. ## API Specifications diff --git a/pubky-homeserver/openapi-client.yml b/pubky-homeserver/openapi-client.yml index 04f09f9eb..feb6a1c4b 100644 --- a/pubky-homeserver/openapi-client.yml +++ b/pubky-homeserver/openapi-client.yml @@ -90,7 +90,8 @@ paths: schema: "$ref": "#/components/schemas/ClientInfoResponse" example: - features: [] + features: + - path-addressed-storage "/signup_tokens/{token}": get: tags: diff --git a/pubky-homeserver/src/client_server/app.rs b/pubky-homeserver/src/client_server/app.rs index c15429c1a..83a5bb961 100644 --- a/pubky-homeserver/src/client_server/app.rs +++ b/pubky-homeserver/src/client_server/app.rs @@ -312,7 +312,7 @@ mod tests { #[tokio::test] #[pubky_test_utils::test] - async fn info_is_public_and_reports_no_features() { + async fn info_is_public_and_reports_features() { let mut config = ConfigToml::minimal_test_config(); config.drive.rate_limits = vec![PathLimit { path: GlobPattern::new("/info"), @@ -333,7 +333,9 @@ mod tests { response.assert_status(StatusCode::OK); response.assert_header(header::CONTENT_TYPE, "application/json"); response.assert_header(header::CACHE_CONTROL, "no-store"); - response.assert_json(&serde_json::json!({ "features": [] })); + response.assert_json(&serde_json::json!({ + "features": ["path-addressed-storage"] + })); } async fn signup_cookie(server: &TestServer, keypair: &Keypair) -> String { diff --git a/pubky-homeserver/src/client_server/routes/info.rs b/pubky-homeserver/src/client_server/routes/info.rs index 47bbb96db..233ddd1ba 100644 --- a/pubky-homeserver/src/client_server/routes/info.rs +++ b/pubky-homeserver/src/client_server/routes/info.rs @@ -5,9 +5,10 @@ use axum::{ response::IntoResponse, Json, }; +use pubky_common::constants::features::PATH_ADDRESSED_STORAGE; use serde::Serialize; -const FEATURES: &[&str] = &[]; +const FEATURES: &[&str] = &[PATH_ADDRESSED_STORAGE]; #[derive(Serialize)] struct InfoResponse { diff --git a/pubky-sdk/bindings/js/pkg/tests/http.ts b/pubky-sdk/bindings/js/pkg/tests/http.ts index afd0c433b..ab1b8832d 100644 --- a/pubky-sdk/bindings/js/pkg/tests/http.ts +++ b/pubky-sdk/bindings/js/pkg/tests/http.ts @@ -1,5 +1,5 @@ import test from "tape"; -import { Client } from "../index.js"; +import { Client, Keypair } from "../index.js"; import type { PubkyError } from "../index.js"; import { hasNetwork } from "./utils.js"; @@ -140,14 +140,30 @@ test("fetch merges plain object headers", async (t) => { test("path-addressed storage omits pubky-host", async (t) => { const client = Client.testnet(); + const owner = Keypair.random().publicKey.z32(); const originalFetch = globalThis.fetch; + let infoRequest: Request | undefined; let seenRequest: Request | undefined; + let storageRequests = 0; globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { const request = input instanceof Request ? input : new Request(input, init); - - if (new URL(request.url).pathname.startsWith("/storage/")) { + const path = new URL(request.url).pathname; + + if (path === "/info") { + infoRequest = request; + const response = new Response(JSON.stringify({ features: ["path-addressed-storage"] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + Object.defineProperty(response, "url", { value: request.url }); + return response; + } else if (path.startsWith("/storage/")) { seenRequest = request; + storageRequests += 1; + const response = new Response(null, { status: 404 }); + Object.defineProperty(response, "url", { value: request.url }); + return response; } return originalFetch(input as RequestInfo | URL, init); @@ -155,13 +171,17 @@ test("path-addressed storage omits pubky-host", async (t) => { try { await client.fetch( - `https://${TLD}/storage/${TLD}/pub/missing.txt?cursor=hello%20world`, + `https://${TLD}/storage/${owner}/pub/missing.txt?cursor=hello%20world`, ); } finally { globalThis.fetch = originalFetch; } + t.ok(infoRequest, "feature discovery requested /info"); + t.equal(infoRequest!.credentials, "omit", "feature discovery omitted credentials"); + t.equal(infoRequest!.headers.get("pubky-host"), null, "feature discovery omitted pubky-host"); t.ok(seenRequest, "fetch preserved the path-addressed storage request"); + t.equal(storageRequests, 1, "storage response did not trigger a fallback retry"); t.equal(seenRequest!.headers.get("pubky-host"), null, "pubky-host omitted"); t.equal(seenRequest!.credentials, "include", "cookie credentials preserved"); t.equal( @@ -172,6 +192,55 @@ test("path-addressed storage omits pubky-host", async (t) => { t.end(); }); +test("storage falls back when homeserver info is unavailable", async (t) => { + const client = Client.testnet(); + const owner = Keypair.random().publicKey.z32(); + const originalFetch = globalThis.fetch; + let infoRequest: Request | undefined; + let storageRequest: Request | undefined; + + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + const path = new URL(request.url).pathname; + + if (path === "/info") { + infoRequest = request; + const response = new Response(null, { status: 404 }); + Object.defineProperty(response, "url", { value: request.url }); + return response; + } + if (path === "/pub/fallback.txt") { + storageRequest = request; + const response = new Response(null, { status: 404 }); + Object.defineProperty(response, "url", { value: request.url }); + return response; + } + + return originalFetch(input as RequestInfo | URL, init); + }) as typeof fetch; + + try { + await client.fetch( + `https://${TLD}/storage/${owner}/pub/fallback.txt?cursor=hello%20world`, + ); + } finally { + globalThis.fetch = originalFetch; + } + + t.ok(infoRequest, "feature discovery requested /info"); + t.equal(infoRequest!.credentials, "omit", "feature discovery omitted credentials"); + t.equal(infoRequest!.headers.get("pubky-host"), null, "feature discovery omitted pubky-host"); + t.ok(storageRequest, "storage used the legacy path"); + t.equal(storageRequest!.headers.get("pubky-host"), owner, "legacy owner header attached"); + t.equal(storageRequest!.credentials, "include", "storage credentials preserved"); + t.equal( + new URL(storageRequest!.url).search, + "?cursor=hello%20world", + "query parameters preserved", + ); + t.end(); +}); + test("fetch failed", async (t) => { const client = Client.testnet(); diff --git a/pubky-sdk/bindings/js/src/utils.rs b/pubky-sdk/bindings/js/src/utils.rs index 5623c663a..427a67d9c 100644 --- a/pubky-sdk/bindings/js/src/utils.rs +++ b/pubky-sdk/bindings/js/src/utils.rs @@ -68,7 +68,7 @@ pub fn set_log_level(level: Level) -> Result<(), JsValue> { /// Resolve a `pubky://` or `pubky/…` identifier into the homeserver transport URL. /// /// @param {string} identifier Either `pubky/...` (preferred) or `pubky:///...`. -/// @returns {string} HTTPS URL in the form `https://_pubky./...`. +/// @returns {string} HTTPS URL in the form `https://_pubky./storage//...`. #[wasm_bindgen(js_name = "resolvePubky")] pub fn resolve_pubky(identifier: &str) -> JsResult { Ok(pubky::resolve_pubky(identifier)?.to_string()) diff --git a/pubky-sdk/src/client/core.rs b/pubky-sdk/src/client/core.rs index 7ffb26111..c683d2200 100644 --- a/pubky-sdk/src/client/core.rs +++ b/pubky-sdk/src/client/core.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::fmt::Debug; use std::time::Duration; +use super::http_targets::HomeserverFeatures; use crate::{cross_log, errors::BuildError}; const DEFAULT_USER_AGENT: &str = concat!("pubky.org", "@", env!("CARGO_PKG_VERSION"),); @@ -262,6 +263,7 @@ impl PubkyHttpClientBuilder { Ok(PubkyHttpClient { pkarr, http: http_builder.build()?, + features: HomeserverFeatures::default(), #[cfg(not(target_arch = "wasm32"))] icann_http: icann_http_builder.build()?, @@ -418,6 +420,7 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { pub struct PubkyHttpClient { pub(crate) http: reqwest::Client, pub(crate) pkarr: pkarr::Client, + pub(crate) features: HomeserverFeatures, #[cfg(not(target_arch = "wasm32"))] pub(crate) icann_http: reqwest::Client, diff --git a/pubky-sdk/src/client/http_targets/features.rs b/pubky-sdk/src/client/http_targets/features.rs new file mode 100644 index 000000000..c789c9b90 --- /dev/null +++ b/pubky-sdk/src/client/http_targets/features.rs @@ -0,0 +1,178 @@ +use std::{ + collections::HashMap, + sync::{Arc, Mutex, PoisonError}, +}; + +use futures_util::StreamExt; +use serde::Deserialize; +use tokio::sync::OnceCell; + +use crate::{Pkdns, PubkyHttpClient, PublicKey}; + +const MAX_INFO_BYTES: usize = 16 * 1024; +type FeatureCell = Arc>>; + +#[derive(Debug, Clone, Default)] +pub(crate) struct HomeserverFeatures { + servers: Arc>>, +} + +#[derive(Deserialize)] +struct InfoResponse { + features: Vec, +} + +impl HomeserverFeatures { + pub(super) async fn supports( + &self, + client: &PubkyHttpClient, + owner: &PublicKey, + homeserver: Option<&PublicKey>, + feature: &str, + ) -> bool { + let homeserver = if let Some(homeserver) = homeserver { + homeserver.clone() + } else { + let Ok(Some(homeserver)) = Pkdns::with_client(client.clone()) + .get_homeserver_of(owner) + .await + else { + return false; + }; + homeserver + }; + let cell = self.cell(&homeserver); + let features = cell.get_or_init(|| Self::fetch(client, &homeserver)).await; + + features.iter().any(|candidate| candidate == feature) + } + + fn cell(&self, homeserver: &PublicKey) -> FeatureCell { + let mut servers = self.servers.lock().unwrap_or_else(PoisonError::into_inner); + Arc::clone(servers.entry(homeserver.clone()).or_default()) + } + + async fn fetch(client: &PubkyHttpClient, homeserver: &PublicKey) -> Vec { + let Ok(request) = client.homeserver_info_request(homeserver).await else { + return Vec::new(); + }; + let Ok(response) = request.send().await else { + return Vec::new(); + }; + if !response.status().is_success() { + return Vec::new(); + } + + let mut body = Vec::new(); + let mut chunks = response.bytes_stream(); + while let Some(chunk) = chunks.next().await { + let Ok(chunk) = chunk else { + return Vec::new(); + }; + if !Self::append_chunk(&mut body, &chunk) { + return Vec::new(); + } + } + + Self::decode(&body) + } + + fn append_chunk(body: &mut Vec, chunk: &[u8]) -> bool { + if body.len().saturating_add(chunk.len()) > MAX_INFO_BYTES { + return false; + } + + body.extend_from_slice(chunk); + true + } + + fn decode(body: &[u8]) -> Vec { + serde_json::from_slice::(body) + .map_or_else(|_error| Vec::new(), |response| response.features) + } + + #[cfg(test)] + pub(super) fn insert(&self, homeserver: &PublicKey, features: &[&str]) { + self.cell(homeserver) + .set(features.iter().map(ToString::to_string).collect()) + .expect("homeserver features were already initialized"); + } +} + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + + use super::*; + use pubky_common::constants::features::PATH_ADDRESSED_STORAGE; + + #[test] + fn decodes_only_valid_feature_lists() { + let cases = [ + ( + br#"{"features":["path-addressed-storage","unknown"]}"#.as_slice(), + true, + ), + (br#"{"features":[]}"#, false), + (br#"{"features":["unknown"]}"#, false), + (br#"{"features":{}}"#, false), + (br"{}", false), + (b"not json", false), + ]; + + for (body, expected) in cases { + assert_eq!( + HomeserverFeatures::decode(body) + .iter() + .any(|candidate| candidate == PATH_ADDRESSED_STORAGE), + expected, + "body={}", + String::from_utf8_lossy(body) + ); + } + } + + #[test] + fn limits_info_response_body() { + let mut body = vec![b'x'; MAX_INFO_BYTES - 1]; + + assert!(HomeserverFeatures::append_chunk(&mut body, b"x")); + assert!(!HomeserverFeatures::append_chunk(&mut body, b"x")); + assert_eq!(body.len(), MAX_INFO_BYTES); + } + + #[tokio::test] + async fn stores_negative_results_and_coalesces_feature_fetches() { + let discovery = HomeserverFeatures::default(); + let homeserver = crate::Keypair::random().public_key(); + let cell = discovery.cell(&homeserver); + let calls = Arc::new(AtomicUsize::new(0)); + + let first_calls = Arc::clone(&calls); + let first = cell.get_or_init(|| async move { + first_calls.fetch_add(1, Ordering::Relaxed); + tokio::task::yield_now().await; + Vec::new() + }); + let second_calls = Arc::clone(&calls); + let second = cell.get_or_init(|| async move { + second_calls.fetch_add(1, Ordering::Relaxed); + vec![PATH_ADDRESSED_STORAGE.to_string()] + }); + + let (first, second) = tokio::join!(first, second); + + assert_eq!(calls.load(Ordering::Relaxed), 1); + assert!(first.is_empty()); + assert!(second.is_empty()); + + let cached = cell + .get_or_init(|| async { + calls.fetch_add(1, Ordering::Relaxed); + Vec::new() + }) + .await; + assert!(cached.is_empty()); + assert_eq!(calls.load(Ordering::Relaxed), 1); + } +} diff --git a/pubky-sdk/src/client/http_targets/mod.rs b/pubky-sdk/src/client/http_targets/mod.rs index 8349d5d60..f576c8620 100644 --- a/pubky-sdk/src/client/http_targets/mod.rs +++ b/pubky-sdk/src/client/http_targets/mod.rs @@ -1,6 +1,17 @@ use crate::{PublicKey, Result}; use url::Url; +mod features; +pub(crate) use features::HomeserverFeatures; +mod storage; + +#[derive(Debug, PartialEq, Eq)] +enum RequestAddressing { + Standard, + PathAddressedStorage, + LegacyStorage { owner: String }, +} + #[cfg(not(target_arch = "wasm32"))] pub mod native; #[cfg(target_arch = "wasm32")] diff --git a/pubky-sdk/src/client/http_targets/native.rs b/pubky-sdk/src/client/http_targets/native.rs index e699da269..bbd4442b9 100644 --- a/pubky-sdk/src/client/http_targets/native.rs +++ b/pubky-sdk/src/client/http_targets/native.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, PoisonError, RwLock}; use std::time::{Duration, Instant}; -use super::{homeserver_url, is_path_addressed_storage}; +use super::{RequestAddressing, homeserver_url, is_path_addressed_storage}; use futures_util::StreamExt; use tokio::net::TcpStream; @@ -20,6 +20,19 @@ pub(crate) enum ResolvedTransport { Icann { domain: String, port: Option }, } +#[derive(Debug)] +enum RequestTransport { + Standard, + Pubky(ResolvedTransport), +} + +#[derive(Debug)] +struct ResolvedRequest { + url: Url, + transport: RequestTransport, + pubky_host: Option, +} + /// Resolves and caches per-host transport decisions (`PubkyTLS` vs ICANN). /// /// Accepts a `&pkarr::Client` reference when resolution is needed — does not @@ -162,20 +175,9 @@ impl PubkyHttpClient { /// /// Returns a [`Result`] containing the prepared `RequestBuilder`, or a URL/transport /// parsing error if the supplied `url` is invalid. - pub(crate) async fn cross_request( - &self, - method: Method, - mut url: Url, - ) -> Result { - let Some(pk) = self.prepare_request(&mut url).await? else { - return Ok(self.request(method, &url)); - }; - // Resolve the transport with the full host: for `_pubky.` hosts the - // endpoints live under that qname, not the bare key apex. `pk` is only - // the `pubky-host` header value. - let qname = url.host_str().unwrap_or(&pk).to_string(); - let transport = self.transport.resolve(&qname, &self.pkarr).await; - self.build_pubky_request(method, &url, &pk, &transport) + pub(crate) async fn cross_request(&self, method: Method, url: Url) -> Result { + let request = self.resolve_request(url).await?; + self.build_request(method, request) } /// Native has no ambient browser cookie jar, so this is `cross_request`. @@ -200,27 +202,33 @@ impl PubkyHttpClient { let pubky_host_z32 = pubky_host.z32(); let transport = self.transport.resolve(&homeserver_z32, &self.pkarr).await; - match transport { - ResolvedTransport::PubkyTls => { - let request = self.http.request(method, url.as_str()); - if is_path_addressed_storage(&url) { - Ok(request) - } else { - Ok(request.header("pubky-host", pubky_host_z32)) - } - } - ResolvedTransport::Icann { .. } => { - self.build_pubky_request(method, &url, &pubky_host_z32, &transport) - } - } + let pubky_host = (!is_path_addressed_storage(&url)).then_some(pubky_host_z32); + + self.build_request( + method, + ResolvedRequest { + url, + transport: RequestTransport::Pubky(transport), + pubky_host, + }, + ) } - /// Build a [`RequestBuilder`] for a resolved pubky host transport. - fn build_pubky_request( + pub(super) async fn homeserver_info_request( + &self, + homeserver: &PublicKey, + ) -> Result { + // Bypass cross_request so discovery cannot recursively trigger itself. + let url = homeserver_url(homeserver, "/info")?; + let transport = self.transport.resolve(&homeserver.z32(), &self.pkarr).await; + + self.build_transport_request(Method::GET, &url, &transport) + } + + fn build_transport_request( &self, method: Method, url: &Url, - pk: &str, transport: &ResolvedTransport, ) -> Result { match transport { @@ -228,34 +236,85 @@ impl PubkyHttpClient { ResolvedTransport::Icann { domain, port } => { let mut icann_url = url.clone(); icann_url.set_host(Some(domain))?; - if let Some(p) = port { + if let Some(port) = port { icann_url - .set_port(Some(*p)) + .set_port(Some(*port)) .map_err(|_err| url::ParseError::InvalidPort)?; } - cross_log!(debug, "ICANN fallback for {pk} via {domain}"); - let request = self.icann_http.request(method, icann_url.as_str()); - if is_path_addressed_storage(url) { - Ok(request) - } else { - Ok(request.header("pubky-host", pk)) - } + cross_log!(debug, "ICANN fallback via {domain}"); + Ok(self.icann_http.request(method, icann_url.as_str())) } } } - /// Detect pubky hosts and return the z32 public key when applicable. - /// - /// Native builds do not rewrite URLs; we only detect pubky hosts and return the - /// `pubky-host` value when applicable. + fn build_request(&self, method: Method, resolved: ResolvedRequest) -> Result { + let request = match &resolved.transport { + RequestTransport::Standard => self.request(method, &resolved.url), + RequestTransport::Pubky(transport) => { + self.build_transport_request(method, &resolved.url, transport)? + } + }; + + Ok(match resolved.pubky_host { + Some(pubky_host) => request.header("pubky-host", pubky_host), + None => request, + }) + } + + async fn resolve_request(&self, mut url: Url) -> Result { + let addressing = self.prepare_request_addressing(&mut url).await?; + let Some(pubky_host) = self.prepare_transport_request(&mut url).await? else { + let pubky_host = match addressing { + RequestAddressing::LegacyStorage { owner } => Some(owner), + RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => None, + }; + + return Ok(ResolvedRequest { + url, + transport: RequestTransport::Standard, + pubky_host, + }); + }; + + // `_pubky.` endpoints live under the full qname, not the bare key apex. + let qname = url.host_str().unwrap_or(&pubky_host).to_string(); + let transport = self.transport.resolve(&qname, &self.pkarr).await; + let pubky_host = match addressing { + RequestAddressing::Standard + if matches!(&transport, ResolvedTransport::Icann { .. }) => + { + Some(pubky_host) + } + RequestAddressing::LegacyStorage { owner } => Some(owner), + RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => None, + }; + + Ok(ResolvedRequest { + url, + transport: RequestTransport::Pubky(transport), + pubky_host, + }) + } + + /// Prepare a URL for transport and return its `pubky-host` value when applicable. /// /// # Errors - /// Returns [`RequestError::Validation`] if the host uses a `pubky` prefix. + /// Returns a validation or resolution error if the URL cannot be prepared. + pub async fn prepare_request(&self, url: &mut Url) -> Result> { + let addressing = self.prepare_request_addressing(url).await?; + let pubky_host = self.prepare_transport_request(url).await?; + + Ok(match addressing { + RequestAddressing::LegacyStorage { owner } => Some(owner), + RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => pubky_host, + }) + } + #[allow( clippy::unused_async, reason = "keep async signature aligned with WASM build" )] - pub async fn prepare_request(&self, url: &mut Url) -> Result> { + async fn prepare_transport_request(&self, url: &mut Url) -> Result> { let host = url.host_str().unwrap_or(""); if let Some(stripped) = host.strip_prefix("_pubky.") { @@ -331,6 +390,7 @@ mod tests { use std::num::NonZeroUsize; use super::*; + use crate::Keypair as PubkyKeypair; use pkarr::dns::rdata::SVCB; use pkarr::{Cache, InMemoryCache, Keypair, SignedPacket}; @@ -365,7 +425,7 @@ mod tests { } #[test] - fn build_pubky_request_icann_rewrites_url_and_sets_header() { + fn build_request_uses_the_resolved_icann_target_and_header() { let client = PubkyHttpClient::builder() .isolated_pkarr_test() .build() @@ -378,7 +438,14 @@ mod tests { }; let req = client - .build_pubky_request(Method::GET, &url, z32, &transport) + .build_request( + Method::GET, + ResolvedRequest { + url, + transport: RequestTransport::Pubky(transport), + pubky_host: Some(z32.to_string()), + }, + ) .unwrap() .build() .unwrap(); @@ -390,7 +457,7 @@ mod tests { } #[test] - fn build_pubky_storage_request_icann_retains_owner_path_without_header() { + fn build_request_retains_path_addressing_without_a_header() { let client = PubkyHttpClient::builder() .isolated_pkarr_test() .build() @@ -406,7 +473,14 @@ mod tests { }; let req = client - .build_pubky_request(Method::GET, &url, z32, &transport) + .build_request( + Method::GET, + ResolvedRequest { + url, + transport: RequestTransport::Pubky(transport), + pubky_host: None, + }, + ) .unwrap() .build() .unwrap(); @@ -418,6 +492,37 @@ mod tests { assert!(!req.headers().contains_key("pubky-host")); } + #[tokio::test] + async fn legacy_storage_attaches_the_path_owner_on_pubky_tls() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = PubkyKeypair::random().public_key(); + let owner = PubkyKeypair::random().public_key(); + client.features.insert(&homeserver, &[]); + client.transport.cache.write().unwrap().insert( + homeserver.z32(), + (Instant::now(), ResolvedTransport::PubkyTls), + ); + let url = Url::parse(&format!( + "https://{}/storage/{}/pub/file.txt", + homeserver.z32(), + owner.z32() + )) + .unwrap(); + + let request = client + .cross_request(Method::GET, url) + .await + .unwrap() + .build() + .unwrap(); + + assert_eq!(request.url().path(), "/pub/file.txt"); + assert_eq!(request.headers().get("pubky-host").unwrap(), &owner.z32()); + } + #[tokio::test] async fn resolve_transport_direct_only() { let kp = Keypair::random(); diff --git a/pubky-sdk/src/client/http_targets/storage.rs b/pubky-sdk/src/client/http_targets/storage.rs new file mode 100644 index 000000000..e5bc01f52 --- /dev/null +++ b/pubky-sdk/src/client/http_targets/storage.rs @@ -0,0 +1,115 @@ +use url::Url; + +use super::RequestAddressing; +use crate::{PubkyHttpClient, PublicKey, Result, errors::RequestError}; +use pubky_common::constants::features::PATH_ADDRESSED_STORAGE; + +impl PubkyHttpClient { + pub(super) async fn prepare_request_addressing( + &self, + url: &mut Url, + ) -> Result { + let Some(path) = url.path().strip_prefix("/storage/") else { + return Ok(RequestAddressing::Standard); + }; + let (owner, path) = path + .split_once('/') + .ok_or_else(|| RequestError::Validation { + message: "path-addressed storage URL is missing a resource path".to_string(), + })?; + let owner = PublicKey::try_from_z32(owner).map_err(|_error| RequestError::Validation { + message: "path-addressed storage URL contains an invalid owner".to_string(), + })?; + let legacy_path = format!("/{path}"); + let homeserver = url + .host_str() + .filter(|host| !host.starts_with("_pubky.")) + .and_then(|host| PublicKey::try_from_z32(host).ok()); + + if self + .features + .supports(self, &owner, homeserver.as_ref(), PATH_ADDRESSED_STORAGE) + .await + { + return Ok(RequestAddressing::PathAddressedStorage); + } + + // Choose compatibility before sending storage; response errors never trigger a retry. + url.set_path(&legacy_path); + Ok(RequestAddressing::LegacyStorage { owner: owner.z32() }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Keypair; + + #[tokio::test] + async fn advertised_feature_keeps_the_storage_path() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = Keypair::random().public_key(); + let owner = Keypair::random().public_key(); + client + .features + .insert(&homeserver, &[PATH_ADDRESSED_STORAGE]); + let mut url = Url::parse(&format!( + "https://{}/storage/{}/pub/file.txt", + homeserver.z32(), + owner.z32() + )) + .unwrap(); + + let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + + assert_eq!(addressing, RequestAddressing::PathAddressedStorage); + assert_eq!(url.path(), format!("/storage/{}/pub/file.txt", owner.z32())); + } + + #[tokio::test] + async fn missing_feature_uses_legacy_path_and_preserves_the_url() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = Keypair::random().public_key(); + let owner = Keypair::random().public_key(); + client.features.insert(&homeserver, &[]); + let mut url = Url::parse(&format!( + "https://{}/storage/{}/pub/My%20File%252FName/?cursor=hello%20world", + homeserver.z32(), + owner.z32() + )) + .unwrap(); + + let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + + assert_eq!( + addressing, + RequestAddressing::LegacyStorage { owner: owner.z32() } + ); + assert_eq!(url.path(), "/pub/My%20File%252FName/"); + assert_eq!(url.query(), Some("cursor=hello%20world")); + } + + #[tokio::test] + async fn validates_storage_owner_and_resource_path() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + + for url in [ + "https://example.com/storage/not-a-key/pub/file.txt", + "https://example.com/storage/missing-path", + ] { + client + .prepare_request_addressing(&mut Url::parse(url).unwrap()) + .await + .unwrap_err(); + } + } +} diff --git a/pubky-sdk/src/client/http_targets/wasm.rs b/pubky-sdk/src/client/http_targets/wasm.rs index 51d3a770a..facaf9538 100644 --- a/pubky-sdk/src/client/http_targets/wasm.rs +++ b/pubky-sdk/src/client/http_targets/wasm.rs @@ -1,6 +1,6 @@ //! HTTP methods that support `https://` with Pkarr domains, including `_pubky.` URLs -use super::{homeserver_url, is_path_addressed_storage}; +use super::{RequestAddressing, homeserver_url, is_path_addressed_storage}; use crate::PublicKey; use crate::errors::{PkarrError, RequestError, Result}; use crate::{PubkyHttpClient, cross_log}; @@ -73,6 +73,17 @@ impl PubkyHttpClient { )) } + pub(super) async fn homeserver_info_request( + &self, + homeserver: &PublicKey, + ) -> Result { + // Bypass cross_request so discovery cannot recursively trigger itself. + let mut url = homeserver_url(homeserver, "/info")?; + self.prepare_transport_request(&mut url).await?; + + Ok(self.http.request(Method::GET, url).fetch_credentials_omit()) + } + async fn cross_request_with_credentials( &self, method: Method, @@ -93,12 +104,21 @@ impl PubkyHttpClient { Ok(Self::attach_pubky_host(builder, &url, pubky_host)) } - /// - Resolves a clearnet host to call with fetch - /// - Returns the `pubky-host` value if available + /// Prepare a URL for transport and return its `pubky-host` value when applicable. /// /// # Errors - /// - Returns [`crate::errors::PkarrError`] when PKARR resolution fails or produces invalid endpoints. + /// Returns a validation or resolution error if the URL cannot be prepared. pub async fn prepare_request(&self, url: &mut Url) -> Result> { + let addressing = self.prepare_request_addressing(url).await?; + let pubky_host = self.prepare_transport_request(url).await?; + + Ok(match addressing { + RequestAddressing::LegacyStorage { owner } => Some(owner), + RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => pubky_host, + }) + } + + async fn prepare_transport_request(&self, url: &mut Url) -> Result> { let host = url.host_str().unwrap_or("").to_string(); let mut pubky_host = None; From 423917c3e832057665797a70bdc7894fdb856538 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Thu, 13 Aug 2026 17:34:30 -0300 Subject: [PATCH 06/10] fix: host gating + timeouts + owner in transport url --- pubky-sdk/bindings/js/pkg/tests/http.ts | 37 +++++ pubky-sdk/src/actors/storage/resource.rs | 61 +++++++- pubky-sdk/src/client/http_targets/features.rs | 139 +++++++++++++----- pubky-sdk/src/client/http_targets/storage.rs | 39 ++++- 4 files changed, 229 insertions(+), 47 deletions(-) diff --git a/pubky-sdk/bindings/js/pkg/tests/http.ts b/pubky-sdk/bindings/js/pkg/tests/http.ts index ab1b8832d..abd3e9ecb 100644 --- a/pubky-sdk/bindings/js/pkg/tests/http.ts +++ b/pubky-sdk/bindings/js/pkg/tests/http.ts @@ -138,6 +138,43 @@ test("fetch merges plain object headers", async (t) => { t.end(); }); +test("ordinary HTTP storage paths remain untouched", async (t) => { + const client = Client.testnet(); + const owner = Keypair.random().publicKey.z32(); + const originalFetch = globalThis.fetch; + const requests: Request[] = []; + + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const request = input instanceof Request ? input : new Request(input, init); + requests.push(request); + const response = new Response(null, { status: 204 }); + Object.defineProperty(response, "url", { value: request.url }); + return response; + }) as typeof fetch; + + try { + await client.fetch("https://example.com/storage/images/logo.png"); + await client.fetch(`https://example.com/storage/${owner}/pub/file.txt`); + } finally { + globalThis.fetch = originalFetch; + } + + t.equal(requests.length, 2, "only the requested resources were fetched"); + t.equal( + requests[0].url, + "https://example.com/storage/images/logo.png", + "ordinary storage path preserved", + ); + t.equal( + requests[1].url, + `https://example.com/storage/${owner}/pub/file.txt`, + "z32 storage segment preserved on an ordinary host", + ); + t.equal(requests[0].headers.get("pubky-host"), null, "pubky-host omitted"); + t.equal(requests[1].headers.get("pubky-host"), null, "pubky-host omitted for z32 path"); + t.end(); +}); + test("path-addressed storage omits pubky-host", async (t) => { const client = Client.testnet(); const owner = Keypair.random().publicKey.z32(); diff --git a/pubky-sdk/src/actors/storage/resource.rs b/pubky-sdk/src/actors/storage/resource.rs index c150056d0..6c936949c 100644 --- a/pubky-sdk/src/actors/storage/resource.rs +++ b/pubky-sdk/src/actors/storage/resource.rs @@ -237,18 +237,28 @@ impl PubkyResource { /// Returns [`Error::Request`] when the owner or path is malformed. pub fn from_transport_url(url: &Url) -> Result { let (owner, path) = if let Some(path) = url.path().strip_prefix("/storage/") { - path.split_once('/') - .ok_or_else(|| invalid("storage transport URL is missing a resource path"))? + let (owner, path) = path + .split_once('/') + .ok_or_else(|| invalid("storage transport URL is missing a resource path"))?; + let owner = parse_transport_owner(owner)?; + + if let Some(host_owner) = url.host_str().and_then(|host| host.strip_prefix("_pubky.")) + && parse_transport_owner(host_owner)? != owner + { + return Err(invalid( + "storage transport URL owner does not match its `_pubky` host", + )); + } + + (owner, path) } else { let owner = url .host_str() .and_then(|host| host.strip_prefix("_pubky.")) .ok_or_else(|| invalid("legacy transport URL is missing a `_pubky` host"))?; - (owner, url.path()) + (parse_transport_owner(owner)?, url.path()) }; - let owner = PublicKey::try_from_z32(owner) - .map_err(|_err| invalid("transport URL contains an invalid owner"))?; let path = if path.is_empty() { "/" } else { path }; let decoded_path = percent_decode_str(path) .decode_utf8() @@ -263,6 +273,17 @@ impl PubkyResource { } } +fn parse_transport_owner(owner: &str) -> Result { + if PublicKey::is_pubky_prefixed(owner) { + return Err(invalid( + "transport URL owner must use raw z32 without `pubky` prefix", + )); + } + + PublicKey::try_from_z32(owner) + .map_err(|_err| invalid("transport URL contains an invalid owner")) +} + impl FromStr for PubkyResource { type Err = Error; @@ -619,7 +640,7 @@ mod tests { } #[test] - fn canonical_transport_path_owner_is_authoritative() { + fn canonical_transport_rejects_a_conflicting_pubky_host() { let host_owner = Keypair::random().public_key(); let path_owner = Keypair::random().public_key(); let url = Url::parse(&format!( @@ -629,8 +650,34 @@ mod tests { )) .unwrap(); + let error = PubkyResource::from_transport_url(&url).unwrap_err(); + assert!(error.to_string().contains("does not match")); + } + + #[test] + fn canonical_transport_accepts_an_icann_homeserver_authority() { + let owner = Keypair::random().public_key(); + let url = Url::parse(&format!( + "https://example.com/storage/{}/pub/My%20File.txt", + owner.z32() + )) + .unwrap(); + let resource = PubkyResource::from_transport_url(&url).unwrap(); - assert_eq!(resource.owner, path_owner); + assert_eq!(resource.owner, owner); assert_eq!(resource.path.as_str(), "/pub/My%20File.txt"); } + + #[test] + fn transport_owner_rejects_the_pubky_prefix() { + let owner = Keypair::random().public_key(); + let url = Url::parse(&format!( + "https://example.com/storage/pubky{}/pub/file.txt", + owner.z32() + )) + .unwrap(); + + let error = PubkyResource::from_transport_url(&url).unwrap_err(); + assert!(error.to_string().contains("must use raw z32")); + } } diff --git a/pubky-sdk/src/client/http_targets/features.rs b/pubky-sdk/src/client/http_targets/features.rs index c789c9b90..b6dd19606 100644 --- a/pubky-sdk/src/client/http_targets/features.rs +++ b/pubky-sdk/src/client/http_targets/features.rs @@ -1,16 +1,20 @@ use std::{ collections::HashMap, sync::{Arc, Mutex, PoisonError}, + time::Duration, }; use futures_util::StreamExt; use serde::Deserialize; -use tokio::sync::OnceCell; +use tokio::sync::Mutex as AsyncMutex; +use web_time::Instant; use crate::{Pkdns, PubkyHttpClient, PublicKey}; const MAX_INFO_BYTES: usize = 16 * 1024; -type FeatureCell = Arc>>; +const INFO_TIMEOUT: Duration = Duration::from_secs(5); +const FAILED_INFO_RETRY_INTERVAL: Duration = Duration::from_secs(60); +type FeatureCell = Arc>>; #[derive(Debug, Clone, Default)] pub(crate) struct HomeserverFeatures { @@ -22,6 +26,22 @@ struct InfoResponse { features: Vec, } +#[derive(Debug)] +enum CachedFeatures { + Available(Vec), + UnavailableUntil(Instant), +} + +impl CachedFeatures { + fn current(&self) -> Option<&[String]> { + match self { + Self::Available(features) => Some(features), + Self::UnavailableUntil(retry_at) if Instant::now() < *retry_at => Some(&[]), + Self::UnavailableUntil(_) => None, + } + } +} + impl HomeserverFeatures { pub(super) async fn supports( &self, @@ -41,10 +61,9 @@ impl HomeserverFeatures { }; homeserver }; - let cell = self.cell(&homeserver); - let features = cell.get_or_init(|| Self::fetch(client, &homeserver)).await; - features.iter().any(|candidate| candidate == feature) + self.supports_for(&homeserver, feature, || Self::fetch(client, &homeserver)) + .await } fn cell(&self, homeserver: &PublicKey) -> FeatureCell { @@ -52,25 +71,47 @@ impl HomeserverFeatures { Arc::clone(servers.entry(homeserver.clone()).or_default()) } - async fn fetch(client: &PubkyHttpClient, homeserver: &PublicKey) -> Vec { + async fn supports_for(&self, homeserver: &PublicKey, feature: &str, fetch: F) -> bool + where + F: FnOnce() -> Fut, + Fut: Future>>, + { + let cell = self.cell(homeserver); + let mut cached = cell.lock().await; + if let Some(features) = cached.as_ref().and_then(CachedFeatures::current) { + return features.iter().any(|candidate| candidate == feature); + } + + let Some(features) = fetch().await else { + *cached = Some(CachedFeatures::UnavailableUntil( + Instant::now() + FAILED_INFO_RETRY_INTERVAL, + )); + return false; + }; + let supports = features.iter().any(|candidate| candidate == feature); + *cached = Some(CachedFeatures::Available(features)); + supports + } + + async fn fetch(client: &PubkyHttpClient, homeserver: &PublicKey) -> Option> { let Ok(request) = client.homeserver_info_request(homeserver).await else { - return Vec::new(); + return None; }; - let Ok(response) = request.send().await else { - return Vec::new(); + let Ok(response) = request.timeout(INFO_TIMEOUT).send().await else { + return None; }; if !response.status().is_success() { - return Vec::new(); + return None; } let mut body = Vec::new(); let mut chunks = response.bytes_stream(); while let Some(chunk) = chunks.next().await { let Ok(chunk) = chunk else { - return Vec::new(); + return None; }; if !Self::append_chunk(&mut body, &chunk) { - return Vec::new(); + return None; } } @@ -86,16 +127,25 @@ impl HomeserverFeatures { true } - fn decode(body: &[u8]) -> Vec { + fn decode(body: &[u8]) -> Option> { serde_json::from_slice::(body) - .map_or_else(|_error| Vec::new(), |response| response.features) + .map(|response| response.features) + .ok() } #[cfg(test)] pub(super) fn insert(&self, homeserver: &PublicKey, features: &[&str]) { - self.cell(homeserver) - .set(features.iter().map(ToString::to_string).collect()) - .expect("homeserver features were already initialized"); + let cell = self.cell(homeserver); + let mut cached = cell + .try_lock() + .expect("homeserver features are not being initialized"); + assert!( + cached.is_none(), + "homeserver features were already initialized" + ); + *cached = Some(CachedFeatures::Available( + features.iter().map(ToString::to_string).collect(), + )); } } @@ -111,20 +161,20 @@ mod tests { let cases = [ ( br#"{"features":["path-addressed-storage","unknown"]}"#.as_slice(), - true, + Some(true), ), - (br#"{"features":[]}"#, false), - (br#"{"features":["unknown"]}"#, false), - (br#"{"features":{}}"#, false), - (br"{}", false), - (b"not json", false), + (br#"{"features":[]}"#, Some(false)), + (br#"{"features":["unknown"]}"#, Some(false)), + (br#"{"features":{}}"#, None), + (br"{}", None), + (b"not json", None), ]; for (body, expected) in cases { assert_eq!( - HomeserverFeatures::decode(body) + HomeserverFeatures::decode(body).map(|features| features .iter() - .any(|candidate| candidate == PATH_ADDRESSED_STORAGE), + .any(|candidate| candidate == PATH_ADDRESSED_STORAGE)), expected, "body={}", String::from_utf8_lossy(body) @@ -142,37 +192,52 @@ mod tests { } #[tokio::test] - async fn stores_negative_results_and_coalesces_feature_fetches() { + async fn temporarily_stores_failures_and_coalesces_feature_fetches() { let discovery = HomeserverFeatures::default(); let homeserver = crate::Keypair::random().public_key(); - let cell = discovery.cell(&homeserver); let calls = Arc::new(AtomicUsize::new(0)); let first_calls = Arc::clone(&calls); - let first = cell.get_or_init(|| async move { + let first = discovery.supports_for(&homeserver, PATH_ADDRESSED_STORAGE, || async move { first_calls.fetch_add(1, Ordering::Relaxed); tokio::task::yield_now().await; - Vec::new() + None }); let second_calls = Arc::clone(&calls); - let second = cell.get_or_init(|| async move { + let second = discovery.supports_for(&homeserver, PATH_ADDRESSED_STORAGE, || async move { second_calls.fetch_add(1, Ordering::Relaxed); - vec![PATH_ADDRESSED_STORAGE.to_string()] + Some(vec![PATH_ADDRESSED_STORAGE.to_string()]) }); let (first, second) = tokio::join!(first, second); assert_eq!(calls.load(Ordering::Relaxed), 1); - assert!(first.is_empty()); - assert!(second.is_empty()); + assert!(!first); + assert!(!second); - let cached = cell - .get_or_init(|| async { + let cached = discovery + .supports_for(&homeserver, PATH_ADDRESSED_STORAGE, || async { calls.fetch_add(1, Ordering::Relaxed); - Vec::new() + Some(Vec::new()) }) .await; - assert!(cached.is_empty()); + assert!(!cached); assert_eq!(calls.load(Ordering::Relaxed), 1); } + + #[tokio::test] + async fn retries_expired_failures() { + let discovery = HomeserverFeatures::default(); + let homeserver = crate::Keypair::random().public_key(); + let cell = discovery.cell(&homeserver); + *cell.lock().await = Some(CachedFeatures::UnavailableUntil(Instant::now())); + + let supported = discovery + .supports_for(&homeserver, PATH_ADDRESSED_STORAGE, || async { + Some(vec![PATH_ADDRESSED_STORAGE.to_string()]) + }) + .await; + + assert!(supported); + } } diff --git a/pubky-sdk/src/client/http_targets/storage.rs b/pubky-sdk/src/client/http_targets/storage.rs index e5bc01f52..ffdddd66b 100644 --- a/pubky-sdk/src/client/http_targets/storage.rs +++ b/pubky-sdk/src/client/http_targets/storage.rs @@ -12,6 +12,13 @@ impl PubkyHttpClient { let Some(path) = url.path().strip_prefix("/storage/") else { return Ok(RequestAddressing::Standard); }; + let Some(host) = url.host_str() else { + return Ok(RequestAddressing::Standard); + }; + let transport_host = host.strip_prefix("_pubky.").unwrap_or(host); + if PublicKey::try_from_z32(transport_host).is_err() { + return Ok(RequestAddressing::Standard); + } let (owner, path) = path .split_once('/') .ok_or_else(|| RequestError::Validation { @@ -96,18 +103,44 @@ mod tests { } #[tokio::test] - async fn validates_storage_owner_and_resource_path() { + async fn ignores_storage_paths_on_regular_hosts() { let client = PubkyHttpClient::builder() .isolated_pkarr_test() .build() .unwrap(); + let owner = Keypair::random().public_key(); for url in [ "https://example.com/storage/not-a-key/pub/file.txt", - "https://example.com/storage/missing-path", + &format!("https://example.com/storage/{}/pub/file.txt", owner.z32()), + ] { + let mut url = Url::parse(url).unwrap(); + let original = url.clone(); + + let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + + assert_eq!(addressing, RequestAddressing::Standard); + assert_eq!(url, original); + } + } + + #[tokio::test] + async fn validates_storage_owner_and_resource_path_on_pubky_hosts() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = Keypair::random().public_key(); + + for url in [ + format!( + "https://{}/storage/not-a-key/pub/file.txt", + homeserver.z32() + ), + format!("https://{}/storage/missing-path", homeserver.z32()), ] { client - .prepare_request_addressing(&mut Url::parse(url).unwrap()) + .prepare_request_addressing(&mut Url::parse(&url).unwrap()) .await .unwrap_err(); } From ee2ebf2fb252f0a22bf0a34781e1e7f78d65c915 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Thu, 13 Aug 2026 18:40:35 -0300 Subject: [PATCH 07/10] refactor: simplify and add consistency to the codfe --- pubky-sdk/bindings/js/src/client/http.rs | 9 +- pubky-sdk/src/client/core.rs | 13 ++ pubky-sdk/src/client/http_targets/features.rs | 19 +-- pubky-sdk/src/client/http_targets/mod.rs | 150 +++++++++++++++--- pubky-sdk/src/client/http_targets/native.rs | 79 ++------- pubky-sdk/src/client/http_targets/storage.rs | 132 +++++++++++---- pubky-sdk/src/client/http_targets/wasm.rs | 128 +++------------ 7 files changed, 287 insertions(+), 243 deletions(-) diff --git a/pubky-sdk/bindings/js/src/client/http.rs b/pubky-sdk/bindings/js/src/client/http.rs index e7ebb45d1..49d2a51f0 100644 --- a/pubky-sdk/bindings/js/src/client/http.rs +++ b/pubky-sdk/bindings/js/src/client/http.rs @@ -34,8 +34,7 @@ impl Client { } // 2) Ask the SDK to prepare (resolve pkarr, adjust host, etc.) - // Returns Some() if this targets a Pubky host. - let pubky_host = self.0.prepare_request(&mut url).await?; + let prepared = self.0.prepare_fetch(&mut url).await?; // 3) Start from caller's init; DO NOT clobber headers. let req_init = init @@ -43,9 +42,7 @@ impl Client { .unwrap_or_default(); // 3a) If needed, ensure `pubky-host` is present in *init.headers* BEFORE Request creation. - if let Some(host) = pubky_host.as_deref() - && !url.path().starts_with("/storage/") - { + if let Some(host) = prepared.pubky_host_header.as_deref() { // Try to read any existing headers off RequestInit via reflection. // This value can be: undefined/null (no headers), a real `Headers`, or // a plain object/array. We handle those cases explicitly. @@ -100,7 +97,7 @@ impl Client { .unwrap_or(JsValue::UNDEFINED); let credentials_provided = !(credentials_js.is_undefined() || credentials_js.is_null()); - if pubky_host.is_some() && !credentials_provided { + if prepared.is_pubky_target && !credentials_provided { // Pubky hosts rely on cookies for authentication/session I/O. If the caller // omitted a credential mode, fall back to `include`. req_init.set_credentials(RequestCredentials::Include); diff --git a/pubky-sdk/src/client/core.rs b/pubky-sdk/src/client/core.rs index c683d2200..d99ec6e64 100644 --- a/pubky-sdk/src/client/core.rs +++ b/pubky-sdk/src/client/core.rs @@ -434,6 +434,19 @@ pub struct PubkyHttpClient { pub(crate) testnet_host: Option, } +/// Prepared browser-fetch metadata used by the JavaScript bindings. +#[doc(hidden)] +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PreparedFetch { + /// The `pubky-host` header to attach, if any. + #[doc(hidden)] + pub pubky_host_header: Option, + + /// Whether the original request targeted a Pubky authority. + #[doc(hidden)] + pub is_pubky_target: bool, +} + impl PubkyHttpClient { /// Creates a client configured for public mainline DHT and pkarr relays. /// diff --git a/pubky-sdk/src/client/http_targets/features.rs b/pubky-sdk/src/client/http_targets/features.rs index b6dd19606..5a42b57c1 100644 --- a/pubky-sdk/src/client/http_targets/features.rs +++ b/pubky-sdk/src/client/http_targets/features.rs @@ -9,7 +9,7 @@ use serde::Deserialize; use tokio::sync::Mutex as AsyncMutex; use web_time::Instant; -use crate::{Pkdns, PubkyHttpClient, PublicKey}; +use crate::{PubkyHttpClient, PublicKey}; const MAX_INFO_BYTES: usize = 16 * 1024; const INFO_TIMEOUT: Duration = Duration::from_secs(5); @@ -46,23 +46,10 @@ impl HomeserverFeatures { pub(super) async fn supports( &self, client: &PubkyHttpClient, - owner: &PublicKey, - homeserver: Option<&PublicKey>, + homeserver: &PublicKey, feature: &str, ) -> bool { - let homeserver = if let Some(homeserver) = homeserver { - homeserver.clone() - } else { - let Ok(Some(homeserver)) = Pkdns::with_client(client.clone()) - .get_homeserver_of(owner) - .await - else { - return false; - }; - homeserver - }; - - self.supports_for(&homeserver, feature, || Self::fetch(client, &homeserver)) + self.supports_for(homeserver, feature, || Self::fetch(client, homeserver)) .await } diff --git a/pubky-sdk/src/client/http_targets/mod.rs b/pubky-sdk/src/client/http_targets/mod.rs index f576c8620..013649710 100644 --- a/pubky-sdk/src/client/http_targets/mod.rs +++ b/pubky-sdk/src/client/http_targets/mod.rs @@ -1,22 +1,86 @@ -use crate::{PublicKey, Result}; +use crate::{PubkyHttpClient, PublicKey, Result, errors::RequestError}; use url::Url; mod features; pub(crate) use features::HomeserverFeatures; mod storage; - -#[derive(Debug, PartialEq, Eq)] -enum RequestAddressing { - Standard, - PathAddressedStorage, - LegacyStorage { owner: String }, -} +use storage::StorageAddressing; #[cfg(not(target_arch = "wasm32"))] pub mod native; #[cfg(target_arch = "wasm32")] pub mod wasm; +#[derive(Debug, Clone, PartialEq, Eq)] +enum TransportHost { + PubkyQname(PublicKey), + BarePublicKey(PublicKey), + Other, +} + +fn classify_transport_host(host: &str) -> Result { + let (host, is_pubky_qname) = host + .strip_prefix("_pubky.") + .map_or((host, false), |host| (host, true)); + + if PublicKey::is_pubky_prefixed(host) { + return Err(RequestError::Validation { + message: "pubky prefix is not allowed in transport hosts; use raw z32".to_string(), + } + .into()); + } + + let Ok(public_key) = PublicKey::try_from_z32(host) else { + return Ok(TransportHost::Other); + }; + + if is_pubky_qname { + Ok(TransportHost::PubkyQname(public_key)) + } else { + Ok(TransportHost::BarePublicKey(public_key)) + } +} + +impl PubkyHttpClient { + async fn prepare_request_parts( + &self, + url: &mut Url, + ) -> Result<(StorageAddressing, Option)> { + // Storage addressing must inspect the canonical URL before WASM transport rewrites it. + let addressing = self.prepare_storage_addressing(url).await?; + let pubky_host = self.prepare_transport_request(url).await?; + Ok((addressing, pubky_host)) + } + + /// Prepare a URL for transport and return its `pubky-host` value when applicable. + /// + /// # Errors + /// Returns a validation or resolution error if the URL cannot be prepared. + pub async fn prepare_request(&self, url: &mut Url) -> Result> { + let (addressing, pubky_host) = self.prepare_request_parts(url).await?; + + Ok(match addressing { + StorageAddressing::LegacyStorage { owner } => Some(owner), + StorageAddressing::Standard | StorageAddressing::PathAddressedStorage => pubky_host, + }) + } + + /// Prepare a URL and browser-fetch metadata for the JavaScript bindings. + /// + /// # Errors + /// Returns a validation or resolution error if the URL cannot be prepared. + #[doc(hidden)] + pub async fn prepare_fetch(&self, url: &mut Url) -> Result { + let (addressing, pubky_host) = self.prepare_request_parts(url).await?; + let is_pubky_target = pubky_host.is_some(); + + Ok(crate::client::core::PreparedFetch { + pubky_host_header: addressing.into_pubky_host(pubky_host), + is_pubky_target, + }) + } +} + fn homeserver_url(homeserver: &PublicKey, path: &str) -> Result { let path = if path.starts_with('/') { path.to_string() @@ -43,26 +107,41 @@ pub(crate) fn user_endpoint_url(user: &PublicKey, path: &str) -> Result { ))?) } -#[inline] -fn is_path_addressed_storage(url: &Url) -> bool { - url.path().starts_with("/storage/") -} - #[cfg(test)] mod tests { use super::*; #[test] - fn detects_only_path_addressed_storage_routes() { - assert!(is_path_addressed_storage( - &Url::parse("https://example.com/storage/user/pub/file.txt").unwrap() - )); - assert!(!is_path_addressed_storage( - &Url::parse("https://example.com/storage-user/pub/file.txt").unwrap() - )); - assert!(!is_path_addressed_storage( - &Url::parse("https://example.com/session").unwrap() - )); + fn classifies_transport_hosts() { + let public_key = crate::Keypair::random().public_key(); + let z32 = public_key.z32(); + + assert_eq!( + classify_transport_host(&format!("_pubky.{z32}")).unwrap(), + TransportHost::PubkyQname(public_key.clone()) + ); + assert_eq!( + classify_transport_host(&z32).unwrap(), + TransportHost::BarePublicKey(public_key) + ); + assert_eq!( + classify_transport_host("example.com").unwrap(), + TransportHost::Other + ); + assert_eq!( + classify_transport_host("_pubky.example.com").unwrap(), + TransportHost::Other + ); + } + + #[test] + fn rejects_pubky_prefixed_transport_hosts() { + let prefixed = crate::Keypair::random().public_key().to_string(); + + for host in [prefixed.clone(), format!("_pubky.{prefixed}")] { + let error = classify_transport_host(&host).unwrap_err(); + assert!(error.to_string().contains("use raw z32")); + } } #[test] @@ -74,4 +153,29 @@ mod tests { assert_eq!(url.host_str(), Some(expected_host.as_str())); assert_eq!(url.path(), "/auth/grant/session"); } + + #[tokio::test] + async fn path_addressed_fetch_is_a_pubky_target_without_a_header() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = crate::Keypair::random().public_key(); + let owner = crate::Keypair::random().public_key(); + client.features.insert( + &homeserver, + &[pubky_common::constants::features::PATH_ADDRESSED_STORAGE], + ); + let mut url = Url::parse(&format!( + "https://{}/storage/{}/pub/file.txt", + homeserver.z32(), + owner.z32() + )) + .unwrap(); + + let prepared = client.prepare_fetch(&mut url).await.unwrap(); + + assert!(prepared.is_pubky_target); + assert_eq!(prepared.pubky_host_header, None); + } } diff --git a/pubky-sdk/src/client/http_targets/native.rs b/pubky-sdk/src/client/http_targets/native.rs index bbd4442b9..2f9fad45c 100644 --- a/pubky-sdk/src/client/http_targets/native.rs +++ b/pubky-sdk/src/client/http_targets/native.rs @@ -2,11 +2,10 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, PoisonError, RwLock}; use std::time::{Duration, Instant}; -use super::{RequestAddressing, homeserver_url, is_path_addressed_storage}; +use super::{TransportHost, classify_transport_host, homeserver_url}; use futures_util::StreamExt; use tokio::net::TcpStream; -use crate::errors::RequestError; use crate::{PubkyHttpClient, PublicKey, Result, cross_log}; use reqwest::{IntoUrl, Method, RequestBuilder}; use url::Url; @@ -189,7 +188,7 @@ impl PubkyHttpClient { self.cross_request(method, url).await } - /// Route through `homeserver` while addressing `pubky_host`. + /// Route an authority-addressed endpoint through `homeserver` for `pubky_host`. pub(crate) async fn cross_request_via_homeserver( &self, method: Method, @@ -202,14 +201,12 @@ impl PubkyHttpClient { let pubky_host_z32 = pubky_host.z32(); let transport = self.transport.resolve(&homeserver_z32, &self.pkarr).await; - let pubky_host = (!is_path_addressed_storage(&url)).then_some(pubky_host_z32); - self.build_request( method, ResolvedRequest { url, transport: RequestTransport::Pubky(transport), - pubky_host, + pubky_host: Some(pubky_host_z32), }, ) } @@ -262,12 +259,9 @@ impl PubkyHttpClient { } async fn resolve_request(&self, mut url: Url) -> Result { - let addressing = self.prepare_request_addressing(&mut url).await?; - let Some(pubky_host) = self.prepare_transport_request(&mut url).await? else { - let pubky_host = match addressing { - RequestAddressing::LegacyStorage { owner } => Some(owner), - RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => None, - }; + let (addressing, transport_pubky_host) = self.prepare_request_parts(&mut url).await?; + let Some(pubky_host) = transport_pubky_host else { + let pubky_host = addressing.into_pubky_host(None); return Ok(ResolvedRequest { url, @@ -279,15 +273,9 @@ impl PubkyHttpClient { // `_pubky.` endpoints live under the full qname, not the bare key apex. let qname = url.host_str().unwrap_or(&pubky_host).to_string(); let transport = self.transport.resolve(&qname, &self.pkarr).await; - let pubky_host = match addressing { - RequestAddressing::Standard - if matches!(&transport, ResolvedTransport::Icann { .. }) => - { - Some(pubky_host) - } - RequestAddressing::LegacyStorage { owner } => Some(owner), - RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => None, - }; + let standard_pubky_host = + matches!(&transport, ResolvedTransport::Icann { .. }).then_some(pubky_host); + let pubky_host = addressing.into_pubky_host(standard_pubky_host); Ok(ResolvedRequest { url, @@ -296,52 +284,19 @@ impl PubkyHttpClient { }) } - /// Prepare a URL for transport and return its `pubky-host` value when applicable. - /// - /// # Errors - /// Returns a validation or resolution error if the URL cannot be prepared. - pub async fn prepare_request(&self, url: &mut Url) -> Result> { - let addressing = self.prepare_request_addressing(url).await?; - let pubky_host = self.prepare_transport_request(url).await?; - - Ok(match addressing { - RequestAddressing::LegacyStorage { owner } => Some(owner), - RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => pubky_host, - }) - } - - #[allow( + #[expect( clippy::unused_async, reason = "keep async signature aligned with WASM build" )] - async fn prepare_transport_request(&self, url: &mut Url) -> Result> { - let host = url.host_str().unwrap_or(""); - - if let Some(stripped) = host.strip_prefix("_pubky.") { - if PublicKey::is_pubky_prefixed(stripped) { - return Err(RequestError::Validation { - message: "pubky prefix is not allowed in transport hosts; use raw z32" - .to_string(), - } - .into()); - } - if PublicKey::try_from_z32(stripped).is_ok() { - return Ok(Some(stripped.to_string())); - } - } else { - if PublicKey::is_pubky_prefixed(host) { - return Err(RequestError::Validation { - message: "pubky prefix is not allowed in transport hosts; use raw z32" - .to_string(), - } - .into()); - } - if PublicKey::try_from_z32(host).is_ok() { - return Ok(Some(host.to_string())); + pub(super) async fn prepare_transport_request(&self, url: &mut Url) -> Result> { + let public_key = match classify_transport_host(url.host_str().unwrap_or_default())? { + TransportHost::PubkyQname(public_key) | TransportHost::BarePublicKey(public_key) => { + public_key } - } + TransportHost::Other => return Ok(None), + }; - Ok(None) + Ok(Some(public_key.z32())) } /// Start building a `Request` with the `Method` and `Url` (native-only). diff --git a/pubky-sdk/src/client/http_targets/storage.rs b/pubky-sdk/src/client/http_targets/storage.rs index ffdddd66b..26543b966 100644 --- a/pubky-sdk/src/client/http_targets/storage.rs +++ b/pubky-sdk/src/client/http_targets/storage.rs @@ -1,23 +1,37 @@ use url::Url; -use super::RequestAddressing; -use crate::{PubkyHttpClient, PublicKey, Result, errors::RequestError}; +use super::{TransportHost, classify_transport_host}; +use crate::{Pkdns, PubkyHttpClient, PublicKey, Result, errors::RequestError}; use pubky_common::constants::features::PATH_ADDRESSED_STORAGE; +#[derive(Debug, PartialEq, Eq)] +pub(super) enum StorageAddressing { + Standard, + PathAddressedStorage, + LegacyStorage { owner: String }, +} + +impl StorageAddressing { + pub(super) fn into_pubky_host(self, standard: Option) -> Option { + match self { + Self::Standard => standard, + Self::PathAddressedStorage => None, + Self::LegacyStorage { owner } => Some(owner), + } + } +} + impl PubkyHttpClient { - pub(super) async fn prepare_request_addressing( + pub(super) async fn prepare_storage_addressing( &self, url: &mut Url, - ) -> Result { + ) -> Result { let Some(path) = url.path().strip_prefix("/storage/") else { - return Ok(RequestAddressing::Standard); + return Ok(StorageAddressing::Standard); }; - let Some(host) = url.host_str() else { - return Ok(RequestAddressing::Standard); - }; - let transport_host = host.strip_prefix("_pubky.").unwrap_or(host); - if PublicKey::try_from_z32(transport_host).is_err() { - return Ok(RequestAddressing::Standard); + let transport_host = classify_transport_host(url.host_str().unwrap_or_default())?; + if transport_host == TransportHost::Other { + return Ok(StorageAddressing::Standard); } let (owner, path) = path .split_once('/') @@ -28,22 +42,28 @@ impl PubkyHttpClient { message: "path-addressed storage URL contains an invalid owner".to_string(), })?; let legacy_path = format!("/{path}"); - let homeserver = url - .host_str() - .filter(|host| !host.starts_with("_pubky.")) - .and_then(|host| PublicKey::try_from_z32(host).ok()); + let homeserver = match transport_host { + TransportHost::BarePublicKey(homeserver) => Some(homeserver), + TransportHost::PubkyQname(_) => Pkdns::with_client(self.clone()) + .get_homeserver_of(&owner) + .await + .ok() + .flatten(), + TransportHost::Other => None, + }; - if self - .features - .supports(self, &owner, homeserver.as_ref(), PATH_ADDRESSED_STORAGE) - .await + if let Some(homeserver) = homeserver + && self + .features + .supports(self, &homeserver, PATH_ADDRESSED_STORAGE) + .await { - return Ok(RequestAddressing::PathAddressedStorage); + return Ok(StorageAddressing::PathAddressedStorage); } // Choose compatibility before sending storage; response errors never trigger a retry. url.set_path(&legacy_path); - Ok(RequestAddressing::LegacyStorage { owner: owner.z32() }) + Ok(StorageAddressing::LegacyStorage { owner: owner.z32() }) } } @@ -70,9 +90,9 @@ mod tests { )) .unwrap(); - let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + let addressing = client.prepare_storage_addressing(&mut url).await.unwrap(); - assert_eq!(addressing, RequestAddressing::PathAddressedStorage); + assert_eq!(addressing, StorageAddressing::PathAddressedStorage); assert_eq!(url.path(), format!("/storage/{}/pub/file.txt", owner.z32())); } @@ -92,11 +112,11 @@ mod tests { )) .unwrap(); - let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + let addressing = client.prepare_storage_addressing(&mut url).await.unwrap(); assert_eq!( addressing, - RequestAddressing::LegacyStorage { owner: owner.z32() } + StorageAddressing::LegacyStorage { owner: owner.z32() } ); assert_eq!(url.path(), "/pub/My%20File%252FName/"); assert_eq!(url.query(), Some("cursor=hello%20world")); @@ -117,9 +137,9 @@ mod tests { let mut url = Url::parse(url).unwrap(); let original = url.clone(); - let addressing = client.prepare_request_addressing(&mut url).await.unwrap(); + let addressing = client.prepare_storage_addressing(&mut url).await.unwrap(); - assert_eq!(addressing, RequestAddressing::Standard); + assert_eq!(addressing, StorageAddressing::Standard); assert_eq!(url, original); } } @@ -140,9 +160,65 @@ mod tests { format!("https://{}/storage/missing-path", homeserver.z32()), ] { client - .prepare_request_addressing(&mut Url::parse(&url).unwrap()) + .prepare_storage_addressing(&mut Url::parse(&url).unwrap()) .await .unwrap_err(); } } + + #[tokio::test] + async fn unresolved_owner_uses_legacy_storage() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let owner = Keypair::random().public_key(); + let mut url = Url::parse(&format!( + "https://_pubky.{}/storage/{}/pub/file.txt", + owner.z32(), + owner.z32() + )) + .unwrap(); + + let addressing = client.prepare_storage_addressing(&mut url).await.unwrap(); + + assert_eq!( + addressing, + StorageAddressing::LegacyStorage { owner: owner.z32() } + ); + assert_eq!(url.path(), "/pub/file.txt"); + } + + #[test] + fn addressing_selects_the_pubky_host_header() { + let fallback = || Some("transport".to_string()); + + assert_eq!( + StorageAddressing::Standard.into_pubky_host(fallback()), + fallback() + ); + assert_eq!(StorageAddressing::Standard.into_pubky_host(None), None); + assert_eq!( + StorageAddressing::PathAddressedStorage.into_pubky_host(fallback()), + None + ); + assert_eq!( + StorageAddressing::PathAddressedStorage.into_pubky_host(None), + None + ); + assert_eq!( + StorageAddressing::LegacyStorage { + owner: "owner".to_string() + } + .into_pubky_host(fallback()), + Some("owner".to_string()) + ); + assert_eq!( + StorageAddressing::LegacyStorage { + owner: "owner".to_string() + } + .into_pubky_host(None), + Some("owner".to_string()) + ); + } } diff --git a/pubky-sdk/src/client/http_targets/wasm.rs b/pubky-sdk/src/client/http_targets/wasm.rs index facaf9538..b83f46d88 100644 --- a/pubky-sdk/src/client/http_targets/wasm.rs +++ b/pubky-sdk/src/client/http_targets/wasm.rs @@ -1,8 +1,8 @@ //! HTTP methods that support `https://` with Pkarr domains, including `_pubky.` URLs -use super::{RequestAddressing, homeserver_url, is_path_addressed_storage}; +use super::{TransportHost, classify_transport_host, homeserver_url}; use crate::PublicKey; -use crate::errors::{PkarrError, RequestError, Result}; +use crate::errors::{PkarrError, Result}; use crate::{PubkyHttpClient, cross_log}; use futures_lite::StreamExt; use pkarr::dns::rdata::SVCParam; @@ -17,20 +17,6 @@ enum AmbientCredentials { } impl PubkyHttpClient { - fn attach_pubky_host( - request: RequestBuilder, - url: &Url, - pubky_host: Option, - ) -> RequestBuilder { - if let Some(pubky_host) = pubky_host - && !is_path_addressed_storage(url) - { - request.header("pubky-host", pubky_host) - } else { - request - } - } - /// A wrapper around [`PubkyHttpClient::request`], with the same signature between native and WASM. pub(crate) async fn cross_request( &self, @@ -51,7 +37,7 @@ impl PubkyHttpClient { .await } - /// Route through `homeserver` while addressing `pubky_host`. + /// Route an authority-addressed endpoint through `homeserver` for `pubky_host`. pub(crate) async fn cross_request_via_homeserver( &self, method: Method, @@ -60,17 +46,13 @@ impl PubkyHttpClient { path: &str, ) -> Result { let mut url = homeserver_url(homeserver, path)?; - self.prepare_request(&mut url).await?; + self.prepare_transport_request(&mut url).await?; - let request = self + Ok(self .http .request(method, url.clone()) - .fetch_credentials_include(); - Ok(Self::attach_pubky_host( - request, - &url, - Some(pubky_host.z32()), - )) + .fetch_credentials_include() + .header("pubky-host", pubky_host.z32())) } pub(super) async fn homeserver_info_request( @@ -93,63 +75,30 @@ impl PubkyHttpClient { let original_url = url.as_str(); let mut url = Url::parse(original_url)?; - let pubky_host = self.prepare_request(&mut url).await?; + let prepared = self.prepare_fetch(&mut url).await?; let request = self.http.request(method, url.clone()); - let builder = match credentials { + let request = match credentials { AmbientCredentials::Include => request.fetch_credentials_include(), AmbientCredentials::Omit => request.fetch_credentials_omit(), }; - Ok(Self::attach_pubky_host(builder, &url, pubky_host)) - } - - /// Prepare a URL for transport and return its `pubky-host` value when applicable. - /// - /// # Errors - /// Returns a validation or resolution error if the URL cannot be prepared. - pub async fn prepare_request(&self, url: &mut Url) -> Result> { - let addressing = self.prepare_request_addressing(url).await?; - let pubky_host = self.prepare_transport_request(url).await?; - - Ok(match addressing { - RequestAddressing::LegacyStorage { owner } => Some(owner), - RequestAddressing::Standard | RequestAddressing::PathAddressedStorage => pubky_host, + Ok(match prepared.pubky_host_header { + Some(pubky_host) => request.header("pubky-host", pubky_host), + None => request, }) } - async fn prepare_transport_request(&self, url: &mut Url) -> Result> { - let host = url.host_str().unwrap_or("").to_string(); - - let mut pubky_host = None; - - if let Some(stripped) = host.strip_prefix("_pubky.") { - if PublicKey::is_pubky_prefixed(stripped) { - return Err(RequestError::Validation { - message: "pubky prefix is not allowed in transport hosts; use raw z32" - .to_string(), - } - .into()); - } - if PublicKey::try_from_z32(stripped).is_ok() { - self.transform_url(url).await?; - pubky_host = Some(stripped.to_string()); - } - } else { - if PublicKey::is_pubky_prefixed(&host) { - return Err(RequestError::Validation { - message: "pubky prefix is not allowed in transport hosts; use raw z32" - .to_string(), - } - .into()); + pub(super) async fn prepare_transport_request(&self, url: &mut Url) -> Result> { + let public_key = match classify_transport_host(url.host_str().unwrap_or_default())? { + TransportHost::PubkyQname(public_key) | TransportHost::BarePublicKey(public_key) => { + public_key } - if PublicKey::try_from_z32(&host).is_ok() { - self.transform_url(url).await?; - pubky_host = Some(host); - } - } + TransportHost::Other => return Ok(None), + }; - Ok(pubky_host) + self.transform_url(url).await?; + Ok(Some(public_key.z32())) } async fn transform_url(&self, url: &mut Url) -> Result<()> { @@ -263,43 +212,6 @@ mod tests { wasm_bindgen_test_configure!(run_in_browser); - #[wasm_bindgen_test] - fn storage_request_preserves_path_and_query_without_pubky_host() { - let client = PubkyHttpClient::new().unwrap(); - let owner = Keypair::random().public_key().z32(); - let url = Url::parse(&format!( - "https://example.com/storage/{owner}/pub/file.txt?cursor=hello%20world" - )) - .unwrap(); - let request = PubkyHttpClient::attach_pubky_host( - client.http.request(Method::GET, url.clone()), - &url, - Some(owner), - ) - .build() - .unwrap(); - - assert!(request.headers().get("pubky-host").is_none()); - assert!(request.url().path().starts_with("/storage/")); - assert_eq!(request.url().query(), Some("cursor=hello%20world")); - } - - #[wasm_bindgen_test] - fn cookie_session_request_keeps_pubky_host() { - let client = PubkyHttpClient::new().unwrap(); - let owner = Keypair::random().public_key().z32(); - let url = Url::parse("https://example.com/session").unwrap(); - let request = PubkyHttpClient::attach_pubky_host( - client.http.request(Method::POST, url.clone()), - &url, - Some(owner.clone()), - ) - .build() - .unwrap(); - - assert_eq!(request.headers().get("pubky-host").unwrap(), &owner); - } - #[wasm_bindgen_test(async)] async fn transform_url_errors_when_no_domain_is_found() { let client = PubkyHttpClient::new().unwrap(); From 31d5c4e36f6d5b07d22bc3d2e6461afb7ad629f0 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Fri, 14 Aug 2026 09:43:18 -0300 Subject: [PATCH 08/10] fix: validate owner = storage path endpoint user --- pubky-sdk/src/client/http_targets/storage.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pubky-sdk/src/client/http_targets/storage.rs b/pubky-sdk/src/client/http_targets/storage.rs index 26543b966..eae7e55ae 100644 --- a/pubky-sdk/src/client/http_targets/storage.rs +++ b/pubky-sdk/src/client/http_targets/storage.rs @@ -41,6 +41,15 @@ impl PubkyHttpClient { let owner = PublicKey::try_from_z32(owner).map_err(|_error| RequestError::Validation { message: "path-addressed storage URL contains an invalid owner".to_string(), })?; + if let TransportHost::PubkyQname(host_owner) = &transport_host + && host_owner != &owner + { + return Err(RequestError::Validation { + message: "path-addressed storage URL owner does not match its `_pubky` host" + .to_string(), + } + .into()); + } let legacy_path = format!("/{path}"); let homeserver = match transport_host { TransportHost::BarePublicKey(homeserver) => Some(homeserver), @@ -151,6 +160,7 @@ mod tests { .build() .unwrap(); let homeserver = Keypair::random().public_key(); + let owner = Keypair::random().public_key(); for url in [ format!( @@ -158,6 +168,11 @@ mod tests { homeserver.z32() ), format!("https://{}/storage/missing-path", homeserver.z32()), + format!( + "https://_pubky.{}/storage/{}/pub/file.txt", + homeserver.z32(), + owner.z32() + ), ] { client .prepare_storage_addressing(&mut Url::parse(&url).unwrap()) From de3998b497d0be6daaacb3bd3a27150482fa47c0 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Fri, 14 Aug 2026 13:05:49 -0300 Subject: [PATCH 09/10] fix: fixes from review --- docs/v0.10-migration/README.md | 7 ++ pubky-sdk/README.md | 29 +++++-- pubky-sdk/src/client/core.rs | 21 ++++- pubky-sdk/src/client/http_targets/features.rs | 79 ++++++++++++------- pubky-sdk/src/client/http_targets/mod.rs | 38 +++++++-- pubky-sdk/src/client/http_targets/native.rs | 4 + 6 files changed, 133 insertions(+), 45 deletions(-) diff --git a/docs/v0.10-migration/README.md b/docs/v0.10-migration/README.md index 3789c8669..8c67eafaa 100644 --- a/docs/v0.10-migration/README.md +++ b/docs/v0.10-migration/README.md @@ -34,6 +34,13 @@ match pubky.get_homeserver_of(&user).await { JavaScript's `getHomeserverOf()` return type is unchanged, but its promise now rejects with `PkarrError` for these failures. The same errors may surface from sign-in, grant exchange, and event-stream subscriptions that resolve a homeserver internally. +## Storage Transport Compatibility + +v0.10 represents public resources as `/storage/{owner}/...` transport URLs. The Rust storage APIs and JavaScript `Client.fetch` query the homeserver's `/info` endpoint and fall back to the legacy path plus `pubky-host` header when `path-addressed-storage` is unavailable. Storage requests made through these APIs remain compatible with v0.9 homeservers. + +If you send a URL from `resolve_pubky` or `PubkyResource::to_transport_url` yourself, call `PubkyHttpClient::prepare_request` and apply its returned `pubky-host` header before sending. A different HTTP client can use the canonical URL only with a homeserver that advertises `path-addressed-storage`. + + ## Deep Link Parsing If your Rust app parses sign-in or sign-up deep links directly, the parameter accessors changed. diff --git a/pubky-sdk/README.md b/pubky-sdk/README.md index 79d5ff105..29e2ca587 100644 --- a/pubky-sdk/README.md +++ b/pubky-sdk/README.md @@ -164,20 +164,31 @@ See [Private Storage](../docs/PRIVATE_STORAGE.md) for `/priv/` access and events ### Resolve identifiers into transport URLs -Need to feed a public resource into a raw HTTP client? Use [`resolve_pubky`] to transform the human-facing identifier into the HTTPS homeserver URL: +Use [`resolve_pubky`] to turn a public resource identifier into its canonical HTTPS homeserver URL. When sending that URL through `PubkyHttpClient`, call `prepare_request` first so older homeservers receive the legacy path and `pubky-host` header when needed: -```rust -# use pubky::resolve_pubky; -# fn main() -> pubky::Result<()> { -let url = resolve_pubky("pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG")?; +```rust no_run +# use pubky::{PubkyHttpClient, resolve_pubky}; +# use reqwest::Method; +# async fn run() -> pubky::Result<()> { +let client = PubkyHttpClient::new()?; +let mut url = resolve_pubky("pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG")?; assert_eq!( url.as_str(), "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/storage/operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" ); + +let pubky_host = client.prepare_request(&mut url).await?; +let mut request = client.request(Method::GET, &url); +if let Some(pubky_host) = pubky_host { + request = request.header("pubky-host", pubky_host); +} +let response = request.send().await?; # Ok(()) # } ``` +The high-level Rust storage APIs and JavaScript `Client.fetch` do this automatically, so their storage requests remain compatible with older homeservers. Other HTTP clients can send the canonical `/storage/{owner}/...` URL only to homeservers that advertise `path-addressed-storage`. + ## PKDNS (Pkarr) Resolve another user’s homeserver (`_pubky` record), or publish your own via the signer. @@ -212,7 +223,7 @@ Request an authorization URL and await approval. 3. Await `await_approval()` to obtain a session-bound `PubkySession`, or `await_credential()` for a raw `GrantCredential`/`CookieCredential` that you can persist, inspect, or lift into a session later via `PubkySession::from_{grant,cookie}_credential`. ```rust -# use pubky::{Pubky, Capabilities, Keypair, AuthFlowKind}; +# use pubky::{AuthFlowKind, Capabilities, ClientId, Keypair, Pubky}; # async fn auth() -> pubky::Result<()> { let pubky = Pubky::new()?; @@ -223,7 +234,11 @@ let caps = Capabilities::builder() .finish(); // Start the flow using the default relay (see “Relay & reliability” below) -let flow = pubky.start_cookie_auth_flow(&caps, AuthFlowKind::signin())?; +let flow = pubky.start_grant_auth_flow( + &caps, + AuthFlowKind::signin(), + ClientId::new("example.com").expect("static client id is valid"), +)?; println!("Scan to sign in: {}", flow.authorization_url()); // On the signing device, approve with: signer.approve_auth(flow.authorization_url()).await?; diff --git a/pubky-sdk/src/client/core.rs b/pubky-sdk/src/client/core.rs index d99ec6e64..d24ea0455 100644 --- a/pubky-sdk/src/client/core.rs +++ b/pubky-sdk/src/client/core.rs @@ -260,10 +260,15 @@ impl PubkyHttpClientBuilder { icann_http_builder = icann_http_builder.pool_max_idle_per_host(max); } + #[cfg(not(target_arch = "wasm32"))] + let features = HomeserverFeatures::new(self.native_http.request_timeout); + #[cfg(target_arch = "wasm32")] + let features = HomeserverFeatures::default(); + Ok(PubkyHttpClient { pkarr, http: http_builder.build()?, - features: HomeserverFeatures::default(), + features, #[cfg(not(target_arch = "wasm32"))] icann_http: icann_http_builder.build()?, @@ -398,18 +403,26 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// actors (e.g., `Pubky`, `SessionStorage`, `PublicStorage`) or the JS bindings’ /// `client.fetch(..)` provided in `bindings/js`. /// -/// Fetching a Pubky resource via its transport URL: +/// Fetching a Pubky resource via its transport URL, including legacy homeserver support: /// ```no_run /// # use pubky::{PubkyHttpClient, Result}; /// # use reqwest::Method; +/// # use url::Url; /// # async fn run() -> Result<()> { /// # #[cfg(doctest)] /// # return Ok(()); /// let client = PubkyHttpClient::new()?; /// // Pubky App profile of user Pubky https://pubky.app/profile/ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y /// let user = "ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y"; -/// let url = format!("https://_pubky.{user}/storage/{user}/pub/pubky.app/profile.json"); -/// let resp = client.request(Method::GET, &url).send().await?; +/// let mut url = Url::parse(&format!( +/// "https://_pubky.{user}/storage/{user}/pub/pubky.app/profile.json" +/// ))?; +/// let pubky_host = client.prepare_request(&mut url).await?; +/// let mut request = client.request(Method::GET, &url); +/// if let Some(pubky_host) = pubky_host { +/// request = request.header("pubky-host", pubky_host); +/// } +/// let resp = request.send().await?; /// let info = resp.text().await?; /// # Ok(()) } /// ``` diff --git a/pubky-sdk/src/client/http_targets/features.rs b/pubky-sdk/src/client/http_targets/features.rs index 5a42b57c1..418da2fad 100644 --- a/pubky-sdk/src/client/http_targets/features.rs +++ b/pubky-sdk/src/client/http_targets/features.rs @@ -13,12 +13,13 @@ use crate::{PubkyHttpClient, PublicKey}; const MAX_INFO_BYTES: usize = 16 * 1024; const INFO_TIMEOUT: Duration = Duration::from_secs(5); -const FAILED_INFO_RETRY_INTERVAL: Duration = Duration::from_secs(60); +const INFO_CACHE_TTL: Duration = Duration::from_secs(60); type FeatureCell = Arc>>; -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub(crate) struct HomeserverFeatures { servers: Arc>>, + request_timeout: Duration, } #[derive(Deserialize)] @@ -27,29 +28,39 @@ struct InfoResponse { } #[derive(Debug)] -enum CachedFeatures { - Available(Vec), - UnavailableUntil(Instant), +struct CachedFeatures { + features: Vec, + expires_at: Instant, } impl CachedFeatures { fn current(&self) -> Option<&[String]> { - match self { - Self::Available(features) => Some(features), - Self::UnavailableUntil(retry_at) if Instant::now() < *retry_at => Some(&[]), - Self::UnavailableUntil(_) => None, - } + (Instant::now() < self.expires_at).then_some(self.features.as_slice()) + } +} + +impl Default for HomeserverFeatures { + fn default() -> Self { + Self::new(None) } } impl HomeserverFeatures { + pub(crate) fn new(request_timeout: Option) -> Self { + Self { + servers: Arc::default(), + request_timeout: request_timeout + .map_or(INFO_TIMEOUT, |timeout| timeout.min(INFO_TIMEOUT)), + } + } + pub(super) async fn supports( &self, client: &PubkyHttpClient, homeserver: &PublicKey, feature: &str, ) -> bool { - self.supports_for(homeserver, feature, || Self::fetch(client, homeserver)) + self.supports_for(homeserver, feature, || self.fetch(client, homeserver)) .await } @@ -69,22 +80,20 @@ impl HomeserverFeatures { return features.iter().any(|candidate| candidate == feature); } - let Some(features) = fetch().await else { - *cached = Some(CachedFeatures::UnavailableUntil( - Instant::now() + FAILED_INFO_RETRY_INTERVAL, - )); - return false; - }; + let features = fetch().await.unwrap_or_default(); let supports = features.iter().any(|candidate| candidate == feature); - *cached = Some(CachedFeatures::Available(features)); + *cached = Some(CachedFeatures { + features, + expires_at: Instant::now() + INFO_CACHE_TTL, + }); supports } - async fn fetch(client: &PubkyHttpClient, homeserver: &PublicKey) -> Option> { + async fn fetch(&self, client: &PubkyHttpClient, homeserver: &PublicKey) -> Option> { let Ok(request) = client.homeserver_info_request(homeserver).await else { return None; }; - let Ok(response) = request.timeout(INFO_TIMEOUT).send().await else { + let Ok(response) = request.timeout(self.request_timeout).send().await else { return None; }; if !response.status().is_success() { @@ -130,9 +139,10 @@ impl HomeserverFeatures { cached.is_none(), "homeserver features were already initialized" ); - *cached = Some(CachedFeatures::Available( - features.iter().map(ToString::to_string).collect(), - )); + *cached = Some(CachedFeatures { + features: features.iter().map(ToString::to_string).collect(), + expires_at: Instant::now() + INFO_CACHE_TTL, + }); } } @@ -178,6 +188,18 @@ mod tests { assert_eq!(body.len(), MAX_INFO_BYTES); } + #[test] + fn info_timeout_respects_a_shorter_client_timeout() { + assert_eq!( + HomeserverFeatures::new(Some(Duration::from_millis(100))).request_timeout, + Duration::from_millis(100) + ); + assert_eq!( + HomeserverFeatures::new(Some(Duration::from_secs(10))).request_timeout, + INFO_TIMEOUT + ); + } + #[tokio::test] async fn temporarily_stores_failures_and_coalesces_feature_fetches() { let discovery = HomeserverFeatures::default(); @@ -213,18 +235,21 @@ mod tests { } #[tokio::test] - async fn retries_expired_failures() { + async fn refreshes_expired_features() { let discovery = HomeserverFeatures::default(); let homeserver = crate::Keypair::random().public_key(); let cell = discovery.cell(&homeserver); - *cell.lock().await = Some(CachedFeatures::UnavailableUntil(Instant::now())); + *cell.lock().await = Some(CachedFeatures { + features: vec![PATH_ADDRESSED_STORAGE.to_string()], + expires_at: Instant::now(), + }); let supported = discovery .supports_for(&homeserver, PATH_ADDRESSED_STORAGE, || async { - Some(vec![PATH_ADDRESSED_STORAGE.to_string()]) + Some(Vec::new()) }) .await; - assert!(supported); + assert!(!supported); } } diff --git a/pubky-sdk/src/client/http_targets/mod.rs b/pubky-sdk/src/client/http_targets/mod.rs index 013649710..10c8d9f6e 100644 --- a/pubky-sdk/src/client/http_targets/mod.rs +++ b/pubky-sdk/src/client/http_targets/mod.rs @@ -52,17 +52,16 @@ impl PubkyHttpClient { Ok((addressing, pubky_host)) } - /// Prepare a URL for transport and return its `pubky-host` value when applicable. + /// Prepare a URL before calling [`Self::request`] and return its `pubky-host` value. + /// + /// This may rewrite path-addressed storage URLs for legacy homeservers. When the + /// return value is `Some`, attach it to the request as the `pubky-host` header. /// /// # Errors /// Returns a validation or resolution error if the URL cannot be prepared. pub async fn prepare_request(&self, url: &mut Url) -> Result> { let (addressing, pubky_host) = self.prepare_request_parts(url).await?; - - Ok(match addressing { - StorageAddressing::LegacyStorage { owner } => Some(owner), - StorageAddressing::Standard | StorageAddressing::PathAddressedStorage => pubky_host, - }) + Ok(addressing.into_pubky_host(pubky_host)) } /// Prepare a URL and browser-fetch metadata for the JavaScript bindings. @@ -155,7 +154,7 @@ mod tests { } #[tokio::test] - async fn path_addressed_fetch_is_a_pubky_target_without_a_header() { + async fn path_addressed_requests_are_pubky_targets_without_a_header() { let client = PubkyHttpClient::builder() .isolated_pkarr_test() .build() @@ -173,9 +172,34 @@ mod tests { )) .unwrap(); + let mut native_url = url.clone(); + let pubky_host = client.prepare_request(&mut native_url).await.unwrap(); let prepared = client.prepare_fetch(&mut url).await.unwrap(); + assert_eq!(pubky_host, None); assert!(prepared.is_pubky_target); assert_eq!(prepared.pubky_host_header, None); } + + #[tokio::test] + async fn legacy_requests_rewrite_the_path_and_return_the_owner_header() { + let client = PubkyHttpClient::builder() + .isolated_pkarr_test() + .build() + .unwrap(); + let homeserver = crate::Keypair::random().public_key(); + let owner = crate::Keypair::random().public_key(); + client.features.insert(&homeserver, &[]); + let mut url = Url::parse(&format!( + "https://{}/storage/{}/pub/file.txt", + homeserver.z32(), + owner.z32() + )) + .unwrap(); + + let pubky_host = client.prepare_request(&mut url).await.unwrap(); + + assert_eq!(url.path(), "/pub/file.txt"); + assert_eq!(pubky_host, Some(owner.z32())); + } } diff --git a/pubky-sdk/src/client/http_targets/native.rs b/pubky-sdk/src/client/http_targets/native.rs index 2f9fad45c..c9d526622 100644 --- a/pubky-sdk/src/client/http_targets/native.rs +++ b/pubky-sdk/src/client/http_targets/native.rs @@ -304,6 +304,10 @@ impl PubkyHttpClient { /// Returns a `RequestBuilder`, which will allow setting headers and /// the request body before sending. /// + /// Call [`Self::prepare_request`] first when sending a path-addressed storage URL. + /// That async step negotiates homeserver features and returns any `pubky-host` + /// header needed by legacy homeservers. + /// /// Differs from [`reqwest::Client::request`], in that it can make requests to: /// 1. HTTPS URLs with a [`crate::PublicKey`] as top-level domain, by resolving /// corresponding endpoints, and verifying TLS certificates accordingly. From d3f916f09dd7bbcc054d7a95cf8abead2155a972 Mon Sep 17 00:00:00 2001 From: MCarlomagno Date: Fri, 14 Aug 2026 13:41:16 -0300 Subject: [PATCH 10/10] fix: add stricter caching policy + async request handler --- Cargo.lock | 1 + docs/v0.10-migration/README.md | 2 +- pubky-sdk/Cargo.toml | 1 + pubky-sdk/README.md | 11 ++--- pubky-sdk/src/client/core.rs | 25 +++++------ pubky-sdk/src/client/http_targets/features.rs | 42 +++++++++++++++++-- pubky-sdk/src/client/http_targets/mod.rs | 17 +++++++- pubky-sdk/src/client/http_targets/native.rs | 32 +++++++------- pubky-sdk/src/client/http_targets/wasm.rs | 2 +- 9 files changed, 89 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 54457ad6f..461f2d1a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4009,6 +4009,7 @@ dependencies = [ "httpdate", "httpmock", "log", + "lru 0.18.2", "percent-encoding", "pkarr", "pubky-common", diff --git a/docs/v0.10-migration/README.md b/docs/v0.10-migration/README.md index 8c67eafaa..4a4dd0de9 100644 --- a/docs/v0.10-migration/README.md +++ b/docs/v0.10-migration/README.md @@ -38,7 +38,7 @@ JavaScript's `getHomeserverOf()` return type is unchanged, but its promise now r v0.10 represents public resources as `/storage/{owner}/...` transport URLs. The Rust storage APIs and JavaScript `Client.fetch` query the homeserver's `/info` endpoint and fall back to the legacy path plus `pubky-host` header when `path-addressed-storage` is unavailable. Storage requests made through these APIs remain compatible with v0.9 homeservers. -If you send a URL from `resolve_pubky` or `PubkyResource::to_transport_url` yourself, call `PubkyHttpClient::prepare_request` and apply its returned `pubky-host` header before sending. A different HTTP client can use the canonical URL only with a homeserver that advertises `path-addressed-storage`. +Send URLs from `resolve_pubky` or `PubkyResource::to_transport_url` with `PubkyHttpClient::request_async`. It handles storage compatibility and native transport resolution. `prepare_request` remains available for callers that resolve transport themselves. A different HTTP client can use the canonical URL only with a homeserver that advertises `path-addressed-storage`. ## Deep Link Parsing diff --git a/pubky-sdk/Cargo.toml b/pubky-sdk/Cargo.toml index 52518910b..4ee8badbb 100644 --- a/pubky-sdk/Cargo.toml +++ b/pubky-sdk/Cargo.toml @@ -36,6 +36,7 @@ percent-encoding.workspace = true cookie = "0.18" flume = { version = "0.11", default-features = false, features = ["async"] } futures-util.workspace = true +lru = { version = "0.18", default-features = false } serde.workspace = true serde_json.workspace = true httpdate.workspace = true diff --git a/pubky-sdk/README.md b/pubky-sdk/README.md index 29e2ca587..b2d0c2383 100644 --- a/pubky-sdk/README.md +++ b/pubky-sdk/README.md @@ -164,25 +164,20 @@ See [Private Storage](../docs/PRIVATE_STORAGE.md) for `/priv/` access and events ### Resolve identifiers into transport URLs -Use [`resolve_pubky`] to turn a public resource identifier into its canonical HTTPS homeserver URL. When sending that URL through `PubkyHttpClient`, call `prepare_request` first so older homeservers receive the legacy path and `pubky-host` header when needed: +Use [`resolve_pubky`] to turn a public resource identifier into its canonical HTTPS homeserver URL. `PubkyHttpClient::request_async` resolves the transport and falls back to legacy storage addressing when needed: ```rust no_run # use pubky::{PubkyHttpClient, resolve_pubky}; # use reqwest::Method; # async fn run() -> pubky::Result<()> { let client = PubkyHttpClient::new()?; -let mut url = resolve_pubky("pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG")?; +let url = resolve_pubky("pubkyoperrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG")?; assert_eq!( url.as_str(), "https://_pubky.operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/storage/operrr8wsbpr3ue9d4qj41ge1kcc6r7fdiy6o3ugjrrhi4y77rdo/pub/pubky.app/posts/0033X02JAN0SG" ); -let pubky_host = client.prepare_request(&mut url).await?; -let mut request = client.request(Method::GET, &url); -if let Some(pubky_host) = pubky_host { - request = request.header("pubky-host", pubky_host); -} -let response = request.send().await?; +let response = client.request_async(Method::GET, url).await?.send().await?; # Ok(()) # } ``` diff --git a/pubky-sdk/src/client/core.rs b/pubky-sdk/src/client/core.rs index d24ea0455..0366915ba 100644 --- a/pubky-sdk/src/client/core.rs +++ b/pubky-sdk/src/client/core.rs @@ -345,8 +345,8 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// /// ### What it does /// - Detects pkarr public-key hosts and resolves them to concrete endpoints. -/// - Internally, uses a unified `cross_request(..)` that works the same on native rust and -/// WASM (WASM performs endpoint resolution & header injection; native is a thin wrapper). +/// - [`PubkyHttpClient::request_async`] resolves Pubky transport and storage compatibility +/// on native Rust and WASM. /// /// ### What it *doesn’t* do /// - It is **not** session/identity aware. No cookies, no per-user scoping. @@ -369,9 +369,9 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// target public key—no CA chain involved. /// - **WASM:** /// - All requests use the browser’s standard X.509 TLS stack. -/// - For Pubky/PKDNS hosts, private method `cross_request(..)` resolves the -/// endpoint via PKARR, rewrites the URL (including testnet/localhost mapping), -/// and may add a `pubky-host` header to convey the intended public-key host. +/// - For Pubky/PKDNS hosts, [`PubkyHttpClient::request_async`] resolves the endpoint +/// via PKARR, rewrites the URL (including testnet/localhost mapping), and may add a +/// `pubky-host` header to convey the intended public-key host. /// /// ### Examples /// Basic construction. Works out of the box for mainline DHT pkarr endpoints. @@ -399,9 +399,9 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// # Ok(()) } /// ``` /// -/// Note: `request(..)` is available on native targets. On WASM, use the high-level -/// actors (e.g., `Pubky`, `SessionStorage`, `PublicStorage`) or the JS bindings’ -/// `client.fetch(..)` provided in `bindings/js`. +/// Note: `request(..)` is available only on native targets. `request_async(..)` is +/// available on native Rust and WASM. JavaScript callers use `client.fetch(..)` from +/// `bindings/js`. /// /// Fetching a Pubky resource via its transport URL, including legacy homeserver support: /// ```no_run @@ -414,15 +414,10 @@ fn icann_tls_config_without_revocation_check() -> rustls::ClientConfig { /// let client = PubkyHttpClient::new()?; /// // Pubky App profile of user Pubky https://pubky.app/profile/ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y /// let user = "ihaqcthsdbk751sxctk849bdr7yz7a934qen5gmpcbwcur49i97y"; -/// let mut url = Url::parse(&format!( +/// let url = Url::parse(&format!( /// "https://_pubky.{user}/storage/{user}/pub/pubky.app/profile.json" /// ))?; -/// let pubky_host = client.prepare_request(&mut url).await?; -/// let mut request = client.request(Method::GET, &url); -/// if let Some(pubky_host) = pubky_host { -/// request = request.header("pubky-host", pubky_host); -/// } -/// let resp = request.send().await?; +/// let resp = client.request_async(Method::GET, url).await?.send().await?; /// let info = resp.text().await?; /// # Ok(()) } /// ``` diff --git a/pubky-sdk/src/client/http_targets/features.rs b/pubky-sdk/src/client/http_targets/features.rs index 418da2fad..357d6bb09 100644 --- a/pubky-sdk/src/client/http_targets/features.rs +++ b/pubky-sdk/src/client/http_targets/features.rs @@ -1,10 +1,11 @@ use std::{ - collections::HashMap, + num::NonZeroUsize, sync::{Arc, Mutex, PoisonError}, time::Duration, }; use futures_util::StreamExt; +use lru::LruCache; use serde::Deserialize; use tokio::sync::Mutex as AsyncMutex; use web_time::Instant; @@ -14,11 +15,12 @@ use crate::{PubkyHttpClient, PublicKey}; const MAX_INFO_BYTES: usize = 16 * 1024; const INFO_TIMEOUT: Duration = Duration::from_secs(5); const INFO_CACHE_TTL: Duration = Duration::from_secs(60); +const INFO_CACHE_CAPACITY: usize = 256; type FeatureCell = Arc>>; #[derive(Debug, Clone)] pub(crate) struct HomeserverFeatures { - servers: Arc>>, + servers: Arc>>, request_timeout: Duration, } @@ -48,7 +50,10 @@ impl Default for HomeserverFeatures { impl HomeserverFeatures { pub(crate) fn new(request_timeout: Option) -> Self { Self { - servers: Arc::default(), + servers: Arc::new(Mutex::new(LruCache::new( + NonZeroUsize::new(INFO_CACHE_CAPACITY) + .expect("homeserver feature cache capacity is non-zero"), + ))), request_timeout: request_timeout .map_or(INFO_TIMEOUT, |timeout| timeout.min(INFO_TIMEOUT)), } @@ -66,7 +71,13 @@ impl HomeserverFeatures { fn cell(&self, homeserver: &PublicKey) -> FeatureCell { let mut servers = self.servers.lock().unwrap_or_else(PoisonError::into_inner); - Arc::clone(servers.entry(homeserver.clone()).or_default()) + if let Some(cell) = servers.get(homeserver) { + return Arc::clone(cell); + } + + let cell = FeatureCell::default(); + servers.put(homeserver.clone(), Arc::clone(&cell)); + cell } async fn supports_for(&self, homeserver: &PublicKey, feature: &str, fetch: F) -> bool @@ -200,6 +211,29 @@ mod tests { ); } + #[test] + fn evicts_the_least_recently_used_homeserver() { + let discovery = HomeserverFeatures::default(); + let homeservers = (0..=INFO_CACHE_CAPACITY) + .map(|_| crate::Keypair::random().public_key()) + .collect::>(); + + for homeserver in &homeservers[..INFO_CACHE_CAPACITY] { + drop(discovery.cell(homeserver)); + } + drop(discovery.cell(&homeservers[0])); + drop(discovery.cell(&homeservers[INFO_CACHE_CAPACITY])); + + let servers = discovery + .servers + .lock() + .unwrap_or_else(PoisonError::into_inner); + assert_eq!(servers.len(), INFO_CACHE_CAPACITY); + assert!(servers.contains(&homeservers[0])); + assert!(!servers.contains(&homeservers[1])); + assert!(servers.contains(&homeservers[INFO_CACHE_CAPACITY])); + } + #[tokio::test] async fn temporarily_stores_failures_and_coalesces_feature_fetches() { let discovery = HomeserverFeatures::default(); diff --git a/pubky-sdk/src/client/http_targets/mod.rs b/pubky-sdk/src/client/http_targets/mod.rs index 10c8d9f6e..03c781597 100644 --- a/pubky-sdk/src/client/http_targets/mod.rs +++ b/pubky-sdk/src/client/http_targets/mod.rs @@ -1,4 +1,5 @@ use crate::{PubkyHttpClient, PublicKey, Result, errors::RequestError}; +use reqwest::{Method, RequestBuilder}; use url::Url; mod features; @@ -42,6 +43,18 @@ fn classify_transport_host(host: &str) -> Result { } impl PubkyHttpClient { + /// Build a request after resolving its transport and storage addressing. + /// + /// Use this for Pubky and PKDNS URLs. On native targets it selects `PubkyTLS` or + /// ICANN/X.509 transport. It also falls back to legacy storage addressing when + /// the homeserver does not advertise path-addressed storage. + /// + /// # Errors + /// Returns a validation or resolution error if the request cannot be prepared. + pub async fn request_async(&self, method: Method, url: Url) -> Result { + self.cross_request(method, url).await + } + async fn prepare_request_parts( &self, url: &mut Url, @@ -52,10 +65,12 @@ impl PubkyHttpClient { Ok((addressing, pubky_host)) } - /// Prepare a URL before calling [`Self::request`] and return its `pubky-host` value. + /// Prepare a URL's storage addressing and return its `pubky-host` value. /// /// This may rewrite path-addressed storage URLs for legacy homeservers. When the /// return value is `Some`, attach it to the request as the `pubky-host` header. + /// Callers using `PubkyHttpClient` should prefer [`Self::request_async`], which also + /// resolves the native transport. /// /// # Errors /// Returns a validation or resolution error if the URL cannot be prepared. diff --git a/pubky-sdk/src/client/http_targets/native.rs b/pubky-sdk/src/client/http_targets/native.rs index c9d526622..a0d021af4 100644 --- a/pubky-sdk/src/client/http_targets/native.rs +++ b/pubky-sdk/src/client/http_targets/native.rs @@ -304,9 +304,8 @@ impl PubkyHttpClient { /// Returns a `RequestBuilder`, which will allow setting headers and /// the request body before sending. /// - /// Call [`Self::prepare_request`] first when sending a path-addressed storage URL. - /// That async step negotiates homeserver features and returns any `pubky-host` - /// header needed by legacy homeservers. + /// This synchronous method does not negotiate storage addressing or resolve ICANN + /// fallback endpoints. Use [`Self::request_async`] for Pubky and PKDNS URLs. /// /// Differs from [`reqwest::Client::request`], in that it can make requests to: /// 1. HTTPS URLs with a [`crate::PublicKey`] as top-level domain, by resolving @@ -314,9 +313,6 @@ impl PubkyHttpClient { /// (example: `https://o4dksfbqk85ogzdb5osziw6befigbuxmuxkuxq8434q89uj56uyy`) /// 2. `_pubky.` URLs like `https://_pubky.o4dksfbqk85ogzdb5osziw6befigbuxmuxkuxq8434q89uj56uyy` /// - /// # Errors - /// - /// This method fails whenever the supplied `Url` cannot be parsed. pub fn request(&self, method: Method, url: &U) -> RequestBuilder { let url_str = url.as_str(); @@ -515,12 +511,8 @@ mod tests { } } - /// Regression test: requests to `https://_pubky./...` must apply the - /// ICANN fallback. The user's endpoints are published under the - /// `_pubky.` qname (an alias to the homeserver key), so the - /// transport must be resolved with the full host, not the bare key. #[tokio::test] - async fn cross_request_pubky_host_falls_back_to_icann_when_direct_unreachable() { + async fn request_async_resolves_icann_path_addressed_storage() { // Homeserver apex: unreachable direct endpoint + reachable ICANN domain. let homeserver = Keypair::random(); let mut direct = SVCB::new(1, ".".try_into().unwrap()); @@ -553,9 +545,17 @@ mod tests { cache.put(&user.public_key().into(), &user_packet); let user_z32 = user.public_key().to_string(); - let url = Url::parse(&format!("https://_pubky.{user_z32}/session")).unwrap(); + let homeserver_pk = PublicKey::try_from_z32(&homeserver_z32).unwrap(); + client.features.insert( + &homeserver_pk, + &[pubky_common::constants::features::PATH_ADDRESSED_STORAGE], + ); + let url = Url::parse(&format!( + "https://_pubky.{user_z32}/storage/{user_z32}/pub/file.txt" + )) + .unwrap(); let req = client - .cross_request(Method::POST, url) + .request_async(Method::GET, url) .await .unwrap() .build() @@ -567,7 +567,11 @@ mod tests { "expected ICANN fallback for _pubky host, got {}", req.url() ); - assert_eq!(req.headers().get("pubky-host").unwrap(), &user_z32); + assert_eq!( + req.url().path(), + format!("/storage/{user_z32}/pub/file.txt") + ); + assert!(!req.headers().contains_key("pubky-host")); } #[tokio::test] diff --git a/pubky-sdk/src/client/http_targets/wasm.rs b/pubky-sdk/src/client/http_targets/wasm.rs index b83f46d88..4b503ca26 100644 --- a/pubky-sdk/src/client/http_targets/wasm.rs +++ b/pubky-sdk/src/client/http_targets/wasm.rs @@ -17,7 +17,7 @@ enum AmbientCredentials { } impl PubkyHttpClient { - /// A wrapper around [`PubkyHttpClient::request`], with the same signature between native and WASM. + /// Platform implementation for [`PubkyHttpClient::request_async`]. pub(crate) async fn cross_request( &self, method: Method,