diff --git a/libs/clipboard/src/platform/unix/serv_files.rs b/libs/clipboard/src/platform/unix/serv_files.rs index ff7503a33..3e93fac97 100644 --- a/libs/clipboard/src/platform/unix/serv_files.rs +++ b/libs/clipboard/src/platform/unix/serv_files.rs @@ -5,9 +5,16 @@ use hbb_common::{ log, }; use parking_lot::Mutex; -use std::{collections::VecDeque, path::PathBuf, sync::Arc, time::SystemTime, usize}; +use std::{ + collections::VecDeque, + path::PathBuf, + sync::{atomic::Ordering, Arc}, + time::SystemTime, + usize, +}; -// Our clients request at most 4 MiB per range. A larger read is refused, not shortened: +// Our clients request at most this much per range: macOS 4 MiB, FUSE less, and Windows +// splits larger reads (WF_CLIPRDR_MAX_RANGE_READ). A larger read is refused, not shortened: // a short response reads as end of file to IStream callers on Windows. const MAX_RANGE_READ: u64 = 16 * 1024 * 1024; @@ -119,6 +126,14 @@ impl ClipFiles { Ok(()) } + // Close open files and their read buffers; `read_exact_at` reopens on the next read. + fn release_handles(&mut self) { + for file in self.file_list.iter_mut() { + file.offset.store(0, Ordering::Relaxed); + file.handle = None; + } + } + fn get_files_for_audit(&self, request: &FileContentsRequest) -> Option { if let FileContentsRequest::Range { file_idx, offset, .. @@ -302,15 +317,21 @@ fn select_clip_files<'a>( } // Keep a list a peer was sent, so its streams do not read the new copy at the same indexes. -fn retire_if_served(clip_files: &mut ClipFiles) { - if clip_files.served_to.is_empty() { +fn retire_if_served(current: &mut ClipFiles, mut replaced: ClipFiles) { + if replaced.served_to.is_empty() { return; } + if replaced.id == current.id { + // Same descriptors: its peers read the new list at the same indexes. + current.served_to = replaced.served_to; + return; + } + replaced.release_handles(); let mut retired = RETIRED_CLIP_FILES.lock(); if retired.len() == MAX_RETIRED_CLIP_FILES { retired.pop_front(); } - retired.push_back(std::mem::take(clip_files)); + retired.push_back(replaced); } pub fn read_file_contents( @@ -377,9 +398,13 @@ pub fn sync_files(files: &[String]) -> Result<(), CliprdrError> { { return Ok(()); } - retire_if_served(&mut files_lock); - files_lock.sync_files(files, current)?; - files_lock.build_file_list_pdu() + // Build aside, so a failure leaves the current list in place. + let mut next = ClipFiles::default(); + next.sync_files(files, current)?; + next.build_file_list_pdu()?; + let replaced = std::mem::replace(&mut *files_lock, next); + retire_if_served(&mut files_lock, replaced); + Ok(()) } pub fn get_file_list_pdu(conn_id: i32) -> Vec { @@ -680,4 +705,76 @@ mod sig_test { clear_files(); } + + #[test] + fn failed_recopy_keeps_the_current_list() { + let tmp = TmpDir::new("failed_recopy"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAAAAAA").unwrap(); + let missing = tmp.join("missing.bin"); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + let pdu = get_file_list_pdu(1); + + assert!(sync_files(&[path_str(&missing)]).is_err()); + assert_eq!(get_file_list_pdu(1), pdu); + + clear_files(); + } + + #[test] + fn identical_recopy_keeps_its_peers_on_the_current_list() { + let tmp = TmpDir::new("same_recopy"); + let dir = tmp.join("dir"); + fs::create_dir_all(&dir).unwrap(); + fs::write(dir.join("file.bin"), b"AAAAAAAA").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&dir)]).unwrap(); + let id = file_list_id(&get_file_list_pdu(1)); + let idx = CLIP_FILES.lock().first_file_index as i32; + + // A selection with a directory is rebuilt on every copy; an unchanged rebuild must + // not use up the slots that lists still being read depend on. + for _ in 0..=MAX_RETIRED_CLIP_FILES { + sync_files(&[path_str(&dir)]).unwrap(); + } + assert!(RETIRED_CLIP_FILES.lock().is_empty()); + let read = |id| range_data(read_file_contents(1, 7, idx, 0x2, 0, 0, 4, id)); + assert_eq!(read(Some(id)), b"AAAA"); + assert_eq!(read(None), b"AAAA"); + + clear_files(); + } + + #[test] + fn retired_list_closes_its_files_and_reads_on() { + let tmp = TmpDir::new("retired_handles"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAABBBB").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"CCCCCCCC").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + let id = file_list_id(&get_file_list_pdu(1)); + let idx = CLIP_FILES.lock().first_file_index as i32; + let read = |offset| range_data(read_file_contents(1, 7, idx, 0x2, offset, 0, 4, Some(id))); + // Leaves the file open at offset 4. + assert_eq!(read(0), b"AAAA"); + + sync_files(&[path_str(&second)]).unwrap(); + assert!(RETIRED_CLIP_FILES.lock()[0] + .file_list + .iter() + .all(|file| file.handle.is_none())); + // The file reopens at its start, so continuing at offset 4 must seek. + assert_eq!(read(4), b"BBBB"); + + clear_files(); + } } diff --git a/libs/clipboard/src/windows/wf_cliprdr.c b/libs/clipboard/src/windows/wf_cliprdr.c index daf05f3f6..30b1a2226 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -57,6 +57,9 @@ /* File lists kept after they were sent, so a transfer still in flight keeps reading its own * files after a later copy or paste replaces them. */ #define WF_CLIPRDR_SERVED_FILE_LISTS 4u +/* Largest range the unix file server serves in one request (MAX_RANGE_READ in + * serv_files.rs); larger IStream reads are split into requests of this size. */ +#define WF_CLIPRDR_MAX_RANGE_READ (16u * 1024u * 1024u) static const WCHAR WF_CLIPRDR_SUPERSCRIPT_DIGITS[] = L"\x00B9\x00B2\x00B3"; static const WCHAR WF_CLIPRDR_INVALID_FILE_NAME_CHARS[] = L"<>:\"|?*"; @@ -398,8 +401,9 @@ typedef struct _CliprdrDataObject CliprdrDataObject; struct wf_served_file_list { - UINT32 connID; UINT32 id; + UINT32 *connIDs; // connections the list was sent to + size_t nConnIDs; size_t nFiles; size_t first_file_index; WCHAR **file_names; @@ -498,6 +502,7 @@ static HRESULT CliprdrEnumFORMATETC_New(ULONG nFormats, FORMATETC *pFormatEtc, static void CliprdrEnumFORMATETC_Delete(CliprdrEnumFORMATETC *instance); static void CliprdrStream_Delete(CliprdrStream *instance); +static HRESULT wf_cliprdr_stream_read_split(IStream *This, BYTE *pv, ULONG cb, ULONG *pcbRead); static BOOL try_open_clipboard(HWND hwnd) { @@ -584,6 +589,9 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO if (instance->m_lOffset.QuadPart >= instance->m_lSize.QuadPart) return S_FALSE; + if (cb > WF_CLIPRDR_MAX_RANGE_READ) + return wf_cliprdr_stream_read_split(This, (BYTE *)pv, cb, pcbRead); + ret = cliprdr_send_request_filecontents(clipboard, instance->m_connID, instance->m_streamId, instance->m_clipDataId, instance->m_lIndex, FILECONTENTS_RANGE, instance->m_lOffset.HighPart, @@ -626,6 +634,26 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO return S_OK; } +static HRESULT wf_cliprdr_stream_read_split(IStream *This, BYTE *pv, ULONG cb, ULONG *pcbRead) +{ + HRESULT hr = S_OK; + ULONG total = 0; + ULONG chunk; + ULONG n; + + while (total < cb && hr == S_OK) + { + chunk = cb - total < WF_CLIPRDR_MAX_RANGE_READ ? cb - total : WF_CLIPRDR_MAX_RANGE_READ; + n = 0; + hr = CliprdrStream_Read(This, pv + total, chunk, &n); + total += n; + } + *pcbRead = total; + if (FAILED(hr)) + return hr; + return total < cb ? S_FALSE : S_OK; +} + static HRESULT STDMETHODCALLTYPE CliprdrStream_Write(IStream *This, const void *pv, ULONG cb, ULONG *pcbWritten) { @@ -2475,9 +2503,22 @@ static void wf_cliprdr_free_served_file_list(wfServedFileList *list) free(list->file_names); } free(list->file_sizes); + free(list->connIDs); ZeroMemory(list, sizeof(*list)); } +static BOOL wf_cliprdr_served_to(const wfServedFileList *list, UINT32 connID) +{ + size_t i; + + for (i = 0; i < list->nConnIDs; i++) + { + if (list->connIDs[i] == connID) + return TRUE; + } + return FALSE; +} + static void wf_cliprdr_forget_served_file_lists(wfClipboard *clipboard) { size_t i; @@ -2487,46 +2528,67 @@ static void wf_cliprdr_forget_served_file_lists(wfClipboard *clipboard) clipboard->next_served_file_list = 0; } -static void wf_cliprdr_remember_served_file_list(wfClipboard *clipboard, UINT32 connID) +static BOOL wf_cliprdr_copy_file_list(wfClipboard *clipboard, wfServedFileList *list) { - wfServedFileList *list; size_t i; - for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) - { - list = &clipboard->served_file_lists[i]; - if (list->file_names && list->connID == connID && list->id == clipboard->file_list_id) - return; - } - - list = &clipboard->served_file_lists[clipboard->next_served_file_list]; - clipboard->next_served_file_list = - (clipboard->next_served_file_list + 1) % WF_CLIPRDR_SERVED_FILE_LISTS; - wf_cliprdr_free_served_file_list(list); - list->file_names = (WCHAR **)calloc(clipboard->nFiles, sizeof(WCHAR *)); list->file_sizes = (UINT64 *)calloc(clipboard->nFiles, sizeof(UINT64)); if (!list->file_names || !list->file_sizes) - { - wf_cliprdr_free_served_file_list(list); - return; - } + return FALSE; list->nFiles = clipboard->nFiles; for (i = 0; i < clipboard->nFiles; i++) { if (!clipboard->file_names[i] || !(list->file_names[i] = _wcsdup(clipboard->file_names[i]))) - { - wf_cliprdr_free_served_file_list(list); - return; - } + return FALSE; if (clipboard->fileDescriptor[i]) list->file_sizes[i] = ((UINT64)clipboard->fileDescriptor[i]->nFileSizeHigh << 32) | clipboard->fileDescriptor[i]->nFileSizeLow; } - list->connID = connID; list->id = clipboard->file_list_id; list->first_file_index = clipboard->first_file_index; + return TRUE; +} + +/* One slot per list, shared by every connection it was sent to, so pastes of the same + * list do not push out a list another transfer is still reading. */ +static void wf_cliprdr_remember_served_file_list(wfClipboard *clipboard, UINT32 connID) +{ + wfServedFileList *list = NULL; + UINT32 *connIDs; + size_t i; + + for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) + { + if (clipboard->served_file_lists[i].file_names && + clipboard->served_file_lists[i].id == clipboard->file_list_id) + { + list = &clipboard->served_file_lists[i]; + break; + } + } + + if (!list) + { + list = &clipboard->served_file_lists[clipboard->next_served_file_list]; + clipboard->next_served_file_list = + (clipboard->next_served_file_list + 1) % WF_CLIPRDR_SERVED_FILE_LISTS; + wf_cliprdr_free_served_file_list(list); + if (!wf_cliprdr_copy_file_list(clipboard, list)) + { + wf_cliprdr_free_served_file_list(list); + return; + } + } + + if (wf_cliprdr_served_to(list, connID)) + return; + connIDs = (UINT32 *)realloc(list->connIDs, (list->nConnIDs + 1) * sizeof(UINT32)); + if (!connIDs) + return; + connIDs[list->nConnIDs++] = connID; + list->connIDs = connIDs; } /* Serves a request whose stream came from an earlier file list sent to the same connection. @@ -2541,8 +2603,8 @@ static BOOL wf_cliprdr_read_served_file_list(wfClipboard *clipboard, for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) { if (clipboard->served_file_lists[i].file_names && - clipboard->served_file_lists[i].connID == request->connID && - clipboard->served_file_lists[i].id == request->clipDataId) + clipboard->served_file_lists[i].id == request->clipDataId && + wf_cliprdr_served_to(&clipboard->served_file_lists[i], request->connID)) { list = &clipboard->served_file_lists[i]; break; @@ -3564,6 +3626,17 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context, goto exit; } + if (fileContentsRequest->haveClipDataId && + fileContentsRequest->clipDataId != clipboard->file_list_id) + { + // Served by path from an earlier list; the clipboard is not needed. + ZeroMemory(&vStgMedium, sizeof(STGMEDIUM)); // read at exit + if (wf_cliprdr_read_served_file_list(clipboard, fileContentsRequest, pData, cbRequested, + &uSize)) + rc = CHANNEL_RC_OK; + goto exit; + } + hRet = OleGetClipboard(&pDataObj); if (FAILED(hRet)) @@ -3581,15 +3654,6 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context, vFormatEtc.lindex = fileContentsRequest->listIndex; vFormatEtc.ptd = NULL; - if (fileContentsRequest->haveClipDataId && - fileContentsRequest->clipDataId != clipboard->file_list_id) - { - if (wf_cliprdr_read_served_file_list(clipboard, fileContentsRequest, pData, cbRequested, - &uSize)) - rc = CHANNEL_RC_OK; - goto exit; - } - if (GetClipboardSequenceNumber() != clipboard->file_list_seq) { // The clipboard changed after the peer got its file list. The live FileContents