diff --git a/crates/ironrdp-web/src/clipboard.rs b/crates/ironrdp-web/src/clipboard.rs index 011cc8d2d..8bd9c62ca 100644 --- a/crates/ironrdp-web/src/clipboard.rs +++ b/crates/ironrdp-web/src/clipboard.rs @@ -173,6 +173,16 @@ pub(crate) enum WasmClipboardBackendMessage { LocksExpired { clip_data_ids: Vec, }, + /// [MS-RDPECLIP] 2.2.3.2 Remote's response to one of our outbound Format Lists. + /// + /// `ok` is `true` when the remote accepted the advertised formats + /// (`CB_RESPONSE_OK`) and `false` when it rejected them (`CB_RESPONSE_FAIL`). + /// A rejected advertise is silently discarded by the remote, so a file paste + /// cannot proceed; backends use this to detect and recover from a refused + /// paste instead of stalling until a lock timeout. + FormatListResponse { + ok: bool, + }, // JS-initiated file transfer operations /// JS requests file contents from remote (download). @@ -235,6 +245,7 @@ pub(crate) struct JsClipboardCallbacks { pub(crate) on_lock: Option, pub(crate) on_unlock: Option, pub(crate) on_locks_expired: Option, + pub(crate) on_format_list_response: Option, } impl WasmClipboard { @@ -841,6 +852,16 @@ impl WasmClipboard { ); } } + WasmClipboardBackendMessage::FormatListResponse { ok } => { + if let Some(callback) = self.js_callbacks.on_format_list_response.as_ref() { + if let Err(e) = callback.call1(&JsValue::NULL, &JsValue::from_bool(ok)) { + error!(error = ?e, ok, "Failed to call JS format list response callback"); + return Ok(()); + } + } else { + trace!(ok, "Format list response received but no JS callback registered"); + } + } // The following variants are handled directly in the event loop and should never reach here WasmClipboardBackendMessage::FileContentsRequestSend { .. } | WasmClipboardBackendMessage::FileContentsResponseSend { .. } @@ -948,6 +969,10 @@ impl CliprdrBackend for WasmClipboardBackend { self.send_event(WasmClipboardBackendMessage::Unlock { data_id }); } + fn on_format_list_response(&mut self, ok: bool) { + self.send_event(WasmClipboardBackendMessage::FormatListResponse { ok }); + } + fn on_remote_file_list(&mut self, files: &[ironrdp::cliprdr::pdu::FileDescriptor], clip_data_id: Option) { let file_metadata: Vec = files.iter().map(FileMetadata::from_file_descriptor).collect(); @@ -1227,6 +1252,7 @@ mod tests { on_lock: Some(js_sys::Function::new_no_args("")), on_unlock: Some(js_sys::Function::new_no_args("")), on_locks_expired: Some(js_sys::Function::new_no_args("")), + on_format_list_response: Some(js_sys::Function::new_no_args("")), } } diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index 618901370..ba1c31e5c 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -79,6 +79,7 @@ struct SessionBuilderInner { lock_callback: Option, unlock_callback: Option, locks_expired_callback: Option, + format_list_response_callback: Option, // Setting printer stream callbacks activates the virtual printer. invalid_print_job_stream_callbacks: bool, @@ -120,6 +121,7 @@ impl Default for SessionBuilderInner { lock_callback: None, unlock_callback: None, locks_expired_callback: None, + format_list_response_callback: None, invalid_print_job_stream_callbacks: false, print_job_stream_callbacks: None, @@ -276,6 +278,9 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { |locks_expired_callback: JsValue| { self.0.borrow_mut().locks_expired_callback = locks_expired_callback.dyn_into::().ok(); }; + |format_list_response_callback: JsValue| { + self.0.borrow_mut().format_list_response_callback = format_list_response_callback.dyn_into::().ok(); + }; |print_job_stream_callbacks: JsValue| { let mut inner = self.0.borrow_mut(); match parse_print_job_stream_callbacks(print_job_stream_callbacks) { @@ -341,6 +346,7 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { lock_callback, unlock_callback, locks_expired_callback, + format_list_response_callback, invalid_print_job_stream_callbacks, print_job_stream_callbacks, printer_name, @@ -381,6 +387,7 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { lock_callback = inner.lock_callback.clone(); unlock_callback = inner.unlock_callback.clone(); locks_expired_callback = inner.locks_expired_callback.clone(); + format_list_response_callback = inner.format_list_response_callback.clone(); invalid_print_job_stream_callbacks = inner.invalid_print_job_stream_callbacks; print_job_stream_callbacks = inner.print_job_stream_callbacks.clone(); printer_name = inner.printer_name.clone(); @@ -410,6 +417,7 @@ impl iron_remote_desktop::SessionBuilder for SessionBuilder { on_lock: lock_callback, on_unlock: unlock_callback, on_locks_expired: locks_expired_callback, + on_format_list_response: format_list_response_callback, }, ) }); diff --git a/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.test.ts b/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.test.ts index e57c6f56f..0e9469270 100644 --- a/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.test.ts +++ b/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.test.ts @@ -11,6 +11,7 @@ vi.mock('./extensions', () => ({ lockCallback: (cb: unknown) => ({ ident: 'lock_callback', value: cb }), unlockCallback: (cb: unknown) => ({ ident: 'unlock_callback', value: cb }), locksExpiredCallback: (cb: unknown) => ({ ident: 'locks_expired_callback', value: cb }), + formatListResponseCallback: (cb: unknown) => ({ ident: 'format_list_response_callback', value: cb }), requestFileContents: (params: unknown) => ({ ident: 'request_file_contents', value: params }), submitFileContents: (params: unknown) => ({ ident: 'submit_file_contents', value: params }), initiateFileCopy: (files: unknown) => ({ ident: 'initiate_file_copy', value: files }), @@ -256,7 +257,7 @@ describe('RdpFileTransferProvider', () => { }); describe('upload lifecycle callbacks', () => { - it('should call onUploadStarted and onUploadFinished around initiateFileCopy', async () => { + it('suppresses monitoring on advertise and defers the resume past the wire send', async () => { const onUploadStarted = vi.fn(); const onUploadFinished = vi.fn(); @@ -268,21 +269,271 @@ describe('RdpFileTransferProvider', () => { const files = [new File(['x'], 'x.txt', { type: 'text/plain' })]; const { completion: uploadPromise } = p.uploadFiles(files); - // Both should fire immediately (monitoring suppression is brief) + // Monitoring is suppressed before the FormatList goes on the wire... expect(onUploadStarted).toHaveBeenCalledTimes(1); - expect(onUploadFinished).toHaveBeenCalledTimes(1); expect(s.invokeExtension).toHaveBeenCalledTimes(1); - - // Upload started should fire before invokeExtension (initiateFileCopy) expect(onUploadStarted.mock.invocationCallOrder[0]).toBeLessThan( s.invokeExtension.mock.invocationCallOrder[0], ); + // ...but the resume is now DEFERRED (held until the paste is pulled or we + // give up), so the 100ms monitor poll cannot clobber the file FormatList. + expect(onUploadFinished).not.toHaveBeenCalled(); - // Clean up + // Dispose resumes monitoring exactly once and rejects the pending upload. p.dispose(); + expect(onUploadFinished).toHaveBeenCalledTimes(1); await expect(uploadPromise).rejects.toThrow('RdpFileTransferProvider disposed'); }); + it('resumes monitoring on the first FileContentsRequest (remote pulled the files)', async () => { + const onUploadFinished = vi.fn(); + const { provider: p } = setupProvider({ onUploadFinished }); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + expect(onUploadFinished).not.toHaveBeenCalled(); + + // Remote requests file size for index 0 -> paste was pulled -> resume. + // @ts-expect-error - exercising the private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 1, index: 0, flags: 1, position: 0, size: 8 }); + expect(onUploadFinished).toHaveBeenCalledTimes(1); + + p.dispose(); + // Already resumed on the pull, so dispose does not resume again. + expect(onUploadFinished).toHaveBeenCalledTimes(1); + await expect(completion).rejects.toThrow('RdpFileTransferProvider disposed'); + }); + + it('fails the upload and resumes monitoring if the remote never pulls (watchdog)', async () => { + vi.useFakeTimers(); + try { + const onUploadFinished = vi.fn(); + const { provider: p } = setupProvider({ onUploadFinished }); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['x'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + const rejected = expect(completion).rejects.toThrow(/did not request the files/i); + + // No FileContentsRequest ever arrives; after the 60s lock window the + // watchdog fires: resume monitoring + fail the upload. + await vi.advanceTimersByTimeAsync(60_000); + await rejected; + + expect(onUploadFinished).toHaveBeenCalledTimes(1); + const err: FileTransferError = errorHandler.mock.calls.at(-1)![0]; + expect(err.direction).toBe('upload'); + + // uploadState was cleared, so a fresh upload starts instead of throwing + // "Upload already in progress" -- the wedge is gone, no reload needed. + const second = p.uploadFiles(files); + p.dispose(); + await expect(second.completion).rejects.toThrow('RdpFileTransferProvider disposed'); + } finally { + vi.useRealTimers(); + } + }); + + it('does NOT fail a pulled upload that keeps making progress', async () => { + // Regression guard for normal uploads: a slow-but-progressing transfer resets + // the inactivity watchdog on every request, so it must never be killed even + // long past the window. + vi.useFakeTimers(); + try { + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + let settled = false; + void completion.then( + () => { + settled = true; + }, + () => { + settled = true; + }, + ); + + // Remote keeps pulling, each request well inside the 60s window: the + // watchdog keeps resetting and never fires (total elapsed well past 60s). + for (let i = 0; i < 5; i++) { + // @ts-expect-error - private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 1, index: 0, flags: 1, position: 0, size: 8 }); + await vi.advanceTimersByTimeAsync(50_000); + } + + expect(errorHandler).not.toHaveBeenCalled(); + expect(settled).toBe(false); + + p.dispose(); + await expect(completion).rejects.toThrow('RdpFileTransferProvider disposed'); + } finally { + vi.useRealTimers(); + } + }); + + it('fails a pulled-then-idle upload after the inactivity window, releasing the wedge', async () => { + // Once pulling starts, if the remote goes silent (e.g. it grabbed the clipboard + // with a text/image copy that never reaches handleFilesAvailable), the + // inactivity watchdog releases uploadState so later uploads aren't wedged. + vi.useFakeTimers(); + try { + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + const rejected = expect(completion).rejects.toThrow(/stopped requesting the files/i); + + // Remote pulls once (arms the inactivity watchdog), then goes silent. + // @ts-expect-error - private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 1, index: 0, flags: 1, position: 0, size: 8 }); + await vi.advanceTimersByTimeAsync(60_000); + await rejected; + + expect(errorHandler).toHaveBeenCalled(); + expect((errorHandler.mock.calls.at(-1)![0] as FileTransferError).direction).toBe('upload'); + + // uploadState released -> a fresh upload doesn't throw "Upload already in progress". + const second = p.uploadFiles(files); + p.dispose(); + await expect(second.completion).rejects.toThrow('RdpFileTransferProvider disposed'); + } finally { + vi.useRealTimers(); + } + }); + + it('stops the inactivity watchdog once the upload completes (no lingering timer)', async () => { + // The final FileContentsRequest arms the inactivity watchdog; completion must + // disarm it (finishUploadBatch). Otherwise a stray 60s timer lingers after every + // completed upload -- harmless today thanks to the uploadState guard, but it + // should not exist, and a future change could let it fire against fresh state. + vi.useFakeTimers(); + try { + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + let resolved = false; + void completion.then(() => { + resolved = true; + }); + + // Remote pulls the whole 4-byte file in one RANGE. jsdom schedules the + // FileReader on a timer, so flushing it completes the batch. + // @ts-expect-error - private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 1, index: 0, flags: 2, position: 0, size: 4 }); + await vi.advanceTimersByTimeAsync(100); + expect(resolved).toBe(true); + + // Completion ran finishUploadBatch, which cleared the inactivity watchdog: + // no upload timer is left pending (the lingering-timer bug this guards). + expect(vi.getTimerCount()).toBe(0); + + // ...and well past the window nothing fails. + await vi.advanceTimersByTimeAsync(60_000); + expect(errorHandler).not.toHaveBeenCalled(); + + p.dispose(); + } finally { + vi.useRealTimers(); + } + }); + + it('does NOT fail a re-paste that goes idle (isRePaste guard)', async () => { + // After an upload completes, the DroppedFile metadata is retained so the remote + // can re-paste. A re-paste rebuilds uploadState with isRePaste=true and carries + // no external promise, so the inactivity watchdog must leave it alone rather than + // emit a bogus upload error / try to reject a promise nobody is awaiting. + vi.useFakeTimers(); + try { + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + let resolved = false; + void completion.then(() => { + resolved = true; + }); + + // First paste: remote pulls the whole file -> upload completes, metadata retained. + // @ts-expect-error - private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 1, index: 0, flags: 2, position: 0, size: 4 }); + await vi.advanceTimersByTimeAsync(100); + expect(resolved).toBe(true); + + // Re-paste: no uploadState but retainedFiles present -> the next request + // rebuilds an isRePaste state and re-arms the inactivity watchdog. + // @ts-expect-error - private callback the WASM layer drives + p.handleFileContentsRequest({ streamId: 2, index: 0, flags: 1, position: 0, size: 8 }); + + // The window elapses with no further pulls. A fresh upload would be failed + // here; a re-paste must NOT be (no promise to reject, no error to surface). + await vi.advanceTimersByTimeAsync(60_000); + expect(errorHandler).not.toHaveBeenCalled(); + + p.dispose(); + } finally { + vi.useRealTimers(); + } + }); + + it('releases the in-flight upload when the remote replaces the clipboard', async () => { + // A remote FormatList (the remote copied its own files) supersedes our advertise; + // the upload can no longer complete, so uploadState must be released or it wedges + // every later upload. Symmetric to the rejected-advertise recovery. + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['data'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + const rejected = expect(completion).rejects.toThrow(/remote clipboard changed/i); + + // Remote copies its own files mid-upload. + // @ts-expect-error - accessing private method for testing + p.handleFilesAvailable([{ name: 'remote.txt', size: 10, lastModified: 0 }] as FileInfo[]); + await rejected; + + expect(errorHandler).toHaveBeenCalled(); + expect((errorHandler.mock.calls.at(-1)![0] as FileTransferError).direction).toBe('upload'); + + // uploadState released -> a fresh upload doesn't throw "Upload already in progress". + const second = p.uploadFiles(files); + p.dispose(); + await expect(second.completion).rejects.toThrow('RdpFileTransferProvider disposed'); + }); + + it('does NOT fail an in-flight upload on an Unlock (Unlock is not a paste-failure signal)', async () => { + // The peer/cliprdr emit Unlock routinely (snapshot of the FormatList, and the + // 60s timeout), often before any FileContentsRequest. Treating that as failure + // tore down uploads that would still be pulled, so Unlock must be ignored here. + const { provider: p } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['x'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + + // @ts-expect-error - private callback the WASM layer drives via unlockCallback + p.handleUnlock(0); + + // No error, and the upload state is intact (a second attempt still hits the guard). + expect(errorHandler).not.toHaveBeenCalled(); + expect(() => p.uploadFiles(files)).toThrow('Upload already in progress'); + + p.dispose(); + await expect(completion).rejects.toThrow('RdpFileTransferProvider disposed'); + }); + it('should call onUploadFinished even on initiateFileCopy failure', async () => { const onUploadStarted = vi.fn(); const onUploadFinished = vi.fn(); @@ -300,11 +551,91 @@ describe('RdpFileTransferProvider', () => { await expect(completion).rejects.toThrow('Failed to initiate file upload'); expect(onUploadStarted).toHaveBeenCalledTimes(1); - // onUploadFinished fires in finally block regardless + // The failure path resumes monitoring (resumeUploadMonitoring), so it fires once. expect(onUploadFinished).toHaveBeenCalledTimes(1); }); }); + describe('format list response (paste accept / reject)', () => { + // Pull the registered format_list_response_callback out of the builder + // extensions and invoke it, exercising the real wiring + handler together. + function fireFormatListResponse(p: RdpFileTransferProviderInstance, ok: boolean): void { + const exts = p.getBuilderExtensions() as unknown as Array<{ ident: string; value: (ok: boolean) => void }>; + const ext = exts.find((e) => e.ident === 'format_list_response_callback'); + if (ext === undefined) { + throw new Error('format_list_response_callback was not registered'); + } + ext.value(ok); + } + + it('registers a format_list_response_callback builder extension', () => { + const idents = (provider.getBuilderExtensions() as unknown as Array<{ ident: string }>).map((e) => e.ident); + expect(idents).toContain('format_list_response_callback'); + }); + + it('emits a format-list-response event carrying the ok flag', () => { + const handler = vi.fn(); + provider.on('format-list-response', handler); + + fireFormatListResponse(provider, true); + fireFormatListResponse(provider, false); + + expect(handler).toHaveBeenNthCalledWith(1, true); + expect(handler).toHaveBeenNthCalledWith(2, false); + }); + + it('fails an in-flight upload cleanly when the remote rejects the advertise', async () => { + const { provider: p, session: s } = setupProvider(); + const errorHandler = vi.fn(); + p.on('error', errorHandler); + + const files = [new File(['x'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + + // Remote rejects the file list before requesting any contents. + fireFormatListResponse(p, false); + + await expect(completion).rejects.toThrow(/rejected the file list/i); + const err: FileTransferError = errorHandler.mock.calls.at(-1)![0]; + expect(err.direction).toBe('upload'); + + // uploadState is cleared, so a fresh upload starts instead of throwing + // "Upload already in progress" (the wedge this fixes). Reaching a handle + // (not a throw) plus the initiateFileCopy call proves it restarted. + s.invokeExtension.mockClear(); + const second = p.uploadFiles(files); + expect(s.invokeExtension).toHaveBeenCalledTimes(1); + + p.dispose(); + await expect(second.completion).rejects.toThrow('RdpFileTransferProvider disposed'); + }); + + it('leaves an accepted advertise (ok=true) untouched', async () => { + const { provider: p } = setupProvider(); + const files = [new File(['x'], 'x.txt', { type: 'text/plain' })]; + const { completion } = p.uploadFiles(files); + + fireFormatListResponse(p, true); + + // Upload is still in progress: a second attempt still hits the guard. + expect(() => p.uploadFiles(files)).toThrow('Upload already in progress'); + + p.dispose(); + await expect(completion).rejects.toThrow('RdpFileTransferProvider disposed'); + }); + + it('surfaces a reject with no upload in progress without erroring', () => { + const errorHandler = vi.fn(); + const eventHandler = vi.fn(); + provider.on('error', errorHandler); + provider.on('format-list-response', eventHandler); + + expect(() => fireFormatListResponse(provider, false)).not.toThrow(); + expect(eventHandler).toHaveBeenCalledWith(false); + expect(errorHandler).not.toHaveBeenCalled(); + }); + }); + describe('sanitizeFileName', () => { it('should return a plain filename as-is', () => { expect(RdpFileTransferProvider.sanitizeFileName('file.txt')).toBe('file.txt'); diff --git a/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.ts b/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.ts index f7876a6b5..e7b48656d 100644 --- a/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.ts +++ b/web-client/iron-remote-desktop-rdp/src/RdpFileTransferProvider.ts @@ -8,6 +8,7 @@ import { lockCallback, unlockCallback, locksExpiredCallback, + formatListResponseCallback, requestFileContents, submitFileContents, initiateFileCopy, @@ -188,6 +189,11 @@ type EventMap = { * listeners can register all transfers eagerly before progress events arrive. */ 'upload-batch-started': [Map, DroppedFile[]]; 'files-available': [FileInfo[]]; + /** Remote's response to one of our outbound Format Lists: `true` = accepted + * (CB_RESPONSE_OK), `false` = rejected (CB_RESPONSE_FAIL). Fires for every + * outbound advertise, so consumers can drive paste from it (e.g. inject the + * paste keystroke on accept, retry on reject). */ + 'format-list-response': [boolean]; error: [FileTransferError]; }; @@ -262,6 +268,24 @@ export class RdpFileTransferProvider { private static readonly MAX_FILE_SIZE = 2 * 1024 * 1024 * 1024; /** Timeout for FileReader operations (60 seconds) to prevent stalled uploads */ private static readonly FILE_READER_TIMEOUT_MS = 60 * 1000; + /** + * How long to keep clipboard monitoring suppressed after advertising an upload + * while waiting for the remote to pull the files (first FileContentsRequest), + * before giving up. Matches the Rust cliprdr lock inactivity timeout (60s), so + * the JS side gives up exactly when the protocol lock does. If the remote never + * requests contents (the paste landed in a non-file target, or the advertise was + * clobbered), the watchdog resumes monitoring and fails the upload so its state + * cannot wedge later uploads. + */ + private static readonly PASTE_ACK_TIMEOUT_MS = 60 * 1000; + /** + * Upload inactivity window. After the remote starts pulling, each FileContentsRequest + * resets this; if pulls then stop for this long -- e.g. the remote grabbed the + * clipboard with a text/image copy that never reaches handleFilesAvailable -- the + * upload is failed so `uploadState` is released and later uploads aren't wedged. A + * slow-but-progressing transfer keeps resetting it, so it is never killed. + */ + private static readonly UPLOAD_INACTIVITY_TIMEOUT_MS = 60 * 1000; /** Maximum recursion depth when traversing dropped directories. */ private static readonly MAX_DIRECTORY_DEPTH = 32; /** Maximum total entries (files + directories) collected from a single drop. */ @@ -276,6 +300,23 @@ export class RdpFileTransferProvider { private activeDownloads: Map = new Map(); private uploadState?: UploadState; + // Upload paste-window watchdog. Armed when we advertise an upload, disarmed on the + // first FileContentsRequest (the remote pulled the files). Being armed is the single + // "advertised but not yet pulled" signal: on timeout it resumes monitoring and fails + // the never-pulled upload, and handleFormatListResponse consults it to fail a refused + // paste early (handleUnlock is a deliberate no-op: Unlock is not a reliable failure + // signal). Upload completion/failure run only after that first request, so they need + // not touch it. + private pasteAckTimeout?: ReturnType; + // Upload inactivity watchdog. Armed/reset on each FileContentsRequest once the remote + // starts pulling; fires if pulls stop for UPLOAD_INACTIVITY_TIMEOUT_MS (a stalled or + // clipboard-superseded paste) to release uploadState. Complements the paste-ack + // watchdog above, which only covers the "advertised but never pulled" case. + private uploadInactivityTimeout?: ReturnType; + // True between onUploadStarted (suppress monitoring) and onUploadFinished + // (resume), so resume fires exactly once even though it is now deferred until + // the paste is pulled, times out, fails, or the provider is disposed. + private uploadMonitoringSuppressed = false; // DroppedFile metadata retained after upload completes so re-paste works // without re-dropping. Cleared when a new upload starts or the manager is disposed. private retainedFiles?: DroppedFile[]; @@ -355,6 +396,7 @@ export class RdpFileTransferProvider { lockCallback((id: number) => this.handleLock(id)), unlockCallback((id: number) => this.handleUnlock(id)), locksExpiredCallback((ids: Uint32Array) => this.handleLocksExpired(ids)), + formatListResponseCallback((ok: boolean) => this.handleFormatListResponse(ok)), ]; } @@ -643,19 +685,23 @@ export class RdpFileTransferProvider { reject, }; - // Suppress clipboard monitoring briefly so the polling loop does not - // clobber our FormatList with a text/image update. Resume immediately - // after the FormatList is sent - the suppression window only needs to - // cover the race between suppressMonitoring() and the wire send. - // Upload state tracking continues independently via this.uploadState. - this.onUploadStarted?.(); + // Suppress clipboard monitoring so the 100ms polling loop cannot clobber our + // file FormatList with a stale text/image update during the paste window. It + // stays suppressed -- NOT just for the synchronous wire send, which left a + // race the monitor could win -- until we leave the advertised-but-not-pulled + // window: the remote pulls (first FileContentsRequest), or we give up on it + // (watchdog timeout or a rejected advertise). + this.suppressUploadMonitoring(); // Initiate file copy (broadcasts file list to remote) try { this.sendInitiateFileCopy(fileInfos); this.emit('upload-batch-started', transferIds, dropped); } catch (error) { + // Immediate failure: resume monitoring now and clear state so the + // next upload is not blocked. this.uploadState = undefined; + this.resumeUploadMonitoring(); const err: FileTransferError = { message: 'Failed to initiate file upload', direction: 'upload', @@ -663,17 +709,159 @@ export class RdpFileTransferProvider { }; this.emit('error', err); reject(new Error(err.message, { cause: error })); - } finally { - // Resume monitoring regardless of success/failure. The brief - // suppression window is intentionally short - just long enough - // to prevent the clipboard poll from racing with our FormatList. - this.onUploadFinished?.(); + return; } + + // FormatList is on the wire. Wait for the remote to pull the files; if it + // never does (paste landed in a non-file target, or the advertise was + // clobbered/rejected), the watchdog resumes monitoring and rejects this + // upload so its state cannot wedge later uploads ("Upload already in progress"). + this.armPasteAckWatchdog(); }); return { transferIds, completion }; } + /** Suppress clipboard monitoring for the duration of an upload's paste window. */ + private suppressUploadMonitoring(): void { + // Flip the flag before the callback so it holds even if the callback throws. + this.uploadMonitoringSuppressed = true; + try { + this.onUploadStarted?.(); + } catch (error) { + console.error('Error in onUploadStarted callback:', error); + } + } + + /** Resume clipboard monitoring (idempotent: fires onUploadFinished at most once + * per upload, since the resume point is now deferred past the wire send). */ + private resumeUploadMonitoring(): void { + if (!this.uploadMonitoringSuppressed) { + return; + } + // Flip the flag before the callback so it holds even if the callback throws. + this.uploadMonitoringSuppressed = false; + try { + this.onUploadFinished?.(); + } catch (error) { + console.error('Error in onUploadFinished callback:', error); + } + } + + /** Arm the paste-acknowledgment watchdog for the current upload. */ + private armPasteAckWatchdog(): void { + this.clearPasteAckWatchdog(); + // A new upload is starting: drop any inactivity watchdog left from a prior upload + // so it can't fire mid-way through this one. + this.clearUploadInactivityWatchdog(); + this.pasteAckTimeout = setTimeout(() => { + this.pasteAckTimeout = undefined; + // The remote never requested the files within the lock window. Resume + // monitoring and fail the pending upload so it cannot wedge later ones. + const state = this.uploadState; + if (state !== undefined && state.isRePaste !== true) { + this.failPendingUpload('The remote did not request the files in time, so the paste was not completed'); + } else { + this.resumeUploadMonitoring(); + } + }, RdpFileTransferProvider.PASTE_ACK_TIMEOUT_MS); + } + + /** Disarm the paste-acknowledgment watchdog, if armed. */ + private clearPasteAckWatchdog(): void { + if (this.pasteAckTimeout !== undefined) { + clearTimeout(this.pasteAckTimeout); + this.pasteAckTimeout = undefined; + } + } + + /** + * (Re)arm the upload inactivity watchdog (see {@link UPLOAD_INACTIVITY_TIMEOUT_MS}). + * Called on every FileContentsRequest, so continued pulls keep resetting it and a + * slow-but-progressing transfer is never killed; if pulls stop for the window the + * upload is failed (releasing uploadState). Skipped for re-pastes, matching the + * paste-ack watchdog. + */ + private resetUploadInactivityWatchdog(): void { + this.clearUploadInactivityWatchdog(); + this.uploadInactivityTimeout = setTimeout(() => { + this.uploadInactivityTimeout = undefined; + const state = this.uploadState; + if (state !== undefined && state.isRePaste !== true) { + this.failPendingUpload('The remote stopped requesting the files, so the paste did not complete'); + } + }, RdpFileTransferProvider.UPLOAD_INACTIVITY_TIMEOUT_MS); + } + + /** Disarm the upload inactivity watchdog, if armed. */ + private clearUploadInactivityWatchdog(): void { + if (this.uploadInactivityTimeout !== undefined) { + clearTimeout(this.uploadInactivityTimeout); + this.uploadInactivityTimeout = undefined; + } + } + + /** + * The remote acknowledged the paste by requesting file contents: the clobber + * window is over, so disarm the paste-ack watchdog and resume clipboard monitoring. + * Called on every FileContentsRequest; only the first disarms/resumes. Every request + * also (re)arms the inactivity watchdog so a stall after pulling began is recovered. + */ + private acknowledgePaste(): void { + if (this.pasteAckTimeout !== undefined) { + this.clearPasteAckWatchdog(); + this.resumeUploadMonitoring(); + } + this.resetUploadInactivityWatchdog(); + } + + /** + * Fail the in-flight upload (reject its completion, emit an upload error, clear + * uploadState) and resume monitoring. Used when the advertise is rejected or the + * paste is never pulled, so `uploadState` is released instead of lingering and + * throwing "Upload already in progress" on every later upload. + */ + private failPendingUpload(message: string): void { + this.clearPasteAckWatchdog(); + this.clearUploadInactivityWatchdog(); + this.resumeUploadMonitoring(); + const state = this.uploadState; + if (state === undefined) { + return; + } + // Abort any in-flight chunk reads and clear their timeouts before releasing the + // state (mirrors dispose()). Otherwise a read still running when the paste is torn + // down keeps its FileReader and a 60s reader-timeout alive for an upload that has + // already been rejected. + for (const timeout of state.readerTimeouts.values()) { + clearTimeout(timeout); + } + state.readerTimeouts.clear(); + for (const reader of state.activeReaders.values()) { + reader.abort(); + } + state.activeReaders.clear(); + + const err: FileTransferError = { message, direction: 'upload' }; + this.emit('error', err); + const { reject } = state; + this.uploadState = undefined; + reject(new Error(message)); + } + + /** + * Finalize a fully-accounted upload batch (every counted file either served in full + * or permanently failed). Stops the inactivity watchdog so it cannot linger past + * completion, retains the DroppedFile metadata so a re-paste from the remote can serve + * the data again, clears `uploadState`, and resolves the completion promise. + */ + private finishUploadBatch(state: UploadState): void { + this.clearUploadInactivityWatchdog(); + this.retainedFiles = state.droppedFiles; + this.uploadState = undefined; + state.resolve(); + } + /** * Show a file picker dialog and return selected files. * @@ -953,6 +1141,12 @@ export class RdpFileTransferProvider { dispose(): void { this.disposed = true; + // Stop the paste-ack watchdog and resume monitoring if an upload was still + // mid-paste-window (we defer the resume, so it may not have fired yet). + this.clearPasteAckWatchdog(); + this.clearUploadInactivityWatchdog(); + this.resumeUploadMonitoring(); + // Cancel active downloads (lock cleanup is handled by the Rust layer) for (const state of this.activeDownloads.values()) { state.chunks = []; @@ -988,6 +1182,16 @@ export class RdpFileTransferProvider { // ==================== Callback Handlers ==================== private handleFilesAvailable(files: FileInfo[], clipDataId?: number): void { + // A remote FormatList means the remote took ownership of the clipboard, which + // supersedes any in-flight upload advertise of ours: the remote will not pull our + // files, and once the paste has been acknowledged the paste-ack watchdog is + // already disarmed, so nothing else would ever release `uploadState`. Left + // lingering it wedges every later upload with "Upload already in progress". Release + // it here. This is the symmetric counterpart of handleFormatListResponse(false): + // that recovers when the remote *rejects* our advertise; this recovers when the + // remote *replaces* it. No-op when no upload is in flight. + this.failPendingUpload('Upload interrupted: the remote clipboard changed'); + // Do NOT cancel active downloads here. // // Per MS-RDPECLIP 2.2.4.1 and 3.1.5.3.2, clipboard locks ensure that @@ -1060,6 +1264,10 @@ export class RdpFileTransferProvider { } private handleFileContentsRequest(request: FileContentsRequest): void { + // The remote is pulling the files, so the paste was accepted: end the + // clobber-protection window (disarm the watchdog, resume monitoring). + this.acknowledgePaste(); + if (!this.uploadState) { if (!this.retainedFiles) { console.warn('Received file contents request but no upload in progress'); @@ -1147,10 +1355,7 @@ export class RdpFileTransferProvider { this.uploadState.failedFiles.add(request.index); this.uploadState.completedFiles.add(request.index); if (this.uploadState.completedFiles.size >= this.uploadState.expectedFileCount) { - const { resolve, droppedFiles: completed } = this.uploadState; - this.retainedFiles = completed; - this.uploadState = undefined; - resolve(); + this.finishUploadBatch(this.uploadState); } } }, RdpFileTransferProvider.FILE_READER_TIMEOUT_MS); @@ -1199,12 +1404,10 @@ export class RdpFileTransferProvider { } if (this.uploadState.completedFiles.size === this.uploadState.expectedFileCount) { - // All files uploaded successfully. Retain DroppedFile - // metadata so re-paste from the remote can serve data again. - const { resolve, droppedFiles: completed } = this.uploadState; - this.retainedFiles = completed; - this.uploadState = undefined; - resolve(); + // All files uploaded successfully. finishUploadBatch retains the + // DroppedFile metadata so a re-paste from the remote can serve the + // data again, and stops the inactivity watchdog. + this.finishUploadBatch(this.uploadState); } } } @@ -1243,10 +1446,7 @@ export class RdpFileTransferProvider { this.uploadState.failedFiles.add(request.index); this.uploadState.completedFiles.add(request.index); if (this.uploadState.completedFiles.size >= this.uploadState.expectedFileCount) { - const { resolve, droppedFiles: completed } = this.uploadState; - this.retainedFiles = completed; - this.uploadState = undefined; - resolve(); + this.finishUploadBatch(this.uploadState); } } }; @@ -1409,6 +1609,25 @@ export class RdpFileTransferProvider { } } + /** + * Handle the remote's response to one of our outbound Format Lists, surfaced via + * `on_format_list_response`. Always re-emitted as a `format-list-response` event so + * a frontend can drive paste from it (inject on accept, retry on reject). + * + * On reject (`ok === false`) the remote silently discards the advertised clipboard + * (MS-RDPECLIP), so an upload still waiting to be pulled can never complete -- + * previously this left `uploadState` set and every later upload threw "Upload + * already in progress". The watchdog-armed check scopes the release to an upload + * that was advertised but not yet pulled, so re-paste and an already-progressing + * transfer are left alone. + */ + private handleFormatListResponse(ok: boolean): void { + this.emit('format-list-response', ok); + if (!ok && this.pasteAckTimeout !== undefined) { + this.failPendingUpload('The remote rejected the file list, so the paste was not accepted'); + } + } + private handleLock(_dataId: number): void { // Remote locked their clipboard (informational only for uploads). } diff --git a/web-client/iron-remote-desktop-rdp/src/extensions.ts b/web-client/iron-remote-desktop-rdp/src/extensions.ts index 5eafe364a..8881a7646 100644 --- a/web-client/iron-remote-desktop-rdp/src/extensions.ts +++ b/web-client/iron-remote-desktop-rdp/src/extensions.ts @@ -45,6 +45,10 @@ export function locksExpiredCallback(cb: (clipDataIds: Uint32Array) => void): Ex return new Extension('locks_expired_callback', cb as unknown); } +export function formatListResponseCallback(cb: (ok: boolean) => void): Extension { + return new Extension('format_list_response_callback', cb as unknown); +} + // Virtual printer (RDPDR) extensions // // Registering `printJobStreamCallbacks` activates the browser-side virtual