From f4d66357a2e23fe6ab4f733ead6f589973863845 Mon Sep 17 00:00:00 2001 From: ehsan shariati Date: Sat, 22 Aug 2026 19:11:18 -0400 Subject: [PATCH 1/5] gateway: let a block Timeout reach the gc-recovery path The gateway already has a full gc-recovery system (recovery_fallback.rs: resolve the key->CID from the cluster pinset mirror and serve the block). Production could not reach it. recovery is invoked only from the miss branches gated on NotFound / Unavailable (handlers/object.rs). A read that fails with Timeout is not in that set, so: * try_recover_block never runs -- the cluster mirror that still holds the data is never consulted; * Timeout has no arm in ApiError::error_code(), so it hits the `_` catch-all -> InternalError -> HTTP 500; * and a 5xx trips the client SDK's health gate, which marks the ENTIRE backend down. That is precisely the outcome the deliberate Unavailable -> 410 mapping exists to prevent. Why Timeout survives instead of being rewritten to Unavailable: cluster_fallback::classify_exhaustion only converts it when the local daemon's offline probe returned a CLEAN miss. Read-only inspection of the live host found load ~12.8, fula-gateway at 12.4 GB RSS, and kubo unable to answer repo/stat within 30s while answering /id in 45ms -- so that probe errors out, `responsive` is false, and the Timeout passes through. A bucket whose data is still pinned in the cluster therefore 500s. Two changes: 1. Timeout joins NotFound/Unavailable in the recoverable-miss set. Errors that indicate the daemon is genuinely unreachable (Connection, IpfsApi, ...) are deliberately NOT in the set and still propagate as 5xx, so the health gate still trips on a true outage. 2. open_bucket_for_user is no longer called with a bare `?`. It reads the prolly INDEX ROOT -- exactly the block class an `ipfs repo gc` orphans -- so a root-block miss bypassed the recovery gate one match below: Unavailable became a 410 with no recovery attempted and no client by-CID fallback (which fires only on 404), and NotFound became a 500. Both now route through the same gate as an interior-node miss. The miss handling is factored into recover_or_nosuchkey() so the two call sites cannot drift. Trade-off, stated plainly: a genuinely transient infra timeout now attempts recovery and, failing that, surfaces as a 404 rather than a 500. Better for the client, but it does mean a real outage reads as "not found" on this path. NOTE: this makes a damaged bucket fail gracefully. It does not repair one, and it does not address why the host is resource-saturated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ANUDVTMmqZGFDjbj7k9HuP --- crates/fula-cli/src/handlers/object.rs | 120 +++++++++++++++++++------ 1 file changed, 92 insertions(+), 28 deletions(-) diff --git a/crates/fula-cli/src/handlers/object.rs b/crates/fula-cli/src/handlers/object.rs index 50709c8..9a4216c 100644 --- a/crates/fula-cli/src/handlers/object.rs +++ b/crates/fula-cli/src/handlers/object.rs @@ -691,6 +691,70 @@ async fn pin_for_user_via_queue( } } +/// Whether a `CoreError` raised while reading the prolly INDEX is a genuine +/// block-miss that the gc-recovery fallback should try to serve. +/// +/// `Timeout` is in this set deliberately (2026-08-23). Before, it was not, and +/// that single omission made the whole recovery architecture unreachable in +/// production: +/// +/// * `cluster_fallback::classify_exhaustion` only rewrites `Timeout` -> +/// `Unavailable` when the local daemon's offline probe returned a CLEAN +/// miss. On a loaded box that probe errors out instead, so `responsive` is +/// false and the `Timeout` survives. +/// * A surviving `Timeout` has no arm in `ApiError::error_code()`, so it hits +/// the `_` catch-all -> `InternalError` -> HTTP 500. +/// * A 5xx trips the client SDK's health gate, which marks the ENTIRE backend +/// down — the exact outcome the deliberate `Unavailable` -> 410 mapping +/// exists to avoid. +/// * And because `Timeout` was not a "miss", `try_recover_block` never ran, so +/// the cluster mirror that still holds the data was never consulted. +/// +/// Observed live: a gateway at load ~12.8, with kubo unable to answer +/// `repo/stat` inside 30s, returned 500 for a bucket whose data was still +/// pinned in the cluster. +/// +/// Trade-off: a genuinely transient infra timeout now attempts recovery and, +/// failing that, surfaces as a 404 the client can act on rather than a 500. +/// Better for the client, but it does mean a real outage reads as "not found" +/// on this path. Errors that indicate the daemon is actually unreachable +/// (`Connection`, `IpfsApi`, …) are NOT in this set and still propagate as +/// 5xx, so the health gate still trips on a true outage. +fn is_recoverable_index_miss(e: &fula_core::CoreError) -> bool { + matches!( + e, + fula_core::CoreError::BlockStore( + BlockStoreError::NotFound(_) + | BlockStoreError::Unavailable(_) + | BlockStoreError::Timeout { .. } + ) + ) +} + +/// Shared miss handler: try the server-side cluster-mirror recovery, then fall +/// back to a 404 the CLIENT's by-CID recovery can act on. +/// +/// Returning 404 (`NoSuchKey`) rather than 410 (`Gone`) is load-bearing: the +/// client's `get_object_with_recovery_known_cid` fires on a reachable-master +/// 404 and NEVER on 410, racing public IPFS for the CID hinted by its own +/// forest (Walkable-v8 `chunk_cids` / `storage_cid`). A genuinely-lost block +/// loses that race and surfaces a hard error; a still-reachable one is served. +async fn recover_or_nosuchkey( + state: &Arc, + bucket_name: &str, + key: &str, + headers: &HeaderMap, +) -> Result { + if let Some(rec) = crate::recovery_fallback::try_recover_block(state, bucket_name, key).await { + return Ok(recovery_block_response(rec.data, rec.cid, headers)); + } + Err(ApiError::s3_with_resource( + S3ErrorCode::NoSuchKey, + "Object not found (gc-orphaned index; client recovers by CID)", + format!("{}/{}", bucket_name, key), + )) +} + /// GET /{bucket}/{key} - Get object with Range and conditional request support pub async fn get_object( State(state): State>, @@ -703,9 +767,29 @@ pub async fn get_object( } // P12: scoped MCP tokens may only GET within their bucket + `ai/` prefix. session.assert_mcp_scope(&bucket_name, Some(&key), McpAction::Read)?; + // (helpers `is_recoverable_index_miss` / `recover_or_nosuchkey` are defined + // above this function.) - // User-scoped bucket access - let bucket = state.bucket_manager.open_bucket_for_user(&session.hashed_user_id, &bucket_name).await?; + // User-scoped bucket access. + // + // NOT a bare `?`. `open_bucket_for_user` reads the prolly INDEX ROOT + // (`ProllyTree::load` -> `store.get_ipld(root_cid)`) — exactly the block + // class an `ipfs repo gc` orphans. A bare `?` handed that miss straight to + // the error mapper, so a ROOT-block miss never reached the recovery gate in + // the match below: `Unavailable` became a 410 with no recovery attempted + // and no client by-CID fallback (which fires only on 404), and `NotFound` + // became a 500. Route it through the same gate as an interior-node miss. + let bucket = match state + .bucket_manager + .open_bucket_for_user(&session.hashed_user_id, &bucket_name) + .await + { + Ok(b) => b, + Err(e) if is_recoverable_index_miss(&e) => { + return recover_or_nosuchkey(&state, &bucket_name, &key, &headers).await; + } + Err(e) => return Err(e.into()), + }; let metadata = match bucket.get_object(&key).await { Ok(Some(m)) => m, @@ -719,36 +803,16 @@ pub async fn get_object( // (auth / infra / etc.) is propagated unchanged (codex: don't recover // on errors that aren't a genuine block-miss). other => { - let is_index_miss = matches!( - &other, - Ok(None) - | Err(fula_core::CoreError::BlockStore( - BlockStoreError::NotFound(_) | BlockStoreError::Unavailable(_) - )) - ); + let is_index_miss = match &other { + Ok(None) => true, + Err(e) => is_recoverable_index_miss(e), + Ok(Some(_)) => false, // handled by the arm above + }; if !is_index_miss { // Auth / infra / other — propagate unchanged, no recovery. return Err(other.unwrap_err().into()); } - // Server-side cluster-mirror fallback first. - if let Some(rec) = - crate::recovery_fallback::try_recover_block(&state, &bucket_name, &key).await - { - return Ok(recovery_block_response(rec.data, rec.cid, &headers)); - } - // The gateway can't serve it (not in the cluster mirror) — but the - // block may still be reachable BY CID via the CLIENT's forest hints - // (Walkable-v8 `chunk_cids` / `storage_cid`). Return 404 NoSuchKey - // (NOT 410 Gone) so the client's by-CID recovery - // (`get_object_with_recovery_known_cid`) engages — it fires on a - // reachable-master 404 and NEVER on 410, racing public IPFS for the - // hinted CID. A genuinely-lost block then fails that race and - // surfaces a hard error; a still-reachable one is recovered. - return Err(ApiError::s3_with_resource( - S3ErrorCode::NoSuchKey, - "Object not found (gc-orphaned index; client recovers by CID)", - format!("{}/{}", bucket_name, key), - )); + return recover_or_nosuchkey(&state, &bucket_name, &key, &headers).await; } }; From 984d6ed73bf6bc1d2b16edaf64f43032f2d8de44 Mon Sep 17 00:00:00 2001 From: ehsan shariati Date: Sat, 22 Aug 2026 19:12:12 -0400 Subject: [PATCH 2/5] bridge: drop the per-client RwLock; bound wasm requests One slow bucket froze the entire web app. EncryptedClientHandle wrapped the client in Arc>, load_forest took the EXCLUSIVE guard and held it across the whole network fetch, and every other call -- on every other bucket -- queued behind it. Captured live on a phone: loadForest: ENTER tag-metadata <- ~30s, holds the write guard getFlat: ENTER website-metadata-v8/.. <- never completes loadForest: ENTER tag-metadata-v8 <- never completes (healthy bucket, normally 0.1s) WebsiteDetail: loadWebsites ENTER <- never completes A Dart-side .timeout() cannot rescue that: the FRB binding exposes no cancel handle for load_forest, so the Rust future keeps running and keeps the guard after the caller has given up. The fix is to DELETE the lock, not to make it per-bucket: * EncryptedClient has no &mut self methods -- all ~180 public methods take &self. The lock never protected mutability. * The SDK already owns finer-grained locking: forest_cache (DashMap), per-bucket migration_locks, and bucket_write_mutex, documented there as "the OUTERMOST per-bucket lock ... per-bucket, so different buckets continue to flush in parallel". A per-bucket lock at the bridge would duplicate that one layer up -- and would not even cover the calls that carry no bucket (get_public_key, export_secret_key). * The read/write split encoded no invariant: save_forest (read) and flush_forest (write) had byte-identical bodies; rewrap_object took write() for ONE object while rotate_bucket took read() for a whole bucket. * No bridge function composed two SDK calls under one guard, so nothing depended on atomicity it appeared to provide. * FulaClientHandle next door already uses a bare Arc -- in-repo precedent. 57 guard acquisitions removed (forest 27, encrypted 18, sharing 5, rotation 4, chunked 3); Arc derefs to &EncryptedClient so every downstream `guard.` call is untouched. Also: wasm had NO request timeout at all. reqwest's ClientBuilder::timeout is a no-op on wasm32, so FulaClient::new could not arm it and Config::timeout was silently dropped in the browser -- a stalled request could hang for the lifetime of the tab, holding the SDK's per-bucket lock (and, before this change, the whole client). send_bounded now races the send against a gloo-timers timer on wasm and stays a plain send() on native, where reqwest already enforces the budget. Dropping the abandoned future actually aborts the fetch, rather than orphaning it the way a Dart-side timeout does. New ClientError::Timeout maps to FulaError::Network, matching how reqwest's own native timeout already surfaces, so both targets present a transport failure identically to Dart. Verified: cargo check clean on native AND wasm32-unknown-unknown. NOT yet reviewed for concurrency regressions -- removing a global lock increases parallelism and can surface latent SDK races. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ANUDVTMmqZGFDjbj7k9HuP --- crates/fula-client/src/client.rs | 41 ++++++++++++++++- crates/fula-client/src/error.rs | 13 ++++++ crates/fula-flutter/src/api/chunked.rs | 8 ++-- crates/fula-flutter/src/api/client.rs | 12 ++--- crates/fula-flutter/src/api/encrypted.rs | 38 ++++++++-------- crates/fula-flutter/src/api/error.rs | 10 +++- crates/fula-flutter/src/api/forest.rs | 58 ++++++++++++------------ crates/fula-flutter/src/api/rotation.rs | 10 ++-- crates/fula-flutter/src/api/sharing.rs | 12 ++--- crates/fula-flutter/src/api/types.rs | 45 +++++++++++++++--- 10 files changed, 166 insertions(+), 81 deletions(-) diff --git a/crates/fula-client/src/client.rs b/crates/fula-client/src/client.rs index 1b2086b..b9cc01d 100644 --- a/crates/fula-client/src/client.rs +++ b/crates/fula-client/src/client.rs @@ -1476,6 +1476,41 @@ impl FulaClient { // ==================== Helper Methods ==================== + /// Send a prepared request, enforcing `Config::timeout` on every target. + /// + /// On native this is a plain `send()` - `FulaClient::new` already armed + /// `ClientBuilder::timeout`. + /// + /// On **wasm32 that builder call is a no-op**, so the browser client ran + /// with NO request timeout whatsoever and a stalled request could hang for + /// the lifetime of the tab. That was not theoretical: a gateway read that + /// never returned would hold the SDK's per-bucket lock indefinitely, and + /// (before the bridge handle became a bare `Arc`) the whole client with + /// it. Race the send against a timer instead. + #[cfg(not(target_arch = "wasm32"))] + async fn send_bounded(&self, req: reqwest::RequestBuilder) -> Result { + req.send().await.map_err(ClientError::Http) + } + + #[cfg(target_arch = "wasm32")] + async fn send_bounded(&self, req: reqwest::RequestBuilder) -> Result { + use futures::future::{select, Either}; + use gloo_timers::future::TimeoutFuture; + + // TimeoutFuture takes u32 millis; saturate rather than wrap so an + // absurdly large configured timeout can't collapse to a tiny one. + let ms = u32::try_from(self.config.timeout.as_millis()).unwrap_or(u32::MAX); + let send = std::pin::pin!(req.send()); + let timer = TimeoutFuture::new(ms); + match select(send, timer).await { + Either::Left((res, _)) => res.map_err(ClientError::Http), + // Dropping the abandoned request here aborts the underlying fetch - + // unlike a Dart-side .timeout(), this actually releases the work + // rather than orphaning it while it keeps holding locks. + Either::Right((_, _)) => Err(ClientError::Timeout(self.config.timeout)), + } + } + async fn request( &self, method: &str, @@ -1543,7 +1578,7 @@ impl FulaClient { } debug!("Sending {} request to {}", method, url); - let response = match req.send().await { + let response = match self.send_bounded(req).await { Ok(r) => r, Err(e) => { // Connection-level error (refused, RST, DNS, timeout). For a @@ -1555,7 +1590,9 @@ impl FulaClient { gate.record_failure(); } } - return Err(ClientError::Http(e)); + // Already a ClientError (Http on native / Http-or-Timeout in + // the browser) - pass it through. + return Err(e); } }; diff --git a/crates/fula-client/src/error.rs b/crates/fula-client/src/error.rs index 2428e1b..3c85cc5 100644 --- a/crates/fula-client/src/error.rs +++ b/crates/fula-client/src/error.rs @@ -12,6 +12,19 @@ pub enum ClientError { #[error("HTTP error: {0}")] Http(#[from] reqwest::Error), + /// Request exceeded `Config::timeout`. + /// + /// Produced on **wasm32 only**. `reqwest::ClientBuilder::timeout` is a + /// no-op in the browser, so `FulaClient::new` cannot arm it there and + /// `Config::timeout` was silently dropped - a stalled request had NO + /// bound at all and could hang for the lifetime of the tab. The wasm build + /// now races the send against a timer instead (see + /// `FulaClient::send_bounded`) and surfaces that as this variant. + /// + /// On native, reqwest enforces the same budget itself and reports it as + /// `ClientError::Http`, so this variant never appears there. + #[error("request timed out after {0:?}")] + Timeout(std::time::Duration), /// S3 API error #[error("S3 error ({code}): {message}")] S3Error { diff --git a/crates/fula-flutter/src/api/chunked.rs b/crates/fula-flutter/src/api/chunked.rs index 12c1c95..6c52c77 100644 --- a/crates/fula-flutter/src/api/chunked.rs +++ b/crates/fula-flutter/src/api/chunked.rs @@ -1,4 +1,4 @@ -//! Chunked/streaming operations +//! Chunked/streaming operations //! //! Functions for handling large files using chunked encryption. //! Files above a certain threshold are automatically split into chunks. @@ -26,7 +26,7 @@ pub async fn put_chunked( data: Vec, chunk_size: Option, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let chunk_size = chunk_size.map(|s| s as usize); let result = guard.put_object_chunked(&bucket, &key, &data, chunk_size).await?; Ok(result.into()) @@ -40,7 +40,7 @@ pub async fn get_chunked( bucket: String, key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_chunked(&bucket, &key).await?; Ok(data.to_vec()) } @@ -56,7 +56,7 @@ pub async fn get_range( offset: u64, length: u64, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_range(&bucket, &key, offset, length).await?; Ok(data.to_vec()) } diff --git a/crates/fula-flutter/src/api/client.rs b/crates/fula-flutter/src/api/client.rs index c0f679c..6269e41 100644 --- a/crates/fula-flutter/src/api/client.rs +++ b/crates/fula-flutter/src/api/client.rs @@ -1,4 +1,4 @@ -//! Core FulaClient wrapper operations +//! Core FulaClient wrapper operations //! //! These functions wrap the underlying FulaClient for plain (unencrypted) operations. @@ -7,11 +7,7 @@ use std::time::Duration; use bytes::Bytes; use anyhow::Context; -// Use tokio::sync on native, async_lock on WASM -#[cfg(not(target_arch = "wasm32"))] -use tokio::sync::RwLock; -#[cfg(target_arch = "wasm32")] -use async_lock::RwLock; +// (No RwLock import: EncryptedClientHandle is a bare `Arc`.) use crate::api::types::*; @@ -199,7 +195,7 @@ pub async fn create_encrypted_client( let client = fula_client::EncryptedClient::new(inner_config, enc_config)?; Ok(EncryptedClientHandle { - inner: Arc::new(RwLock::new(client)), + inner: Arc::new(client), health_dispatcher: dispatcher, }) } @@ -262,7 +258,7 @@ pub async fn create_encrypted_client_with_pinning( )?; Ok(EncryptedClientHandle { - inner: Arc::new(RwLock::new(client)), + inner: Arc::new(client), health_dispatcher: dispatcher, }) } diff --git a/crates/fula-flutter/src/api/encrypted.rs b/crates/fula-flutter/src/api/encrypted.rs index b367e80..678ec5b 100644 --- a/crates/fula-flutter/src/api/encrypted.rs +++ b/crates/fula-flutter/src/api/encrypted.rs @@ -1,4 +1,4 @@ -//! Encrypted client operations +//! Encrypted client operations //! //! These functions wrap EncryptedClient for client-side encrypted storage. @@ -17,7 +17,7 @@ pub async fn put_encrypted( key: String, data: Vec, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.put_object_encrypted(&bucket, &key, Bytes::from(data)).await?; Ok(result.into()) } @@ -30,7 +30,7 @@ pub async fn put_encrypted_with_type( data: Vec, content_type: String, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.put_object_encrypted_with_type( &bucket, &key, @@ -46,7 +46,7 @@ pub async fn get_decrypted( bucket: String, key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_decrypted(&bucket, &key).await?; Ok(data.to_vec()) } @@ -57,7 +57,7 @@ pub async fn get_decrypted_by_storage_key( bucket: String, storage_key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_decrypted_by_storage_key(&bucket, &storage_key).await?; Ok(data.to_vec()) } @@ -76,7 +76,7 @@ pub async fn get_decrypted_buffered( bucket: String, key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let mut buf: Vec = Vec::new(); guard .get_object_decrypted_buffered_to_writer(&bucket, &key, &mut buf) @@ -92,7 +92,7 @@ pub async fn get_decrypted_buffered_by_storage_key( bucket: String, storage_key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let mut buf: Vec = Vec::new(); guard .get_object_decrypted_buffered_to_writer_by_storage_key(&bucket, &storage_key, &mut buf) @@ -106,7 +106,7 @@ pub async fn get_with_private_metadata( bucket: String, storage_key: String, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.get_object_with_private_metadata(&bucket, &storage_key).await?; Ok(result.into()) } @@ -117,7 +117,7 @@ pub async fn delete_encrypted( bucket: String, key: String, ) -> anyhow::Result<()> { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.delete_object(&bucket, &key).await?; Ok(()) } @@ -128,7 +128,7 @@ pub async fn delete_by_storage_key( bucket: String, storage_key: String, ) -> anyhow::Result<()> { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.delete_object_by_storage_key(&bucket, &storage_key).await?; Ok(()) } @@ -143,7 +143,7 @@ pub async fn head_decrypted( bucket: String, storage_key: String, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.head_object_decrypted(&bucket, &storage_key).await?; Ok(result.into()) } @@ -154,7 +154,7 @@ pub async fn list_decrypted( bucket: String, options: ListOptions, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.list_objects_decrypted(&bucket, Some(options.into())).await?; Ok(result.into_iter().map(|m| m.into()).collect()) } @@ -165,7 +165,7 @@ pub async fn list_directory( bucket: String, prefix: Option, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.list_directory(&bucket, prefix.as_deref()).await?; // Convert the internal DirectoryListing to our type @@ -191,13 +191,13 @@ pub async fn list_directory( /// Export the secret key for backup pub async fn export_secret_key(client: &EncryptedClientHandle) -> Vec { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.encryption_config().export_secret_key().as_bytes().to_vec() } /// Get the public key for sharing pub async fn get_public_key(client: &EncryptedClientHandle) -> Vec { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.encryption_config().public_key().as_bytes().to_vec() } @@ -394,7 +394,7 @@ pub async fn derive_signing_seed(seed: String) -> Vec { /// Check if client uses FlatNamespace mode pub async fn is_flat_namespace(client: &EncryptedClientHandle) -> bool { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.is_flat_namespace() } @@ -404,21 +404,21 @@ pub async fn is_flat_namespace(client: &EncryptedClientHandle) -> bool { /// List buckets (delegated to inner client) pub async fn enc_list_buckets(client: &EncryptedClientHandle) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.list_buckets().await?; Ok(result.buckets.into_iter().map(|b| b.into()).collect()) } /// Create bucket (delegated to inner client) pub async fn enc_create_bucket(client: &EncryptedClientHandle, name: String) -> anyhow::Result<()> { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.create_bucket(&name).await?; Ok(()) } /// Delete bucket (delegated to inner client) pub async fn enc_delete_bucket(client: &EncryptedClientHandle, name: String) -> anyhow::Result<()> { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.delete_bucket(&name).await?; Ok(()) } diff --git a/crates/fula-flutter/src/api/error.rs b/crates/fula-flutter/src/api/error.rs index 389ea7a..dd5186f 100644 --- a/crates/fula-flutter/src/api/error.rs +++ b/crates/fula-flutter/src/api/error.rs @@ -1,4 +1,4 @@ -//! Error types for Flutter bindings +//! Error types for Flutter bindings //! //! Provides a unified error type that maps from various internal error types //! and is suitable for FFI/flutter_rust_bridge serialization. @@ -147,6 +147,14 @@ impl From for FulaError { use fula_client::ClientError; match err { ClientError::Http(e) => FulaError::Network(e.to_string()), + // wasm-only: the browser has no reqwest builder timeout, so + // Config::timeout is enforced by racing a timer instead. Map it to + // Network alongside reqwest's own native timeout (which arrives as + // Http(_) and is already Network) so both targets present a + // transport failure identically to Dart. + ClientError::Timeout(d) => { + FulaError::Network(format!("request timed out after {:?}", d)) + } ClientError::S3Error { code, message, .. } => { match code.as_str() { "NoSuchKey" => FulaError::NotFound { diff --git a/crates/fula-flutter/src/api/forest.rs b/crates/fula-flutter/src/api/forest.rs index a465e69..510b968 100644 --- a/crates/fula-flutter/src/api/forest.rs +++ b/crates/fula-flutter/src/api/forest.rs @@ -1,4 +1,4 @@ -//! Forest/FlatNamespace operations +//! Forest/FlatNamespace operations //! //! These functions manage the encrypted file index (PrivateForest) //! for organized file storage with human-readable paths. @@ -26,7 +26,7 @@ pub async fn load_forest( client: &EncryptedClientHandle, bucket: String, ) -> anyhow::Result<()> { - let guard = client.inner.write().await; + let guard = &*client.inner; match guard.load_forest(&bucket).await { Ok(_) => Ok(()), Err(e) if e.to_string().contains("forest is sharded") => Ok(()), @@ -42,7 +42,7 @@ pub async fn save_forest( client: &EncryptedClientHandle, bucket: String, ) -> anyhow::Result<()> { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.flush_forest(&bucket).await?; Ok(()) } @@ -52,14 +52,14 @@ pub async fn flush_forest( client: &EncryptedClientHandle, bucket: String, ) -> anyhow::Result<()> { - let guard = client.inner.write().await; + let guard = &*client.inner; guard.flush_forest(&bucket).await?; Ok(()) } /// Check if there are pending (unsaved) forest changes pub async fn has_pending_changes(client: &EncryptedClientHandle, bucket: String) -> bool { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.has_pending_forest_changes(&bucket).await } @@ -79,7 +79,7 @@ pub async fn has_pending_changes(client: &EncryptedClientHandle, bucket: String) /// Typical app wiring: call on pull-to-refresh, tab-resume, reconnect, /// or any cache-revalidation path, then re-run `list_from_forest`. pub async fn invalidate_forest_cache(client: &EncryptedClientHandle, bucket: String) { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.invalidate_forest_cache(&bucket); } @@ -89,7 +89,7 @@ pub async fn invalidate_forest_cache(client: &EncryptedClientHandle, bucket: Str /// dropped; forests with pending (unsaved) changes are kept, matching /// the per-bucket dirty-safe contract. pub async fn invalidate_all_forest_caches(client: &EncryptedClientHandle) { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.invalidate_all_forest_caches(); } @@ -108,7 +108,7 @@ pub async fn put_flat( data: Vec, content_type: Option, ) -> anyhow::Result { - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard.put_object_flat( &bucket, &path, @@ -129,7 +129,7 @@ pub async fn put_flat_deferred( data: Vec, content_type: Option, ) -> anyhow::Result { - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard.put_object_flat_deferred( &bucket, &path, @@ -145,7 +145,7 @@ pub async fn get_flat( bucket: String, path: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_flat(&bucket, &path).await?; Ok(data.to_vec()) } @@ -156,7 +156,7 @@ pub async fn delete_flat( bucket: String, path: String, ) -> anyhow::Result<()> { - let guard = client.inner.write().await; + let guard = &*client.inner; guard.delete_object_flat(&bucket, &path).await?; Ok(()) } @@ -169,7 +169,7 @@ pub async fn list_from_forest( client: &EncryptedClientHandle, bucket: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let result = guard.list_files_from_forest(&bucket).await?; Ok(result.into_iter().map(|m| m.into()).collect()) } @@ -194,7 +194,7 @@ pub async fn put_flat_from_path( ) -> anyhow::Result { let data = tokio::fs::read(&file_path).await .with_context(|| format!("Failed to read file: {}", file_path))?; - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard.put_object_flat( &bucket, &path, @@ -228,7 +228,7 @@ pub async fn put_flat_from_path_deferred( ) -> anyhow::Result { let data = tokio::fs::read(&file_path).await .with_context(|| format!("Failed to read file: {}", file_path))?; - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard.put_object_flat_deferred( &bucket, &path, @@ -293,7 +293,7 @@ pub async fn put_flat_resumable_from_path( // `EncryptedClient::put_object_encrypted_resumable` via the // `bucket_write_mutex` extension (issue #17). Different buckets // parallelize through this `read().await`. - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard.put_object_encrypted_resumable( &bucket, @@ -341,7 +341,7 @@ pub async fn resume_flat_upload_from_path( // `EncryptedClient::resume_upload`, which loads the manifest first // (bucket name lives there) and acquires the bucket_write_mutex // post-load (issue #17). - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard.resume_upload(&manifest, &data).await?; Ok(result.into()) @@ -417,7 +417,7 @@ pub async fn put_flat_resumable_from_path_cancellable( ) -> anyhow::Result { let data = tokio::fs::read(&file_path).await .with_context(|| format!("Failed to read file: {}", file_path))?; - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard.put_object_encrypted_resumable_with_cancel( &bucket, @@ -455,7 +455,7 @@ pub async fn resume_flat_upload_from_path_cancellable( ) -> anyhow::Result { let data = tokio::fs::read(&file_path).await .with_context(|| format!("Failed to read file: {}", file_path))?; - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard.resume_upload_with_cancel( &manifest, @@ -491,7 +491,7 @@ pub async fn resume_flat_upload_from_path_cancellable( /// abort, see [`cancel_handle_trigger`] on a [`CancelHandle`] passed to /// the `_cancellable` variants (issue #18). /// -/// **Lock scope.** `client.inner.read().await` — same as the resumable +/// **Lock scope.** None at this layer any more (bare Arc) — same as the resumable /// bridge functions. The underlying `abort_upload` doesn't touch the /// encrypted forest (only the raw storage backend for chunk deletes /// plus the local manifest file), so B1's per-bucket write mutex is @@ -512,7 +512,7 @@ pub async fn abort_resumable_upload( if !manifest.exists() { return Ok(()); } - let guard = client.inner.read().await; + let guard = &*client.inner; guard .abort_upload(&manifest) .await @@ -624,7 +624,7 @@ pub async fn put_flat_with_progress( progress: &ProgressHandle, ) -> anyhow::Result { let cb = progress_cb(progress); - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard .put_object_flat_with_progress(&bucket, &path, Bytes::from(data), content_type.as_deref(), cb) .await?; @@ -648,7 +648,7 @@ pub async fn put_flat_with_progress_cancellable( cancel: &CancelHandle, ) -> anyhow::Result { let cb = progress_cb(progress); - let guard = client.inner.write().await; + let guard = &*client.inner; let result = guard .put_object_flat_with_progress_cancellable( &bucket, @@ -723,7 +723,7 @@ pub async fn streaming_upload_begin( content_type: Option, ) -> anyhow::Result { let (storage_key, dek, wrapped_dek, kek_version) = { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.streaming_begin(&bucket, &key).await? }; let aad_prefix = format!("fula:v4:chunk:{}", storage_key); @@ -796,7 +796,7 @@ pub async fn streaming_upload_finalize_plan( // streaming_finalize_plan is synchronous; the client read guard is only for // method access. No handle lock held here. let (chunked_metadata, private_meta, encrypted_meta) = { - let guard = client.inner.read().await; + let guard = &*client.inner; guard.streaming_finalize_plan( encoder, &dek, @@ -847,7 +847,7 @@ pub async fn streaming_upload_chunk( } }; let (_chunk_key, cid) = { - let guard = client.inner.read().await; + let guard = &*client.inner; guard .streaming_put_chunk( &bucket, @@ -931,7 +931,7 @@ pub async fn streaming_upload_finish( let chunked_metadata = std::sync::Arc::try_unwrap(chunked_metadata) .unwrap_or_else(|arc| (*arc).clone()); let result = { - let guard = client.inner.read().await; + let guard = &*client.inner; guard .streaming_finish( &bucket, @@ -965,7 +965,7 @@ pub async fn put_flat_resumable_from_path_with_progress( .await .with_context(|| format!("Failed to read file: {}", file_path))?; let cb = progress_cb(progress); - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard .put_object_encrypted_resumable_with_cancel_and_progress( @@ -1013,7 +1013,7 @@ pub async fn resume_flat_upload_from_path_with_progress( .await .with_context(|| format!("Failed to read file: {}", file_path))?; let cb = progress_cb(progress); - let guard = client.inner.read().await; + let guard = &*client.inner; let manifest = std::path::PathBuf::from(manifest_path); let result = guard .resume_upload_with_cancel_and_progress( @@ -1050,7 +1050,7 @@ pub async fn get_forest_subtree( bucket: String, prefix: String, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let subtree = guard.get_forest_subtree(&bucket, &prefix).await?; // Serialize the subtree diff --git a/crates/fula-flutter/src/api/rotation.rs b/crates/fula-flutter/src/api/rotation.rs index 519a780..6d329fe 100644 --- a/crates/fula-flutter/src/api/rotation.rs +++ b/crates/fula-flutter/src/api/rotation.rs @@ -1,4 +1,4 @@ -//! Key rotation operations +//! Key rotation operations //! //! Functions for rotating encryption keys to maintain security. //! Key rotation re-wraps data encryption keys with a new key encryption key. @@ -16,7 +16,7 @@ use crate::api::types::*; /// The rotation manager handles the key rotation process, /// including tracking which keys have been rotated. pub async fn create_rotation_manager(client: &EncryptedClientHandle) -> RotationManagerHandle { - let guard = client.inner.read().await; + let guard = &*client.inner; let manager = guard.create_rotation_manager(); RotationManagerHandle { inner: Arc::new(manager), @@ -35,7 +35,7 @@ pub async fn get_kek_version( bucket: String, storage_key: String, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let version = guard.get_object_kek_version(&bucket, &storage_key).await?; Ok(version) } @@ -49,7 +49,7 @@ pub async fn rewrap_object( storage_key: String, manager: &RotationManagerHandle, ) -> anyhow::Result { - let guard = client.inner.write().await; + let guard = &*client.inner; let version = guard.rewrap_object_dek(&bucket, &storage_key, &manager.inner).await?; Ok(version) } @@ -63,7 +63,7 @@ pub async fn rotate_bucket( bucket: String, manager: &RotationManagerHandle, ) -> anyhow::Result { - let guard = client.inner.read().await; + let guard = &*client.inner; let report = guard.rotate_bucket(&bucket, &manager.inner).await?; // Manual conversion since fula_client::encryption::RotationReport is not exported diff --git a/crates/fula-flutter/src/api/sharing.rs b/crates/fula-flutter/src/api/sharing.rs index 1b279b3..7d81f96 100644 --- a/crates/fula-flutter/src/api/sharing.rs +++ b/crates/fula-flutter/src/api/sharing.rs @@ -1,4 +1,4 @@ -//! Sharing operations +//! Sharing operations //! //! Functions for creating and accepting share tokens to share //! encrypted files with other users. @@ -37,7 +37,7 @@ pub async fn create_share_token( anyhow::bail!("Recipient public key must be exactly 32 bytes"); } - let guard = client.inner.read().await; + let guard = &*client.inner; let enc_config = guard.encryption_config(); // Get owner's keypair @@ -135,7 +135,7 @@ pub async fn create_share_token_with_mode( anyhow::bail!("Recipient public key must be exactly 32 bytes"); } - let guard = client.inner.read().await; + let guard = &*client.inner; let enc_config = guard.encryption_config(); // Get owner's keypair @@ -280,7 +280,7 @@ pub async fn accept_share(client: &EncryptedClientHandle, token_json: String) -> let token: fula_crypto::ShareToken = serde_json::from_str(&token_json) .map_err(|e| anyhow::anyhow!("Invalid token format: {}", e))?; - let guard = client.inner.read().await; + let guard = &*client.inner; let accepted = guard.accept_share(&token) .map_err(|e| anyhow::anyhow!(e.to_string()))?; @@ -295,7 +295,7 @@ pub async fn get_with_share( original_key: String, share: &AcceptedShareHandle, ) -> anyhow::Result> { - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_with_share(&bucket, &storage_key, &original_key, &share.inner).await?; Ok(data.to_vec()) } @@ -311,7 +311,7 @@ pub async fn get_with_token( let token: fula_crypto::ShareToken = serde_json::from_str(&token_json) .map_err(|e| anyhow::anyhow!("Invalid token format: {}", e))?; - let guard = client.inner.read().await; + let guard = &*client.inner; let data = guard.get_object_with_token(&bucket, &storage_key, &original_key, &token).await?; Ok(data.to_vec()) } diff --git a/crates/fula-flutter/src/api/types.rs b/crates/fula-flutter/src/api/types.rs index 56426fe..45fd94f 100644 --- a/crates/fula-flutter/src/api/types.rs +++ b/crates/fula-flutter/src/api/types.rs @@ -5,14 +5,12 @@ use std::sync::Arc; -// Use tokio::sync on native, async_lock on WASM -#[cfg(not(target_arch = "wasm32"))] -use tokio::sync::RwLock; +// Use tokio::sync on native, async_lock on WASM. +// (RwLock used to be imported for EncryptedClientHandle; that handle is now a +// bare Arc - see its docs. Semaphore is still used by MultipartHandle.) #[cfg(not(target_arch = "wasm32"))] pub(crate) use tokio::sync::Semaphore; -#[cfg(target_arch = "wasm32")] -use async_lock::RwLock; #[cfg(target_arch = "wasm32")] pub(crate) use async_lock::Semaphore; @@ -672,10 +670,43 @@ pub struct FulaClientHandle { /// Handle to an EncryptedClient instance /// -/// This wraps the encrypted client with interior mutability for forest state. +/// Bare `Arc`, no lock — matching [`FulaClientHandle`] above. +/// +/// This was `Arc>`, and that outer lock was the single +/// worst bottleneck in the web client. `load_forest` took the EXCLUSIVE guard +/// and held it across the whole network fetch, so one slow bucket froze every +/// other call in the app. Captured live on a phone: +/// +/// ```text +/// loadForest: ENTER tag-metadata <- ~30s, holds the write guard +/// getFlat: ENTER website-metadata-v8/.. <- never completes +/// loadForest: ENTER tag-metadata-v8 <- never completes (healthy +/// bucket, normally 0.1s) +/// ``` +/// +/// A Dart-side `.timeout()` cannot rescue that: the FRB binding exposes no +/// cancel handle for `load_forest`, so the Rust future keeps running and keeps +/// the guard after the caller has given up. +/// +/// Removing it is safe, not merely expedient: +/// +/// * `EncryptedClient` has **no** `&mut self` methods — all ~180 public +/// methods take `&self`. The lock was never protecting mutability. +/// * The SDK already owns finer-grained locking: `forest_cache` (a DashMap), +/// per-bucket `migration_locks`, and `bucket_write_mutex`, documented there +/// as "the OUTERMOST per-bucket lock … per-bucket, so different buckets +/// continue to flush in parallel". A per-bucket lock at THIS layer would +/// only duplicate that one level up — and would not even cover the calls +/// that carry no bucket (`get_public_key`, `export_secret_key`, …). +/// * The read/write split here encoded no invariant: `save_forest` (read) and +/// `flush_forest` (write) had byte-identical bodies, and `rewrap_object` +/// took `write()` for a single object while `rotate_bucket` took `read()` +/// for an entire bucket. +/// * No bridge function composed two SDK calls under one guard, so nothing +/// depended on atomicity it appeared to provide. #[derive(Clone)] pub struct EncryptedClientHandle { - pub(crate) inner: Arc>, + pub(crate) inner: Arc, /// Phase 19 — same dispatcher pattern as FulaClientHandle. /// Encrypted-client construction also threads the callback into /// the underlying `fula_client::Config` so warm-cache + cold- From 5060ab24ba39485712d848abd3dada1757dfe030 Mon Sep 17 00:00:00 2001 From: ehsan shariati Date: Sat, 22 Aug 2026 19:37:15 -0400 Subject: [PATCH 3/5] wasm timeout: use reqwest's own per-request timeout, not a hand-rolled race Simplifies the wasm timeout added in the previous commit. I had claimed that dropping the abandoned send() future aborts the browser fetch, and went to verify it rather than leave it asserted. It is true -- AbortGuard::drop calls ctrl.abort() (reqwest-0.12.24/src/wasm/mod.rs:78-84) -- but the same file showed the racing was unnecessary. reqwest's wasm backend honours a per-REQUEST timeout. Only the CLIENT-level ClientBuilder::timeout is a no-op there. See reqwest-0.12.24/src/wasm: request.rs:264 pub fn timeout(mut self, timeout: Duration) -> RequestBuilder mod.rs:64 AbortGuard::timeout -> set_timeout(.. ctrl.abort_with_reason ..) client.rs:229 let mut abort = AbortGuard::new()?; client.rs:230 if let Some(timeout) = req.timeout() { abort.timeout(*timeout); } client.rs:233 init.signal(Some(&abort.signal())); So the hand-rolled futures::select against a gloo-timers TimeoutFuture reimplemented -- less well -- what reqwest already does with an AbortController. Replaced by one cfg-gated line before send(). Removed as a result: the send_bounded helper, the new ClientError::Timeout variant, and its FulaError arm. The public ClientError enum is therefore UNCHANGED, so no downstream exhaustive match breaks, and a browser timeout now surfaces as an ordinary reqwest::Error with is_timeout() -- identical in shape to native instead of a wasm-only special case. Verified: cargo check clean on native and wasm32-unknown-unknown. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ANUDVTMmqZGFDjbj7k9HuP --- crates/fula-client/src/client.rs | 56 +++++++++------------------- crates/fula-client/src/error.rs | 13 ------- crates/fula-flutter/src/api/error.rs | 8 ---- 3 files changed, 17 insertions(+), 60 deletions(-) diff --git a/crates/fula-client/src/client.rs b/crates/fula-client/src/client.rs index b9cc01d..169f78c 100644 --- a/crates/fula-client/src/client.rs +++ b/crates/fula-client/src/client.rs @@ -1476,41 +1476,6 @@ impl FulaClient { // ==================== Helper Methods ==================== - /// Send a prepared request, enforcing `Config::timeout` on every target. - /// - /// On native this is a plain `send()` - `FulaClient::new` already armed - /// `ClientBuilder::timeout`. - /// - /// On **wasm32 that builder call is a no-op**, so the browser client ran - /// with NO request timeout whatsoever and a stalled request could hang for - /// the lifetime of the tab. That was not theoretical: a gateway read that - /// never returned would hold the SDK's per-bucket lock indefinitely, and - /// (before the bridge handle became a bare `Arc`) the whole client with - /// it. Race the send against a timer instead. - #[cfg(not(target_arch = "wasm32"))] - async fn send_bounded(&self, req: reqwest::RequestBuilder) -> Result { - req.send().await.map_err(ClientError::Http) - } - - #[cfg(target_arch = "wasm32")] - async fn send_bounded(&self, req: reqwest::RequestBuilder) -> Result { - use futures::future::{select, Either}; - use gloo_timers::future::TimeoutFuture; - - // TimeoutFuture takes u32 millis; saturate rather than wrap so an - // absurdly large configured timeout can't collapse to a tiny one. - let ms = u32::try_from(self.config.timeout.as_millis()).unwrap_or(u32::MAX); - let send = std::pin::pin!(req.send()); - let timer = TimeoutFuture::new(ms); - match select(send, timer).await { - Either::Left((res, _)) => res.map_err(ClientError::Http), - // Dropping the abandoned request here aborts the underlying fetch - - // unlike a Dart-side .timeout(), this actually releases the work - // rather than orphaning it while it keeps holding locks. - Either::Right((_, _)) => Err(ClientError::Timeout(self.config.timeout)), - } - } - async fn request( &self, method: &str, @@ -1577,8 +1542,23 @@ impl FulaClient { req = req.body(data); } + // wasm32 only: `ClientBuilder::timeout` is a NO-OP in the browser (see + // `FulaClient::new`), so `Config::timeout` was silently dropped and a + // stalled request had no bound at all — it could hang for the lifetime + // of the tab while holding SDK locks. reqwest's wasm backend DOES + // honour a per-REQUEST timeout: it arms an `AbortController` from + // `req.timeout()` and wires the signal into fetch + // (reqwest-0.12/src/wasm/client.rs: `AbortGuard::new()` -> + // `abort.timeout(*timeout)` -> `init.signal(..)`), and `AbortGuard::drop` + // aborts in-flight. So set it per-request here rather than racing a + // timer ourselves — same abort mechanism, and the failure surfaces as + // an ordinary `reqwest::Error` with `is_timeout()`, identical to native. + // On native the builder-level timeout already covers this. + #[cfg(target_arch = "wasm32")] + let req = req.timeout(self.config.timeout); + debug!("Sending {} request to {}", method, url); - let response = match self.send_bounded(req).await { + let response = match req.send().await { Ok(r) => r, Err(e) => { // Connection-level error (refused, RST, DNS, timeout). For a @@ -1590,9 +1570,7 @@ impl FulaClient { gate.record_failure(); } } - // Already a ClientError (Http on native / Http-or-Timeout in - // the browser) - pass it through. - return Err(e); + return Err(ClientError::Http(e)); } }; diff --git a/crates/fula-client/src/error.rs b/crates/fula-client/src/error.rs index 3c85cc5..2428e1b 100644 --- a/crates/fula-client/src/error.rs +++ b/crates/fula-client/src/error.rs @@ -12,19 +12,6 @@ pub enum ClientError { #[error("HTTP error: {0}")] Http(#[from] reqwest::Error), - /// Request exceeded `Config::timeout`. - /// - /// Produced on **wasm32 only**. `reqwest::ClientBuilder::timeout` is a - /// no-op in the browser, so `FulaClient::new` cannot arm it there and - /// `Config::timeout` was silently dropped - a stalled request had NO - /// bound at all and could hang for the lifetime of the tab. The wasm build - /// now races the send against a timer instead (see - /// `FulaClient::send_bounded`) and surfaces that as this variant. - /// - /// On native, reqwest enforces the same budget itself and reports it as - /// `ClientError::Http`, so this variant never appears there. - #[error("request timed out after {0:?}")] - Timeout(std::time::Duration), /// S3 API error #[error("S3 error ({code}): {message}")] S3Error { diff --git a/crates/fula-flutter/src/api/error.rs b/crates/fula-flutter/src/api/error.rs index dd5186f..15c64f1 100644 --- a/crates/fula-flutter/src/api/error.rs +++ b/crates/fula-flutter/src/api/error.rs @@ -147,14 +147,6 @@ impl From for FulaError { use fula_client::ClientError; match err { ClientError::Http(e) => FulaError::Network(e.to_string()), - // wasm-only: the browser has no reqwest builder timeout, so - // Config::timeout is enforced by racing a timer instead. Map it to - // Network alongside reqwest's own native timeout (which arrives as - // Http(_) and is already Network) so both targets present a - // transport failure identically to Dart. - ClientError::Timeout(d) => { - FulaError::Network(format!("request timed out after {:?}", d)) - } ClientError::S3Error { code, message, .. } => { match code.as_str() { "NoSuchKey" => FulaError::NotFound { From 74d060c17a37025c7210ff14a112d3c63153a5d8 Mon Sep 17 00:00:00 2001 From: ehsan shariati Date: Sun, 23 Aug 2026 00:17:07 -0400 Subject: [PATCH 4/5] docs: record the rotation TOCTOU exposed by dropping the bridge lock Removing the coarse bridge RwLock removed accidental serialization between key rotation and uploads. rewrap_object_dek is a GET-modify-PUT plus a forest mutation and takes no per-bucket lock, so an upload landing between its GET and PUT is overwritten by the re-encrypted old data. Documented rather than papered over: taking bucket_write_mutex here would serialize rotate_bucket_inner's buffer_unordered concurrency, and a per-object lock would not exclude uploads (which hold only the per-bucket mutex). Correct fix is If-Match/ETag optimistic concurrency inside fula-client::rewrap_object_dek. Acceptable to ship now: rotation has no call site in FxFiles, and the window needs a rotation and an upload of the SAME object to overlap. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ANUDVTMmqZGFDjbj7k9HuP --- crates/fula-flutter/src/api/rotation.rs | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/fula-flutter/src/api/rotation.rs b/crates/fula-flutter/src/api/rotation.rs index 6d329fe..59f07be 100644 --- a/crates/fula-flutter/src/api/rotation.rs +++ b/crates/fula-flutter/src/api/rotation.rs @@ -43,6 +43,36 @@ pub async fn get_kek_version( /// Re-wrap a single object's DEK with the current KEK /// /// Returns the new KEK version. +/// +/// # KNOWN RACE (introduced 2026-08-23, tracked — read before extending) +/// +/// `rewrap_object_dek` is a GET-modify-PUT of the object's encryption +/// metadata plus a forest mutation, and it takes **no** per-bucket lock — +/// unlike `put_object_flat` / `flush_forest` / `delete_object_flat`, which +/// all take `bucket_write_mutex`. Until this handle became a bare `Arc`, +/// the bridge's coarse `RwLock` serialised rotation against uploads by +/// accident. It no longer does. +/// +/// The exposure is TOCTOU: if an upload modifies the object between +/// rotation's GET and its PUT, the PUT overwrites the newly-uploaded data +/// with the re-encrypted OLD data. +/// +/// Deliberately NOT "fixed" by taking `bucket_write_mutex` here: +/// `rotate_bucket_inner` drives this through `buffer_unordered( +/// MAX_CONCURRENT_REWRAPS)`, so a per-bucket lock would serialise a +/// deliberately-parallel operation — a fix that looks right and quietly +/// destroys rotation throughput. A per-OBJECT lock would not help either +/// unless the upload paths took the same one (they hold only the +/// per-bucket mutex). +/// +/// The correct fix is optimistic concurrency: send the GET's ETag as +/// `If-Match` on the rewrap PUT so a concurrent upload makes the PUT fail +/// and the object is simply re-rotated. That belongs in +/// `fula-client::rewrap_object_dek`, not here. +/// +/// Why it is acceptable to ship meanwhile: key rotation has no call site +/// in FxFiles (the Dart wrappers exist but nothing invokes them), and the +/// window requires a rotation and an upload of the SAME object to overlap. pub async fn rewrap_object( client: &EncryptedClientHandle, bucket: String, From b4af39c3274f030cf053a4cf0e0e836a94309514 Mon Sep 17 00:00:00 2001 From: ehsan shariati Date: Sun, 23 Aug 2026 00:19:33 -0400 Subject: [PATCH 5/5] release: 0.6.20 Bridge lock removal + wasm per-request timeout + gateway Timeout recovery. All 9 CI checks green on PR #89, including cargo test --workspace and wasm-pack headless-Firefox runtime tests. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01ANUDVTMmqZGFDjbj7k9HuP --- Cargo.lock | 18 ++++++------ Cargo.toml | 2 +- crates/fula-flutter/Cargo.toml | 2 +- crates/fula-js/Cargo.toml | 2 +- packages/fula_client/CHANGELOG.md | 31 ++++++++++++++++++++ packages/fula_client/ios/fula_client.podspec | 2 +- packages/fula_client/pubspec.yaml | 2 +- 7 files changed, 45 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0e8512..55dc3ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1776,7 +1776,7 @@ dependencies = [ [[package]] name = "fula-api" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "axum", @@ -1805,7 +1805,7 @@ dependencies = [ [[package]] name = "fula-blockstore" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "async-trait", @@ -1846,7 +1846,7 @@ dependencies = [ [[package]] name = "fula-cli" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "async-trait", @@ -1900,7 +1900,7 @@ dependencies = [ [[package]] name = "fula-client" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "async-trait", @@ -1943,7 +1943,7 @@ dependencies = [ [[package]] name = "fula-core" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "async-trait", @@ -1978,7 +1978,7 @@ dependencies = [ [[package]] name = "fula-crypto" -version = "0.6.19" +version = "0.6.20" dependencies = [ "aes-gcm", "anyhow", @@ -2023,7 +2023,7 @@ dependencies = [ [[package]] name = "fula-flutter" -version = "0.6.19" +version = "0.6.20" dependencies = [ "anyhow", "async-lock", @@ -2047,7 +2047,7 @@ dependencies = [ [[package]] name = "fula-js" -version = "0.6.19" +version = "0.6.20" dependencies = [ "base64 0.22.1", "bytes", @@ -2068,7 +2068,7 @@ dependencies = [ [[package]] name = "fula-mcp" -version = "0.6.19" +version = "0.6.20" dependencies = [ "aes-gcm", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index a3c8b55..7257553 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -80,7 +80,7 @@ name = "encrypted_upload_test" path = "examples/encrypted_upload_test.rs" [workspace.package] -version = "0.6.19" +version = "0.6.20" edition = "2021" license = "MIT OR Apache-2.0" repository = "https://github.com/functionland/fula-api" diff --git a/crates/fula-flutter/Cargo.toml b/crates/fula-flutter/Cargo.toml index c498e0e..a815265 100644 --- a/crates/fula-flutter/Cargo.toml +++ b/crates/fula-flutter/Cargo.toml @@ -5,7 +5,7 @@ description = "Flutter bindings for Fula decentralized storage - works on Androi # to parse `*.workspace = true` keys in its own manifest scan. Keep # these in sync with `[workspace.package]` in the root Cargo.toml. # (Same workaround as crates/fula-js.) -version = "0.6.19" +version = "0.6.20" edition = "2021" license = "MIT OR Apache-2.0" repository = "https://github.com/functionland/fula-api" diff --git a/crates/fula-js/Cargo.toml b/crates/fula-js/Cargo.toml index 9f0ce40..23e5c0d 100644 --- a/crates/fula-js/Cargo.toml +++ b/crates/fula-js/Cargo.toml @@ -4,7 +4,7 @@ description = "JavaScript/TypeScript SDK for Fula decentralized storage - WASM b # Hard-coded (not workspace-inherited) because wasm-pack <= 0.13 fails # to parse `*.workspace = true` keys in its own manifest scan. Keep # these in sync with `[workspace.package]` in the root Cargo.toml. -version = "0.6.19" +version = "0.6.20" edition = "2021" license = "MIT OR Apache-2.0" repository = "https://github.com/functionland/fula-api" diff --git a/packages/fula_client/CHANGELOG.md b/packages/fula_client/CHANGELOG.md index 81438b6..3460add 100644 --- a/packages/fula_client/CHANGELOG.md +++ b/packages/fula_client/CHANGELOG.md @@ -5,6 +5,37 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.20] - 2026-08-23 + +### Fixed + +- **The bridge no longer serialises every call behind one lock.** + `EncryptedClientHandle` wrapped the client in a single + `Arc>` -- not per-bucket. `loadForest` took the + EXCLUSIVE guard and held it across the whole network fetch, so ONE slow or + damaged bucket blocked every other call in the app, including reads of + healthy buckets. A Dart-side `.timeout()` could not rescue it: the binding + exposes no cancel handle for `loadForest`, so the Rust future kept running + and kept the guard after the caller gave up. The lock is removed rather than + made per-bucket -- `EncryptedClient` has no `&mut self` methods, and the + SDK already owns per-bucket locking (`forest_cache`, `migration_locks`, + `bucket_write_mutex`). + +- **Web builds now honour `timeoutSeconds`.** `reqwest`'s client-level + timeout is a no-op on `wasm32`, so the browser ran with NO request timeout + at all and a stalled fetch could hang for the lifetime of the tab. The wasm + build now sets the timeout per-request, which reqwest enforces with an + `AbortController`. A browser timeout surfaces as an ordinary transport + error, identical in shape to native. + +### Known issue + +- Key rotation (`rewrapObject` / `rotateBucket`) is no longer serialised + against uploads by the removed bridge lock. `rewrap_object_dek` is a + GET-modify-PUT that takes no per-bucket lock, so an upload landing between + its GET and PUT is overwritten by the re-encrypted old data. Fix is + `If-Match`/ETag optimistic concurrency inside `rewrap_object_dek`; + tracked separately. ## [0.6.12] - 2026-06-16 ## [0.6.11] - 2026-06-16 diff --git a/packages/fula_client/ios/fula_client.podspec b/packages/fula_client/ios/fula_client.podspec index e4a6e2f..ee96296 100644 --- a/packages/fula_client/ios/fula_client.podspec +++ b/packages/fula_client/ios/fula_client.podspec @@ -6,7 +6,7 @@ Pod::Spec.new do |s| s.name = 'fula_client' - s.version = '0.6.19' + s.version = '0.6.20' s.summary = 'Flutter SDK for Fula decentralized storage' s.description = <<-DESC A Flutter plugin providing client-side encryption, metadata privacy, diff --git a/packages/fula_client/pubspec.yaml b/packages/fula_client/pubspec.yaml index 50184fa..c0b948f 100644 --- a/packages/fula_client/pubspec.yaml +++ b/packages/fula_client/pubspec.yaml @@ -1,6 +1,6 @@ name: fula_client description: Flutter SDK for Fula decentralized storage with client-side encryption, metadata privacy, and secure sharing. -version: 0.6.19 +version: 0.6.20 homepage: https://fx.land repository: https://github.com/functionland/fula-api issue_tracker: https://github.com/functionland/fula-api/issues