From 4f0f69131b77d2a80be8c7e8d464c7da50aa1268 Mon Sep 17 00:00:00 2001 From: Jinpy Date: Tue, 22 Sep 2026 14:15:13 +0800 Subject: [PATCH] fix(plugins): carry plugin file handle ids as uuid strings --- .../plugins/PluginWorkbenchHost.spec.ts | 5 +- .../plugins/PluginWorkbenchHost.vue | 30 +++-- apps/desktop/src/lib/backend/http.ts | 6 +- apps/desktop/src/lib/backend/tauri.ts | 9 +- src-tauri/src/commands/plugin_file.rs | 120 +++++++++++------- 5 files changed, 104 insertions(+), 66 deletions(-) diff --git a/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts b/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts index c86b8882fc..0403cec228 100644 --- a/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts +++ b/apps/desktop/src/components/plugins/PluginWorkbenchHost.spec.ts @@ -148,7 +148,8 @@ describe("PluginWorkbenchHost initialization", () => { it("claims OS drops over its iframe and forwards opened handles to the plugin", async () => { const { frame, postMessage } = await mountHost(); const elementFromPoint = vi.spyOn(document, "elementFromPoint").mockReturnValue(frame); - mocks.openPluginLocalFile.mockResolvedValue({ handleId: 7, name: "a.txt", size: 3, contentType: "text/plain", write: false }); + // The Rust registry hands out uuid strings; the `t` prefix stays opaque. + mocks.openPluginLocalFile.mockResolvedValue({ handleId: "0d9f6d26-9e0e-4b1f-8f9a-2b6d3c5a7e81", name: "a.txt", size: 3, contentType: "text/plain", write: false }); const claimed = !document.dispatchEvent( new CustomEvent("dbx:tauri-file-drop", { @@ -160,7 +161,7 @@ describe("PluginWorkbenchHost initialization", () => { expect(claimed).toBe(true); await vi.waitFor(() => { const posted = postMessage.mock.calls.map(([message]) => message as Record); - expect(posted.some((message) => message.type === "filedrop" && (message.files as Array>)?.some((file) => file.handleId === "t7" && file.name === "a.txt"))).toBe(true); + expect(posted.some((message) => message.type === "filedrop" && (message.files as Array>)?.some((file) => file.handleId === "t0d9f6d26-9e0e-4b1f-8f9a-2b6d3c5a7e81" && file.name === "a.txt"))).toBe(true); }); expect(mocks.openPluginLocalFile).toHaveBeenCalledWith("sample", "/tmp/a.txt", false); elementFromPoint.mockRestore(); diff --git a/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue b/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue index 55bd305455..a3dd3f6c00 100644 --- a/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue +++ b/apps/desktop/src/components/plugins/PluginWorkbenchHost.vue @@ -60,15 +60,17 @@ let loadGeneration = 0; // --- Plugin file-transfer bridge (native dialogs + OS file drops) --------- // The sandboxed iframe cannot reach local files, so handles live here: Tauri -// handles wrap the plugin_file registry in Rust (`t` ids); the web host -// keeps File objects and in-memory save buffers (`w` ids). Only paths that -// came from a native dialog or an OS drop reach plugin_file_open — never a -// plugin-supplied string. +// handles wrap the plugin_file registry in Rust (`t` ids); the web host +// keeps File objects and in-memory save buffers (`w` ids). Handle ids are +// opaque strings end to end — never run them through Number(): ids above +// Number.MAX_SAFE_INTEGER silently round, and the registry then rejects every +// read with "unknown plugin file handle". Only paths that came from a native +// dialog or an OS drop reach plugin_file_open — never a plugin-supplied string. let webFileSequence = 0; const webPickedFiles = new Map(); const webSaveBuffers = new Map }>(); -const openTauriHandles = new Set(); +const openTauriHandles = new Set(); const tauriHandlePrefix = "t"; const webHandlePrefix = "w"; @@ -81,9 +83,9 @@ function encodeBytesBase64(bytes: Uint8Array): string { return btoa(binary); } -function parseHandleId(handleId: string): { source: "tauri" | "web"; numericId: number } { - if (handleId.startsWith(tauriHandlePrefix)) return { source: "tauri", numericId: Number(handleId.slice(tauriHandlePrefix.length)) }; - if (handleId.startsWith(webHandlePrefix)) return { source: "web", numericId: Number(handleId.slice(webHandlePrefix.length)) }; +function parseHandleId(handleId: string): { source: "tauri" | "web"; rawId: string } { + if (handleId.startsWith(tauriHandlePrefix)) return { source: "tauri", rawId: handleId.slice(tauriHandlePrefix.length) }; + if (handleId.startsWith(webHandlePrefix)) return { source: "web", rawId: handleId.slice(webHandlePrefix.length) }; throw new Error("Unknown file handle"); } @@ -149,7 +151,7 @@ async function readPluginFileChunkById(pluginId: string, handleId: string, offse const parsed = parseHandleId(handleId); if (parsed.source === "tauri") { const { readPluginLocalFileChunk } = await tauriFileApi(); - return readPluginLocalFileChunk(pluginId, parsed.numericId, offset, length); + return readPluginLocalFileChunk(pluginId, parsed.rawId, offset, length); } const file = webPickedFiles.get(handleId); if (!file) throw new Error("Unknown file handle"); @@ -217,7 +219,7 @@ async function writePluginFileChunkById(pluginId: string, handleId: string, offs const parsed = parseHandleId(handleId); if (parsed.source === "tauri") { const { writePluginLocalFileChunk } = await tauriFileApi(); - return writePluginLocalFileChunk(pluginId, parsed.numericId, offset, encodeBytesBase64(bytes)); + return writePluginLocalFileChunk(pluginId, parsed.rawId, offset, encodeBytesBase64(bytes)); } const buffer = webSaveBuffers.get(handleId); if (!buffer) throw new Error("Unknown file handle"); @@ -229,8 +231,8 @@ async function finishPluginFileSave(pluginId: string, handleId: string): Promise const parsed = parseHandleId(handleId); if (parsed.source === "tauri") { const { closePluginLocalFile } = await tauriFileApi(); - openTauriHandles.delete(parsed.numericId); - await closePluginLocalFile(pluginId, parsed.numericId); + openTauriHandles.delete(parsed.rawId); + await closePluginLocalFile(pluginId, parsed.rawId); return; } const buffer = webSaveBuffers.get(handleId); @@ -256,8 +258,8 @@ async function closePluginFileHandleById(pluginId: string, handleId: string): Pr const parsed = parseHandleId(handleId); if (parsed.source === "tauri") { const { closePluginLocalFile } = await tauriFileApi(); - openTauriHandles.delete(parsed.numericId); - await closePluginLocalFile(pluginId, parsed.numericId); + openTauriHandles.delete(parsed.rawId); + await closePluginLocalFile(pluginId, parsed.rawId); return; } webPickedFiles.delete(handleId); diff --git a/apps/desktop/src/lib/backend/http.ts b/apps/desktop/src/lib/backend/http.ts index eceea980e8..3126827a36 100644 --- a/apps/desktop/src/lib/backend/http.ts +++ b/apps/desktop/src/lib/backend/http.ts @@ -5181,15 +5181,15 @@ export async function openPluginLocalFile(_pluginId: string, _path: string, _wri throw new Error("Plugin local file access is not available in the web backend"); } -export async function readPluginLocalFileChunk(_pluginId: string, _handleId: number, _offset: number, _length?: number): Promise { +export async function readPluginLocalFileChunk(_pluginId: string, _handleId: string, _offset: number, _length?: number): Promise { throw new Error("Plugin local file access is not available in the web backend"); } -export async function writePluginLocalFileChunk(_pluginId: string, _handleId: number, _offset: number, _dataBase64: string): Promise { +export async function writePluginLocalFileChunk(_pluginId: string, _handleId: string, _offset: number, _dataBase64: string): Promise { throw new Error("Plugin local file access is not available in the web backend"); } -export async function closePluginLocalFile(_pluginId: string, _handleId: number): Promise { +export async function closePluginLocalFile(_pluginId: string, _handleId: string): Promise { throw new Error("Plugin local file access is not available in the web backend"); } diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index 54a27f65ca..531f3a2bf9 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -2462,7 +2462,8 @@ export async function sendPluginBinary(pluginId: string, channel: string, dataBa } export interface PluginLocalFileHandle { - handleId: number; + /** Opaque uuid string from the Rust registry — never a number (JS doubles lose precision above 2^53). */ + handleId: string; name: string; size: number; contentType: string; @@ -2484,15 +2485,15 @@ export async function openPluginLocalFile(pluginId: string, path: string, write: return invoke("plugin_file_open", { pluginId, path, write }); } -export async function readPluginLocalFileChunk(pluginId: string, handleId: number, offset: number, length?: number): Promise { +export async function readPluginLocalFileChunk(pluginId: string, handleId: string, offset: number, length?: number): Promise { return invoke("plugin_file_read", { pluginId, handleId, offset, length }); } -export async function writePluginLocalFileChunk(pluginId: string, handleId: number, offset: number, dataBase64: string): Promise { +export async function writePluginLocalFileChunk(pluginId: string, handleId: string, offset: number, dataBase64: string): Promise { return invoke("plugin_file_write", { pluginId, handleId, offset, dataBase64 }); } -export async function closePluginLocalFile(pluginId: string, handleId: number): Promise { +export async function closePluginLocalFile(pluginId: string, handleId: string): Promise { return invoke("plugin_file_close", { pluginId, handleId }); } diff --git a/src-tauri/src/commands/plugin_file.rs b/src-tauri/src/commands/plugin_file.rs index 5f085ea204..8d50ba9981 100644 --- a/src-tauri/src/commands/plugin_file.rs +++ b/src-tauri/src/commands/plugin_file.rs @@ -40,7 +40,7 @@ struct OpenFile { #[derive(Default)] pub struct PluginFileState { - handles: Mutex>, + handles: Mutex>, } impl PluginFileState { @@ -52,7 +52,12 @@ impl PluginFileState { #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct PluginFileHandle { - handle_id: u64, + /// Opaque handle id, carried as a UUID string on the wire — the same + /// convention as `downloadId`. It must never be a JSON number: ids are + /// parsed as doubles in every JS layer, and a u64 loses precision above + /// `Number.MAX_SAFE_INTEGER`, which silently corrupted ids and failed + /// every bridge read/write with "unknown plugin file handle". + handle_id: String, name: String, size: u64, content_type: String, @@ -102,21 +107,23 @@ fn guess_content_type(path: &Path) -> String { fn with_handles( state: &PluginFileState, - body: impl FnOnce(&mut HashMap) -> Result, + body: impl FnOnce(&mut HashMap) -> Result, ) -> Result { let mut handles = state.handles.lock().map_err(|_| "plugin file registry poisoned".to_string())?; body(&mut handles) } fn owned_handle<'a>( - handles: &'a mut HashMap, + handles: &'a mut HashMap, plugin_id: &str, - handle_id: u64, + handle_id: &str, ) -> Result<&'a mut OpenFile, String> { - let entry = handles.get_mut(&handle_id).ok_or_else(|| "unknown plugin file handle".to_string())?; + let entry = handles.get_mut(handle_id).ok_or_else(|| "unknown plugin file handle".to_string())?; if entry.owner != plugin_id { - // Deliberately indistinguishable from a missing handle: a plugin must - // not learn that another plugin's handle ids exist. + // Response stays indistinguishable from a missing handle: a plugin must + // not learn that another plugin's handle ids exist. The host log is the + // one place the two cases can be told apart when debugging. + log::warn!("plugin file handle belongs to another plugin (caller: {plugin_id})"); return Err("unknown plugin file handle".to_string()); } Ok(entry) @@ -162,10 +169,11 @@ pub fn open_plugin_file( return Err(format!("too many open plugin file handles (max {MAX_OPEN_HANDLES})")); } // Unguessable id: a plugin that can only reach its own bridge must not - // be able to walk the registry by trying sequential ids. - let bytes = Uuid::new_v4().into_bytes(); - let handle_id = u64::from_be_bytes(bytes[0..8].try_into().expect("uuid prefix")); - handles.insert(handle_id, OpenFile { file, write, size, owner: plugin_id.to_string() }); + // be able to walk the registry by trying sequential ids. Same wire + // convention as downloadId: uuid v4, carried as a string so no JS + // layer can lose precision on it. + let handle_id = Uuid::new_v4().to_string(); + handles.insert(handle_id.clone(), OpenFile { file, write, size, owner: plugin_id.to_string() }); Ok(PluginFileHandle { handle_id, name, size, content_type: guess_content_type(path), write }) }) } @@ -173,7 +181,7 @@ pub fn open_plugin_file( pub fn read_plugin_file_chunk( state: &PluginFileState, plugin_id: &str, - handle_id: u64, + handle_id: &str, offset: u64, length: Option, ) -> Result { @@ -202,7 +210,7 @@ pub fn read_plugin_file_chunk( pub fn write_plugin_file_chunk( state: &PluginFileState, plugin_id: &str, - handle_id: u64, + handle_id: &str, offset: u64, data_base64: &str, ) -> Result { @@ -224,7 +232,7 @@ pub fn write_plugin_file_chunk( /// Close a handle. Write handles are flushed to disk before dropping; the /// bytes were already handed to the OS through `write_all`, so failure here /// means the OS could not flush — surfaced to the plugin as an error. -pub fn close_plugin_file(state: &PluginFileState, plugin_id: &str, handle_id: u64) -> Result<(), String> { +pub fn close_plugin_file(state: &PluginFileState, plugin_id: &str, handle_id: &str) -> Result<(), String> { with_handles(state, |handles| { { let entry = owned_handle(handles, plugin_id, handle_id)?; @@ -232,7 +240,7 @@ pub fn close_plugin_file(state: &PluginFileState, plugin_id: &str, handle_id: u6 entry.file.sync_all().map_err(|error| format!("flush failed: {error}"))?; } } - handles.remove(&handle_id).map(|_| ()).ok_or_else(|| "unknown plugin file handle".to_string()) + handles.remove(handle_id).map(|_| ()).ok_or_else(|| "unknown plugin file handle".to_string()) }) } @@ -250,27 +258,31 @@ pub fn plugin_file_open( pub fn plugin_file_read( state: State<'_, PluginFileState>, plugin_id: String, - handle_id: u64, + handle_id: String, offset: u64, length: Option, ) -> Result { - read_plugin_file_chunk(&state, &plugin_id, handle_id, offset, length) + read_plugin_file_chunk(&state, &plugin_id, &handle_id, offset, length) } #[tauri::command] pub fn plugin_file_write( state: State<'_, PluginFileState>, plugin_id: String, - handle_id: u64, + handle_id: String, offset: u64, data_base64: String, ) -> Result { - write_plugin_file_chunk(&state, &plugin_id, handle_id, offset, &data_base64) + write_plugin_file_chunk(&state, &plugin_id, &handle_id, offset, &data_base64) } #[tauri::command] -pub fn plugin_file_close(state: State<'_, PluginFileState>, plugin_id: String, handle_id: u64) -> Result<(), String> { - close_plugin_file(&state, &plugin_id, handle_id) +pub fn plugin_file_close( + state: State<'_, PluginFileState>, + plugin_id: String, + handle_id: String, +) -> Result<(), String> { + close_plugin_file(&state, &plugin_id, &handle_id) } #[cfg(test)] @@ -296,15 +308,15 @@ mod tests { assert!(!handle.write); assert_eq!(handle.content_type, "application/octet-stream"); - let chunk = read_plugin_file_chunk(&state, OWNER, handle.handle_id, 6, Some(6)).expect("read"); + let chunk = read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 6, Some(6)).expect("read"); assert_eq!(BASE64.decode(chunk.data_base64).unwrap(), b"plugin"); assert!(!chunk.eof); - let tail = read_plugin_file_chunk(&state, OWNER, handle.handle_id, 13, Some(64)).expect("read tail"); + let tail = read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 13, Some(64)).expect("read tail"); assert_eq!(BASE64.decode(tail.data_base64).unwrap(), b"bridge"); assert!(tail.eof); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); - assert!(read_plugin_file_chunk(&state, OWNER, handle.handle_id, 0, None).is_err()); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); + assert!(read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, None).is_err()); let _ = std::fs::remove_file(path); } @@ -314,16 +326,16 @@ mod tests { let state = PluginFileState::new(); let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), false).expect("open"); - let foreign_read = read_plugin_file_chunk(&state, OTHER, handle.handle_id, 0, None).unwrap_err(); + let foreign_read = read_plugin_file_chunk(&state, OTHER, &handle.handle_id, 0, None).unwrap_err(); assert_eq!(foreign_read, "unknown plugin file handle"); let foreign_write = - write_plugin_file_chunk(&state, OTHER, handle.handle_id, 0, &BASE64.encode(b"x")).unwrap_err(); + write_plugin_file_chunk(&state, OTHER, &handle.handle_id, 0, &BASE64.encode(b"x")).unwrap_err(); assert_eq!(foreign_write, "unknown plugin file handle"); - let foreign_close = close_plugin_file(&state, OTHER, handle.handle_id).unwrap_err(); + let foreign_close = close_plugin_file(&state, OTHER, &handle.handle_id).unwrap_err(); assert_eq!(foreign_close, "unknown plugin file handle"); - assert!(read_plugin_file_chunk(&state, OWNER, handle.handle_id, 0, None).is_ok()); + assert!(read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, None).is_ok()); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); let _ = std::fs::remove_file(path); } @@ -335,15 +347,15 @@ mod tests { assert!(handle.write); assert_eq!(handle.size, 0); assert!( - read_plugin_file_chunk(&state, OWNER, handle.handle_id, 0, None).is_err(), + read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, None).is_err(), "write handles reject reads" ); let written = - write_plugin_file_chunk(&state, OWNER, handle.handle_id, 0, &BASE64.encode(b"fresh")).expect("write"); + write_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, &BASE64.encode(b"fresh")).expect("write"); assert_eq!(written.written, 5); assert_eq!(written.next_offset, 5); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); assert_eq!(std::fs::read(&path).unwrap(), b"fresh"); let _ = std::fs::remove_file(path); @@ -355,8 +367,8 @@ mod tests { let _ = std::fs::remove_file(&path); let state = PluginFileState::new(); let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), true).expect("open write"); - write_plugin_file_chunk(&state, OWNER, handle.handle_id, 0, &BASE64.encode(b"data")).expect("write"); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + write_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, &BASE64.encode(b"data")).expect("write"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); assert_eq!(std::fs::read(&path).unwrap(), b"data"); let _ = std::fs::remove_file(path); } @@ -368,8 +380,8 @@ mod tests { open_plugin_file(&state, OWNER, std::env::temp_dir().to_string_lossy().as_ref(), false).unwrap_err(); assert!(error.contains("directory")); assert!(open_plugin_file(&state, " ", std::env::temp_dir().to_string_lossy().as_ref(), false).is_err()); - assert!(read_plugin_file_chunk(&state, OWNER, 999_999, 0, None).is_err()); - assert!(close_plugin_file(&state, OWNER, 999_999).is_err()); + assert!(read_plugin_file_chunk(&state, OWNER, "00000000-0000-4000-8000-000000000099", 0, None).is_err()); + assert!(close_plugin_file(&state, OWNER, "00000000-0000-4000-8000-000000000099").is_err()); } #[test] @@ -377,11 +389,11 @@ mod tests { let path = temp_file("bounds.bin", b"abc"); let state = PluginFileState::new(); let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), false).expect("open"); - assert!(read_plugin_file_chunk(&state, OWNER, handle.handle_id, 4, None).is_err()); - let at_end = read_plugin_file_chunk(&state, OWNER, handle.handle_id, 3, None).expect("read at eof"); + assert!(read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 4, None).is_err()); + let at_end = read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 3, None).expect("read at eof"); assert!(at_end.eof); assert_eq!(at_end.length, 0); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); let _ = std::fs::remove_file(path); } @@ -391,14 +403,36 @@ mod tests { let state = PluginFileState::new(); let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), false).expect("open"); assert_eq!(handle.content_type, "image/svg+xml"); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); let _ = std::fs::remove_file(&path); path.set_extension("weird"); std::fs::write(&path, b"x").unwrap(); let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), false).expect("open"); assert_eq!(handle.content_type, "application/octet-stream"); - close_plugin_file(&state, OWNER, handle.handle_id).expect("close"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); + let _ = std::fs::remove_file(path); + } + + #[test] + fn handle_id_follows_the_uuid_string_wire_convention() { + // Handle ids ride the bridge as opaque strings, same as downloadId. + // The old wire format serialized a u64 JSON number: ids above + // Number.MAX_SAFE_INTEGER were rounded by every JS layer, so the host + // looked a fresh handle up under a different key and failed every + // read/write with "unknown plugin file handle". + let path = temp_file("wire-uuid.bin", b"payload"); + let state = PluginFileState::new(); + let handle = open_plugin_file(&state, OWNER, path.to_string_lossy().as_ref(), false).expect("open"); + let wire_json = serde_json::to_string(&handle.handle_id).expect("serialize id"); + assert_eq!(wire_json, format!("\"{}\"", handle.handle_id), "the wire id is a JSON string, never a number"); + let parsed = uuid::Uuid::parse_str(&handle.handle_id).expect("id parses as uuid, like downloadId"); + assert_eq!(parsed.to_string(), handle.handle_id); + + // The exact string the host sent back must hit the same registry key. + let chunk = read_plugin_file_chunk(&state, OWNER, &handle.handle_id, 0, None).expect("read via wire id"); + assert_eq!(BASE64.decode(chunk.data_base64).unwrap(), b"payload"); + close_plugin_file(&state, OWNER, &handle.handle_id).expect("close"); let _ = std::fs::remove_file(path); } }