diff --git a/AGENTS.md b/AGENTS.md index bd6e42075..00d130cd1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,6 +48,20 @@ workspace member. `base::config::keys` re-exports the handful of keys * Do not add dependencies unless needed. * Keep code simple and idiomatic. +### Logging + +* `debug` and above are written to the log file. A log call that can fire + repeatedly (per packet, frame, input event, or loop iteration, or at a rate a + peer controls) must not use `debug` or higher unthrottled. +* For such a site, pick one: + + * `log::trace!` when the event is expected and the line only helps while + actively debugging; + * `hbb_common::throttled_log!(interval, level, ...)` when it signals a fault + that should still show up in a user's log. It keeps one line per interval + with a count of the rest. Use `hbb_common::log_throttle::LogThrottle` + directly only when the decision drives more than one log call. + ## Tokio Rules * Assume a Tokio runtime already exists. @@ -127,6 +141,15 @@ Before considering any implementation complete, perform a minimization pass over * Before finalizing, explicitly report the regression surface: list the existing files and existing runtime paths whose behavior changed, and explain why each change is unavoidable. * During review, treat an unnecessarily modified legacy path as a review finding even if tests pass and the rewritten behavior appears equivalent. +### Corner cases raised in review + +A refactor added to cover a corner case rarely converges. Each new counter, timestamp, cache or eviction/expiry rule interacts with state that existing code relies on, and the next review round finds the problems it introduced. + +* A corner case is still worth fixing when the fix is easy and low-risk: a local change of a few lines that adds no state and changes no existing lookup, such as moving a check or refusing bad input earlier. +* When the only fix needs new state, a new lifecycle rule or a restructure, and the code already fails cleanly there or behaves as master does, document it as a known limit in the PR instead. Anything beyond the easy fix needs the maintainer's explicit go-ahead first. +* Before adding state that reorders, expires or reuses existing data, list every lookup that reads that data and check each one still holds. +* Prefer a clean failure, where the operation reports an error, over machinery that tries to make a rare case succeed. + ## Reviewing a PR * Review only what the diff introduces. Verify ownership with `gh pr diff` before reporting a finding — if the offending lines are untouched context, it is a pre-existing problem, not this PR's. diff --git a/libs/clipboard/src/platform/unix/filetype.rs b/libs/clipboard/src/platform/unix/filetype.rs index 52d637bbe..db04ad9de 100644 --- a/libs/clipboard/src/platform/unix/filetype.rs +++ b/libs/clipboard/src/platform/unix/filetype.rs @@ -48,6 +48,19 @@ pub struct FileDescription { pub creation_time: SystemTime, pub size: u64, pub perm: u16, + /// `file_list_id()` of the list this file came from, sent back in file contents requests + #[serde(default)] + pub clip_data_id: i32, +} + +/// Identifies a file list by its descriptor PDU, which both peers hold, so file contents +/// requests can name the list they read from. FNV-1a, same as `wf_cliprdr_file_list_id()`. +pub fn file_list_id(file_descriptor_pdu: &[u8]) -> i32 { + file_descriptor_pdu + .iter() + .fold(0x811c_9dc5_u32, |id, byte| { + (id ^ *byte as u32).wrapping_mul(0x0100_0193) + }) as i32 } pub(super) fn validate_file_name(name: &str) -> Result<(), CliprdrError> { @@ -69,6 +82,7 @@ impl FileDescription { fn parse_file_descriptor( bytes: &mut Bytes, conn_id: i32, + clip_data_id: i32, ) -> Result { let flags = bytes.get_u32_le(); // skip reserved 32 bytes @@ -175,6 +189,7 @@ impl FileDescription { creation_time: last_modified, size, perm, + clip_data_id, }; Ok(desc) @@ -186,6 +201,7 @@ impl FileDescription { file_descriptor_pdu: Vec, conn_id: i32, ) -> Result, CliprdrError> { + let clip_data_id = file_list_id(&file_descriptor_pdu); let mut data = Bytes::from(file_descriptor_pdu); if data.remaining() < 4 { return Err(CliprdrError::InvalidRequest { @@ -206,7 +222,7 @@ impl FileDescription { let mut files = Vec::with_capacity(count); for _ in 0..count { - let desc = Self::parse_file_descriptor(&mut data, conn_id)?; + let desc = Self::parse_file_descriptor(&mut data, conn_id, clip_data_id)?; files.push(desc); } @@ -282,6 +298,22 @@ mod tests { assert_eq!(files[0].name, PathBuf::from("file.txt")); } + #[test] + fn file_list_id_is_fnv1a() { + // Reference vectors; the Windows side computes the same id in C. + assert_eq!(file_list_id(b"") as u32, 0x811c_9dc5); + assert_eq!(file_list_id(b"a") as u32, 0xe40c_292c); + assert_eq!(file_list_id(b"foobar") as u32, 0xbf9c_f968); + } + + #[test] + fn parsed_files_carry_their_list_id() { + let pdu = descriptor_pdu("file.txt"); + let id = file_list_id(&pdu); + let files = FileDescription::parse_file_descriptors(pdu, 0).unwrap(); + assert_eq!(files[0].clip_data_id, id); + } + #[test] fn rejects_non_terminated_file_name() { let name = "a".repeat(FILE_NAME_CODE_UNITS); diff --git a/libs/clipboard/src/platform/unix/fuse/cs.rs b/libs/clipboard/src/platform/unix/fuse/cs.rs index 307c2fb16..dc15c55e3 100644 --- a/libs/clipboard/src/platform/unix/fuse/cs.rs +++ b/libs/clipboard/src/platform/unix/fuse/cs.rs @@ -549,8 +549,8 @@ impl FuseServer { n_position_low, n_position_high, cb_requested, - have_clip_data_id: false, - clip_data_id: 0, + have_clip_data_id: true, + clip_data_id: node.clip_data_id, }; send_data(node.conn_id, request.clone()).map_err(|e| { @@ -611,6 +611,9 @@ struct FuseNode { /// connection id pub conn_id: i32, + /// id of the peer's file list this node came from + pub clip_data_id: i32, + /// file index in peer's file list /// NOTE: /// it is NOT the same as inode, this is the index in the file list @@ -634,6 +637,7 @@ impl FuseNode { pub fn from_description(inode: Inode, desc: FileDescription) -> Self { Self { conn_id: desc.conn_id, + clip_data_id: desc.clip_data_id, index: inode as usize - 2, name: desc .name @@ -650,6 +654,7 @@ impl FuseNode { pub fn new_root() -> Self { Self { conn_id: 0, + clip_data_id: 0, index: 0, name: String::from("/"), parent: None, @@ -898,6 +903,7 @@ mod fuse_test { size: 0, perm: 0, + clip_data_id: 0, } } fn generate_descriptions(prefix: &str) -> Vec { diff --git a/libs/clipboard/src/platform/unix/macos/paste_task.rs b/libs/clipboard/src/platform/unix/macos/paste_task.rs index 76e054643..d66c93ad2 100644 --- a/libs/clipboard/src/platform/unix/macos/paste_task.rs +++ b/libs/clipboard/src/platform/unix/macos/paste_task.rs @@ -617,6 +617,7 @@ impl PasteTaskHandle { }; let cb_requested = min(BLOCK_SIZE as u64, file.size - self.progress.offset); let conn_id = file.conn_id; + let clip_data_id = file.clip_data_id; let (n_position_high, n_position_low) = ( (self.progress.offset >> 32) as i32, @@ -629,8 +630,8 @@ impl PasteTaskHandle { n_position_low, n_position_high, cb_requested: cb_requested as _, - have_clip_data_id: false, - clip_data_id: 0, + have_clip_data_id: true, + clip_data_id, }; allow_err!(send_data(conn_id, request)); self.progress.last_sent_time = Instant::now(); @@ -703,6 +704,7 @@ mod tests { creation_time: SystemTime::UNIX_EPOCH, size, perm: 0, + clip_data_id: 0, } } diff --git a/libs/clipboard/src/platform/unix/serv_files.rs b/libs/clipboard/src/platform/unix/serv_files.rs index 507de3ef3..f2cd46c26 100644 --- a/libs/clipboard/src/platform/unix/serv_files.rs +++ b/libs/clipboard/src/platform/unix/serv_files.rs @@ -1,11 +1,27 @@ -use super::local_file::LocalFile; -use crate::{platform::unix::local_file::construct_file_list, ClipboardFile, CliprdrError}; +use crate::{ + platform::unix::{ + filetype::file_list_id, + local_file::{construct_file_list, LocalFile}, + }, + ClipboardFile, CliprdrError, +}; use hbb_common::{ bytes::{BufMut, BytesMut}, log, }; use parking_lot::Mutex; -use std::{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 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; lazy_static::lazy_static! { // local files are cached, this value should not be changed when copying files @@ -13,8 +29,18 @@ lazy_static::lazy_static! { // We need to keep the file list in the same order as the remote side. // We may add a `FileId` field to `CliprdrFileContentsRequest` in the future. static ref CLIP_FILES: Arc> = Default::default(); + // Lists replaced by a newer copy after a peer was sent them, oldest first. The peer's + // streams keep reading the list they came from. Lock after `CLIP_FILES`. + static ref RETIRED_CLIP_FILES: Mutex> = Default::default(); } +const MAX_RETIRED_CLIP_FILES: usize = 4; + +// The first descriptor's `clsid`, inside the 32 reserved bytes every peer skips. A random +// nonce there makes each copy's list id distinct, even when two copies have identical +// names, sizes and times (the same file in two folders). +const LIST_NONCE: std::ops::Range = 8..24; + #[derive(Debug)] enum FileContentsRequest { Size { @@ -62,6 +88,10 @@ struct ClipFiles { file_list: Vec, first_file_index: usize, files_pdu: Vec, + // `file_list_id(files_pdu)`, which peers send back as `clip_data_id`. + id: i32, + // Connections that were sent `files_pdu`. + served_to: Vec, } impl ClipFiles { @@ -71,6 +101,8 @@ impl ClipFiles { self.file_list.clear(); self.first_file_index = usize::MAX; self.files_pdu.clear(); + self.id = 0; + self.served_to.clear(); } fn sync_files( @@ -100,9 +132,54 @@ impl ClipFiles { data.put(file.as_bin()?.as_slice()); } self.files_pdu = data.to_vec(); + if let Some(nonce) = self.files_pdu.get_mut(LIST_NONCE) { + nonce.copy_from_slice(&rand::random::<[u8; 16]>()); + } + self.id = file_list_id(&self.files_pdu); Ok(()) } + // Close files that are not being read, such as the preloaded next file; `read_exact_at` + // reopens them on the next read. A file part-way through keeps its handle and offset, so + // its remaining bytes come from the same file even if its path is replaced meanwhile. + fn release_handles(&mut self) { + for file in self.file_list.iter_mut() { + if file.offset.load(Ordering::Relaxed) == 0 { + file.handle = None; + } + } + } + + // A retired list's file is reopened by path once its handle was closed, and the path may now + // name a replacement. Check size and modified time on the handle the read will use, and refuse + // a file that no longer matches the list. + fn check_retired_file(&mut self, request: &FileContentsRequest) -> Result<(), CliprdrError> { + let FileContentsRequest::Range { file_idx, .. } = request else { + return Ok(()); + }; + let Some(file) = self.file_list.get_mut(*file_idx) else { + return Ok(()); + }; + if file.is_dir { + return Ok(()); + } + file.load_handle()?; + let unchanged = file + .handle + .as_ref() + .and_then(|handle| handle.get_ref().metadata().ok()) + .map_or(false, |md| { + md.len() == file.size && md.modified().ok() == Some(file.last_write_time) + }); + if unchanged { + Ok(()) + } else { + Err(CliprdrError::InvalidRequest { + description: format!("file {} changed since its list was sent", file.name), + }) + } + } + fn get_files_for_audit(&self, request: &FileContentsRequest) -> Option { if let FileContentsRequest::Range { file_idx, offset, .. @@ -213,12 +290,21 @@ impl ClipFiles { ), }); } - let read_size = if offset + length > file.size { + let read_size = if length > file.size - offset { file.size - offset } else { length }; + if read_size > MAX_RANGE_READ { + return Err(CliprdrError::InvalidRequest { + description: format!( + "file contents request of {} bytes exceeds the {} byte limit, conn: {}", + read_size, MAX_RANGE_READ, conn_id + ), + }); + } + let mut buf = vec![0u8; read_size as usize]; file.read_exact_at(&mut buf, offset)?; @@ -249,6 +335,44 @@ impl ClipFiles { #[inline] pub fn clear_files() { CLIP_FILES.lock().clear(); + RETIRED_CLIP_FILES.lock().clear(); +} + +// The list a request reads from: the one its `clip_data_id` names, else the one last sent to +// the connection. A named list that is gone yields `None`. A peer that sends no id and was +// never sent a list reads the current one, as before. +fn select_clip_files<'a>( + current: &'a mut ClipFiles, + retired: &'a mut VecDeque, + conn_id: i32, + clip_data_id: Option, +) -> Option<&'a mut ClipFiles> { + match clip_data_id { + Some(id) if current.id == id => Some(current), + Some(id) => retired + .iter_mut() + .rev() + .find(|files| files.id == id && files.served_to.contains(&conn_id)), + None if current.served_to.contains(&conn_id) => Some(current), + None => retired + .iter_mut() + .rev() + .find(|files| files.served_to.contains(&conn_id)) + .or(Some(current)), + } +} + +// Keep a list a peer was sent, so its streams do not read the new copy at the same indexes. +fn retire_if_served(mut replaced: ClipFiles) { + if replaced.served_to.is_empty() { + return; + } + replaced.release_handles(); + let mut retired = RETIRED_CLIP_FILES.lock(); + if retired.len() == MAX_RETIRED_CLIP_FILES { + retired.pop_front(); + } + retired.push_back(replaced); } pub fn read_file_contents( @@ -259,6 +383,7 @@ pub fn read_file_contents( n_position_low: i32, n_position_high: i32, cb_requested: i32, + clip_data_id: Option, ) -> Vec> { let fcr = if dw_flags == 0x1 { FileContentsRequest::Size { @@ -266,8 +391,11 @@ pub fn read_file_contents( file_idx: list_index as usize, } } else if dw_flags == 0x2 { - let offset = (n_position_high as u64) << 32 | n_position_low as u64; - let length = cb_requested as u64; + // nPositionLow and cbRequested are UINT32s carried in i32 fields. Sign-extending + // them rejects offsets whose low word is >= 2 GiB and turns a negative length + // into a near-u64::MAX read. + let offset = (n_position_high as u64) << 32 | n_position_low as u32 as u64; + let length = cb_requested as u32 as u64; FileContentsRequest::Range { stream_id, @@ -281,7 +409,23 @@ pub fn read_file_contents( })]; }; - let mut clip_files = CLIP_FILES.lock(); + let mut current = CLIP_FILES.lock(); + let mut retired = RETIRED_CLIP_FILES.lock(); + let current_ptr: *const ClipFiles = &*current; + let Some(clip_files) = select_clip_files(&mut current, &mut retired, conn_id, clip_data_id) + else { + return vec![Err(CliprdrError::InvalidRequest { + description: format!( + "file list {:?} is not available to conn: {}", + clip_data_id, conn_id + ), + })]; + }; + if !std::ptr::eq(&*clip_files, current_ptr) { + if let Err(e) = clip_files.check_retired_file(&fcr) { + return vec![Err(e)]; + } + } let mut res = vec![]; if let Some(files_res) = clip_files.get_files_for_audit(&fcr) { res.push(Ok(files_res)); @@ -301,12 +445,21 @@ pub fn sync_files(files: &[String]) -> Result<(), CliprdrError> { { return Ok(()); } - 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(replaced); + Ok(()) } -pub fn get_file_list_pdu() -> Vec { - CLIP_FILES.lock().files_pdu.clone() +pub fn get_file_list_pdu(conn_id: i32) -> Vec { + let mut clip_files = CLIP_FILES.lock(); + if !clip_files.files_pdu.is_empty() && !clip_files.served_to.contains(&conn_id) { + clip_files.served_to.push(conn_id); + } + clip_files.files_pdu.clone() } #[cfg(test)] @@ -343,6 +496,30 @@ mod sig_test { p.to_string_lossy().to_string() } + // Tests that drive the global CLIP_FILES must not interleave. + static CLIP_FILES_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + fn lock_clip_files() -> std::sync::MutexGuard<'static, ()> { + CLIP_FILES_TEST_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + } + + // Sparse, so a multi-GiB file costs no disk space. + fn write_at(path: &PathBuf, offset: u64, data: &[u8]) { + use std::io::{Seek, SeekFrom, Write}; + let mut f = fs::File::create(path).unwrap(); + f.seek(SeekFrom::Start(offset)).unwrap(); + f.write_all(data).unwrap(); + } + + fn range_data(res: Vec>) -> Vec { + match res.into_iter().last() { + Some(Ok(ClipboardFile::FileContentsResponse { requested_data, .. })) => requested_data, + other => panic!("unexpected response: {:?}", other), + } + } + #[test] fn fingerprint_missing_path_is_default() { let tmp = TmpDir::new("missing"); @@ -393,7 +570,8 @@ mod sig_test { let files = vec![path_str(&file)]; // Drive the public, guarded `sync_files` over the global CLIP_FILES; - // reset first (this is the only test that touches the global). + // reset first. + let _guard = lock_clip_files(); clear_files(); sync_files(&files).unwrap(); @@ -415,4 +593,329 @@ mod sig_test { clear_files(); // leave the global clean for other tests } + + #[test] + fn range_request_huge_length_is_clamped() { + let tmp = TmpDir::new("range"); + let file = tmp.join("data.bin"); + fs::write(&file, b"0123456789").unwrap(); + let files = vec![path_str(&file)]; + + let mut clip = ClipFiles::default(); + clip.sync_files(&files, fingerprint(&files)).unwrap(); + let file_idx = clip.first_file_index; + + // offset + length used to wrap to 0, skip the clamp and ask for a u64::MAX buffer. + let resp = clip + .serve_file_contents( + 0, + FileContentsRequest::Range { + stream_id: 0, + file_idx, + offset: 1, + length: u64::MAX, + }, + ) + .unwrap(); + match resp { + ClipboardFile::FileContentsResponse { requested_data, .. } => { + assert_eq!(requested_data, b"123456789"); + } + _ => panic!("unexpected response"), + } + } + + #[test] + fn range_request_decodes_wire_fields_as_u32() { + let tmp = TmpDir::new("decode"); + let small = tmp.join("small.bin"); + fs::write(&small, b"0123456789").unwrap(); + let large = tmp.join("large.bin"); + write_at(&large, 0x8000_0000, b"tail"); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&small), path_str(&large)]).unwrap(); + let small_idx = CLIP_FILES.lock().first_file_index as i32; + let large_idx = small_idx + 1; + + // A negative cbRequested is a UINT32 length, clamped to the rest of the file. + let res = read_file_contents(0, 0, small_idx, 0x2, 1, 0, -1, None); + assert_eq!(range_data(res), b"123456789"); + + // An nPositionLow with bit 31 set is an offset past 2 GiB, not a huge one. + let res = read_file_contents(0, 0, large_idx, 0x2, 0x8000_0000u32 as i32, 0, 4, None); + assert_eq!(range_data(res), b"tail"); + + clear_files(); + } + + #[test] + fn range_request_over_limit_is_rejected() { + let tmp = TmpDir::new("limit"); + let file = tmp.join("big.bin"); + let size = MAX_RANGE_READ + 1; + write_at(&file, size - 1, b"x"); + let files = vec![path_str(&file)]; + + let mut clip = ClipFiles::default(); + clip.sync_files(&files, fingerprint(&files)).unwrap(); + let file_idx = clip.first_file_index; + + let res = clip.serve_file_contents( + 0, + FileContentsRequest::Range { + stream_id: 0, + file_idx, + offset: 0, + length: u32::MAX as u64, + }, + ); + assert!(matches!(res, Err(CliprdrError::InvalidRequest { .. }))); + } + + #[test] + fn host_copy_during_transfer_keeps_serving_the_sent_list() { + let tmp = TmpDir::new("recopy_in_flight"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAAAAAA").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"BBBBBBBB").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + 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, None)); + assert_eq!(read(0), b"AAAA"); + + // Copying another file on this side must not redirect the peer's stream. + sync_files(&[path_str(&second)]).unwrap(); + assert_eq!(read(4), b"AAAA"); + + clear_files(); + } + + #[test] + fn stream_keeps_its_list_after_a_second_paste() { + let tmp = TmpDir::new("repaste_in_flight"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAAAAAA").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"BBBBBBBB").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + let first_id = file_list_id(&get_file_list_pdu(1)); + let idx = CLIP_FILES.lock().first_file_index as i32; + + // The peer copies the second file on this side and pastes it while the first + // transfer is still reading. + sync_files(&[path_str(&second)]).unwrap(); + let second_id = file_list_id(&get_file_list_pdu(1)); + assert_ne!(first_id, second_id); + + let read = |id| range_data(read_file_contents(1, 7, idx, 0x2, 4, 0, 4, Some(id))); + assert_eq!(read(first_id), b"AAAA"); + assert_eq!(read(second_id), b"BBBB"); + + clear_files(); + } + + #[test] + fn named_list_must_have_been_sent_to_the_conn() { + let tmp = TmpDir::new("foreign_list"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAAAAAA").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"BBBBBBBB").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + let first_id = file_list_id(&get_file_list_pdu(1)); + let idx = CLIP_FILES.lock().first_file_index as i32; + sync_files(&[path_str(&second)]).unwrap(); + + let refused = |conn_id, id| { + matches!( + read_file_contents(conn_id, 7, idx, 0x2, 0, 0, 4, Some(id)).last(), + Some(Err(CliprdrError::InvalidRequest { .. })) + ) + }; + // Retired list, asked for by a connection it was never sent to. + assert!(refused(2, first_id)); + // A list id this side never sent. + assert!(refused(1, first_id ^ 1)); + + 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(); + } + + fn file_index(path: &PathBuf) -> i32 { + let files = CLIP_FILES.lock(); + files + .file_list + .iter() + .position(|f| &f.path == path) + .unwrap() as i32 + } + + // Replaces `path` the way editors save: write aside, then rename over it. + fn replace_atomically(path: &PathBuf, data: &[u8]) { + let aside = path.with_extension("new"); + fs::write(&aside, data).unwrap(); + fs::rename(&aside, path).unwrap(); + } + + #[test] + fn retired_list_closes_only_files_not_being_read() { + let tmp = TmpDir::new("retired_handles"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAABBBB").unwrap(); + let next = tmp.join("next.bin"); + fs::write(&next, b"CCCCDDDD").unwrap(); + let other = tmp.join("other.bin"); + fs::write(&other, b"EEEEEEEE").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first), path_str(&next)]).unwrap(); + let id = file_list_id(&get_file_list_pdu(1)); + let (first_idx, next_idx) = (file_index(&first), file_index(&next)); + let read = + |idx, offset| range_data(read_file_contents(1, 7, idx, 0x2, offset, 0, 4, Some(id))); + // Leaves `first` open at offset 4, and preloads `next`. + assert_eq!(read(first_idx, 0), b"AAAA"); + + sync_files(&[path_str(&other)]).unwrap(); + { + let retired = RETIRED_CLIP_FILES.lock(); + let open = |idx: i32| retired[0].file_list[idx as usize].handle.is_some(); + assert!(open(first_idx)); + assert!(!open(next_idx)); + } + assert_eq!(read(first_idx, 4), b"BBBB"); + assert_eq!(read(next_idx, 0), b"CCCC"); + + clear_files(); + } + + #[test] + fn files_being_read_keep_their_content_when_their_paths_are_replaced() { + let tmp = TmpDir::new("replaced_paths"); + let first = tmp.join("first.bin"); + fs::write(&first, b"1111aaaa").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"2222bbbb").unwrap(); + let other = tmp.join("other.bin"); + fs::write(&other, b"EEEEEEEE").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first), path_str(&second)]).unwrap(); + let id = file_list_id(&get_file_list_pdu(1)); + let (first_idx, second_idx) = (file_index(&first), file_index(&second)); + let read = + |idx, offset| range_data(read_file_contents(1, 7, idx, 0x2, offset, 0, 4, Some(id))); + // Two files part-way through at once. + assert_eq!(read(first_idx, 0), b"1111"); + assert_eq!(read(second_idx, 0), b"2222"); + + sync_files(&[path_str(&other)]).unwrap(); + replace_atomically(&first, b"3333cccc"); + replace_atomically(&second, b"4444dddd"); + + assert_eq!(read(first_idx, 4), b"aaaa"); + assert_eq!(read(second_idx, 4), b"bbbb"); + + clear_files(); + } + + #[test] + fn same_file_metadata_in_another_folder_is_another_list() { + let tmp = TmpDir::new("same_metadata"); + let (dir_a, dir_b) = (tmp.join("a"), tmp.join("b")); + fs::create_dir_all(&dir_a).unwrap(); + fs::create_dir_all(&dir_b).unwrap(); + let (first, second) = (dir_a.join("report.bin"), dir_b.join("report.bin")); + fs::write(&first, b"AAAAAAAA").unwrap(); + fs::write(&second, b"BBBBBBBB").unwrap(); + // Same name, size and mtime: the descriptors differ only by the nonce. + let mtime = SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_700_000_000); + for path in [&first, &second] { + fs::File::options() + .write(true) + .open(path) + .unwrap() + .set_modified(mtime) + .unwrap(); + } + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first)]).unwrap(); + let first_id = file_list_id(&get_file_list_pdu(1)); + let idx = CLIP_FILES.lock().first_file_index as i32; + sync_files(&[path_str(&second)]).unwrap(); + let second_id = file_list_id(&get_file_list_pdu(1)); + assert_ne!(first_id, second_id); + + let read = |id| range_data(read_file_contents(1, 7, idx, 0x2, 4, 0, 4, Some(id))); + assert_eq!(read(first_id), b"AAAA"); + assert_eq!(read(second_id), b"BBBB"); + + clear_files(); + } + + #[test] + fn a_retired_list_refuses_a_file_replaced_before_its_read() { + let tmp = TmpDir::new("replaced_unread"); + let first = tmp.join("first.bin"); + fs::write(&first, b"AAAAAAAA").unwrap(); + let second = tmp.join("second.bin"); + fs::write(&second, b"BBBBBBBB").unwrap(); + let other = tmp.join("other.bin"); + fs::write(&other, b"EEEEEEEE").unwrap(); + + let _guard = lock_clip_files(); + clear_files(); + sync_files(&[path_str(&first), path_str(&second)]).unwrap(); + let id = file_list_id(&get_file_list_pdu(1)); + let (first_idx, second_idx) = (file_index(&first), file_index(&second)); + let read = |idx, offset| read_file_contents(1, 7, idx, 0x2, offset, 0, 4, Some(id)); + assert_eq!(range_data(read(first_idx, 0)), b"AAAA"); + + // Retiring closes `second`, which has not been read yet; it is then replaced. + sync_files(&[path_str(&other)]).unwrap(); + replace_atomically(&second, b"CCCCCCCCCC"); + + assert!(matches!( + read(second_idx, 0).last(), + Some(Err(CliprdrError::InvalidRequest { .. })) + )); + // The file part-way through is unaffected. + assert_eq!(range_data(read(first_idx, 4)), b"AAAA"); + + clear_files(); + } } diff --git a/libs/clipboard/src/windows/wf_cliprdr.c b/libs/clipboard/src/windows/wf_cliprdr.c index c32a20259..1e3bcbb13 100644 --- a/libs/clipboard/src/windows/wf_cliprdr.c +++ b/libs/clipboard/src/windows/wf_cliprdr.c @@ -54,6 +54,12 @@ /* File clipboard redirection always advertises the descriptor and contents formats. */ #define WF_CLIPRDR_FILE_FORMAT_COUNT 2u #define WF_CLIPRDR_COM_LPT_PREFIX_LENGTH 3u +/* 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 served in one request, here and by the unix file server (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"<>:\"|?*"; @@ -213,6 +219,21 @@ static BOOL wf_cliprdr_bounded_strlen(const char *value, size_t max_len, size_t return FALSE; } +/* Identifies a file list by its FILEGROUPDESCRIPTORW bytes, which both peers hold, and is + * carried as FileContentsRequest.clipDataId. FNV-1a, same as `file_list_id()` on unix. */ +static UINT32 wf_cliprdr_file_list_id(const BYTE *data, SIZE_T size) +{ + UINT32 id = 2166136261u; + SIZE_T i; + + for (i = 0; i < size; i++) + { + id ^= data[i]; + id *= 16777619u; + } + return id; +} + /** * Clipboard Formats */ @@ -358,6 +379,7 @@ struct _CliprdrStream void *m_pData; UINT32 m_connID; UINT32 m_streamId; // unique CLIPRDR streamId; avoids leaking a heap pointer + UINT32 m_clipDataId; // id of the file list this stream was created from }; typedef struct _CliprdrStream CliprdrStream; @@ -377,6 +399,21 @@ struct _CliprdrDataObject }; typedef struct _CliprdrDataObject CliprdrDataObject; +struct wf_served_file_list +{ + UINT32 id; + UINT32 *connIDs; // connections the list was sent to + size_t nConnIDs; + UINT64 last_served; // served_file_list_seq when last sent; the lowest is evicted first + size_t nFiles; + size_t first_file_index; + WCHAR **file_names; + UINT64 *file_sizes; + BYTE *descriptors; // the FILEGROUPDESCRIPTORW sent, nonce included + SIZE_T descriptorsSize; +}; +typedef struct wf_served_file_list wfServedFileList; + struct wf_clipboard { // wfContext* wfc; @@ -424,6 +461,10 @@ struct wf_clipboard WCHAR **file_names; size_t first_file_index; FILEDESCRIPTORW **fileDescriptor; + DWORD file_list_seq; // clipboard sequence number the file list was read at + UINT32 file_list_id; // wf_cliprdr_file_list_id() of the file list, 0 while none is valid + wfServedFileList served_file_lists[WF_CLIPRDR_SERVED_FILE_LISTS]; + UINT64 served_file_list_seq; BOOL legacyApi; HMODULE hUser32; @@ -451,8 +492,8 @@ static UINT cliprdr_send_data_request(UINT32 connID, wfClipboard *clipboard, UIN static UINT cliprdr_send_lock(wfClipboard *clipboard); static UINT cliprdr_send_unlock(wfClipboard *clipboard); static UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, UINT32 streamId, - ULONG index, UINT32 flag, DWORD positionhigh, - DWORD positionlow, ULONG request); + UINT32 clipDataId, ULONG index, UINT32 flag, + DWORD positionhigh, DWORD positionlow, ULONG request); static BOOL is_file_descriptor_from_remote(); static BOOL is_set_by_instance(wfClipboard *clipboard); @@ -464,6 +505,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) { @@ -550,7 +592,11 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Read(IStream *This, void *pv, ULO if (instance->m_lOffset.QuadPart >= instance->m_lSize.QuadPart) return S_FALSE; - ret = cliprdr_send_request_filecontents(clipboard, instance->m_connID, instance->m_streamId, instance->m_lIndex, + 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, instance->m_lOffset.LowPart, cb); @@ -591,6 +637,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) { @@ -736,7 +802,8 @@ static HRESULT STDMETHODCALLTYPE CliprdrStream_Clone(IStream *This, IStream **pp return E_NOTIMPL; } -static CliprdrStream *CliprdrStream_New(UINT32 connID, ULONG index, void *pData, const FILEDESCRIPTORW *dsc) +static CliprdrStream *CliprdrStream_New(UINT32 connID, ULONG index, void *pData, const FILEDESCRIPTORW *dsc, + UINT32 clipDataId) { IStream *iStream = NULL; BOOL success = FALSE; @@ -779,6 +846,7 @@ static CliprdrStream *CliprdrStream_New(UINT32 connID, ULONG index, void *pData, instance->m_lOffset.QuadPart = 0; instance->m_connID = connID; instance->m_streamId = (UINT32)InterlockedIncrement(&clipboard->req_f_stream_id_seq); + instance->m_clipDataId = clipDataId; if (instance->m_Dsc.dwFlags & FD_ATTRIBUTES) { @@ -790,8 +858,8 @@ static CliprdrStream *CliprdrStream_New(UINT32 connID, ULONG index, void *pData, { /* get content size of this stream */ if (cliprdr_send_request_filecontents(clipboard, instance->m_connID, instance->m_streamId, - instance->m_lIndex, FILECONTENTS_SIZE, 0, 0, - 8) == CHANNEL_RC_OK) + instance->m_clipDataId, instance->m_lIndex, + FILECONTENTS_SIZE, 0, 0, 8) == CHANNEL_RC_OK) { success = TRUE; } @@ -1007,6 +1075,7 @@ static HRESULT STDMETHODCALLTYPE CliprdrDataObject_GetData(IDataObject *This, FO IStream **streams = NULL; UINT stream_count = 0; SIZE_T hmem_size; + UINT32 clipDataId; // DWORD remote_format_id = get_remote_format_id(clipboard, instance->m_pFormatEtc[idx].cfFormat); // FIXME: origin code may be failed here??? if (cliprdr_send_data_request(instance->m_connID, clipboard, instance->m_pFormatEtc[idx].cfFormat) != 0) @@ -1053,10 +1122,11 @@ static HRESULT STDMETHODCALLTYPE CliprdrDataObject_GetData(IDataObject *This, FO return wf_cliprdr_fail_locked_file_descriptor_data( clipboard, pMedium, instance, NULL, 0, E_OUTOFMEMORY); + clipDataId = wf_cliprdr_file_list_id((const BYTE *)dsc, hmem_size); for (i = 0; i < stream_count; i++) { - streams[i] = - (IStream *)CliprdrStream_New(instance->m_connID, i, clipboard, &dsc->fgd[i]); + streams[i] = (IStream *)CliprdrStream_New(instance->m_connID, i, clipboard, + &dsc->fgd[i], clipDataId); if (!streams[i]) { return wf_cliprdr_fail_locked_file_descriptor_data( @@ -1996,9 +2066,9 @@ static UINT cliprdr_send_data_request(UINT32 connID, wfClipboard *clipboard, UIN return wait_response_event(connID, clipboard, clipboard->formatDataRespEvent, &clipboard->formatDataRespReceived, &clipboard->hmem); } -static UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, UINT32 streamId, ULONG index, - UINT32 flag, DWORD positionhigh, DWORD positionlow, - ULONG nreq) +static UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 connID, UINT32 streamId, + UINT32 clipDataId, ULONG index, UINT32 flag, + DWORD positionhigh, DWORD positionlow, ULONG nreq) { UINT rc; CLIPRDR_FILE_CONTENTS_REQUEST fileContentsRequest = { 0 }; @@ -2023,7 +2093,8 @@ static UINT cliprdr_send_request_filecontents(wfClipboard *clipboard, UINT32 con fileContentsRequest.nPositionLow = positionlow; fileContentsRequest.nPositionHigh = positionhigh; fileContentsRequest.cbRequested = nreq; - fileContentsRequest.clipDataId = 0; + fileContentsRequest.haveClipDataId = TRUE; + fileContentsRequest.clipDataId = clipDataId; fileContentsRequest.msgFlags = 0; rc = clipboard->context->ClientFileContentsRequest(clipboard->context, &fileContentsRequest); if (rc != ERROR_SUCCESS) @@ -2424,6 +2495,242 @@ error: return res; } +static void wf_cliprdr_free_served_file_list(wfServedFileList *list) +{ + size_t i; + + if (list->file_names) + { + for (i = 0; i < list->nFiles; i++) + free(list->file_names[i]); + free(list->file_names); + } + free(list->file_sizes); + free(list->connIDs); + free(list->descriptors); + 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; + + for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) + wf_cliprdr_free_served_file_list(&clipboard->served_file_lists[i]); +} + +/* Whether a kept list has the current file_names and the same descriptors as dsc, apart + * from the nonce in fgd[0].clsid. */ +static BOOL wf_cliprdr_is_same_file_list(wfClipboard *clipboard, const wfServedFileList *list, + const FILEGROUPDESCRIPTORW *dsc, SIZE_T size) +{ + SIZE_T nonce_start = offsetof(FILEGROUPDESCRIPTORW, fgd) + offsetof(FILEDESCRIPTORW, clsid); + SIZE_T nonce_end = nonce_start + sizeof(CLSID); + size_t i; + + if (!list->file_names || list->nFiles != clipboard->nFiles || + list->descriptorsSize != size || size < nonce_end) + return FALSE; + for (i = 0; i < list->nFiles; i++) + { + if (!clipboard->file_names[i] || wcscmp(list->file_names[i], clipboard->file_names[i]) != 0) + return FALSE; + } + return memcmp(list->descriptors, dsc, nonce_start) == 0 && + memcmp(list->descriptors + nonce_end, (const BYTE *)dsc + nonce_end, + size - nonce_end) == 0; +} + +/* Puts a nonce in fgd[0].clsid, which peers ignore without FD_CLSID. The list id hashes it, + * so each copy gets its own id even when two copies have identical names, sizes and times + * (the same file in two folders). Re-sending unchanged files keeps their nonce, and id. */ +static void wf_cliprdr_stamp_file_list(wfClipboard *clipboard, FILEGROUPDESCRIPTORW *dsc, + SIZE_T size) +{ + size_t i; + + for (i = 0; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) + { + if (wf_cliprdr_is_same_file_list(clipboard, &clipboard->served_file_lists[i], dsc, size)) + { + dsc->fgd[0].clsid = + ((const FILEGROUPDESCRIPTORW *)clipboard->served_file_lists[i].descriptors) + ->fgd[0] + .clsid; + return; + } + } + if (FAILED(CoCreateGuid(&dsc->fgd[0].clsid))) + ZeroMemory(&dsc->fgd[0].clsid, sizeof(CLSID)); +} + +static BOOL wf_cliprdr_copy_file_list(wfClipboard *clipboard, wfServedFileList *list, + const FILEGROUPDESCRIPTORW *dsc, SIZE_T size) +{ + size_t i; + + list->file_names = (WCHAR **)calloc(clipboard->nFiles, sizeof(WCHAR *)); + list->file_sizes = (UINT64 *)calloc(clipboard->nFiles, sizeof(UINT64)); + list->descriptors = (BYTE *)malloc(size); + if (!list->file_names || !list->file_sizes || !list->descriptors) + return FALSE; + CopyMemory(list->descriptors, dsc, size); + list->descriptorsSize = size; + 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]))) + return FALSE; + if (clipboard->fileDescriptor[i]) + list->file_sizes[i] = ((UINT64)clipboard->fileDescriptor[i]->nFileSizeHigh << 32) | + clipboard->fileDescriptor[i]->nFileSizeLow; + } + 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. A full table evicts the list + * sent least recently, so a list a transfer has just started on is kept. */ +static void wf_cliprdr_remember_served_file_list(wfClipboard *clipboard, UINT32 connID, + const FILEGROUPDESCRIPTORW *dsc, SIZE_T size) +{ + 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) + { + // An empty slot has last_served 0, so it is taken first. + list = &clipboard->served_file_lists[0]; + for (i = 1; i < WF_CLIPRDR_SERVED_FILE_LISTS; i++) + { + if (clipboard->served_file_lists[i].last_served < list->last_served) + list = &clipboard->served_file_lists[i]; + } + wf_cliprdr_free_served_file_list(list); + if (!wf_cliprdr_copy_file_list(clipboard, list, dsc, size)) + { + wf_cliprdr_free_served_file_list(list); + return; + } + } + list->last_served = ++clipboard->served_file_list_seq; + + 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; +} + +/* Reads like wf_cliprdr_get_file_contents, but only from the file the list was sent with: a + * different size or last-write time means the path now names another file, and the read fails + * rather than continuing a transfer with its bytes. */ +static BOOL wf_cliprdr_get_served_file_contents(const WCHAR *file_name, const FILEDESCRIPTORW *dsc, + BYTE *buffer, DWORD positionLow, DWORD positionHigh, + DWORD nRequested, DWORD *puSize) +{ + BOOL res = FALSE; + HANDLE hFile; + LARGE_INTEGER size; + LARGE_INTEGER position; + FILETIME lastWrite; + DWORD nGet; + + hFile = CreateFileW(file_name, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hFile == INVALID_HANDLE_VALUE) + return FALSE; + + if (GetFileSizeEx(hFile, &size) && + (UINT64)size.QuadPart == (((UINT64)dsc->nFileSizeHigh << 32) | dsc->nFileSizeLow) && + (!(dsc->dwFlags & FD_WRITESTIME) || + (GetFileTime(hFile, NULL, NULL, &lastWrite) && + CompareFileTime(&lastWrite, &dsc->ftLastWriteTime) == 0))) + { + position.LowPart = positionLow; + position.HighPart = (LONG)positionHigh; + if (SetFilePointerEx(hFile, position, NULL, FILE_BEGIN) && + ReadFile(hFile, buffer, nRequested, &nGet, NULL)) + { + *puSize = nGet; + res = TRUE; + } + } + + CloseHandle(hFile); + return res; +} + +/* Serves a request by path from the file list it names, if that list was sent to the same + * connection. An unknown list fails the request rather than falling back to other files. */ +static BOOL wf_cliprdr_read_served_file_list(wfClipboard *clipboard, + const CLIPRDR_FILE_CONTENTS_REQUEST *request, + BYTE *data, UINT32 cbRequested, DWORD *puSize) +{ + wfServedFileList *list = NULL; + 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 == request->clipDataId && + wf_cliprdr_served_to(&clipboard->served_file_lists[i], request->connID)) + { + list = &clipboard->served_file_lists[i]; + break; + } + } + if (!list || request->listIndex >= list->nFiles) + return FALSE; + + if (request->dwFlags == FILECONTENTS_SIZE) + { + CopyMemory(data, &list->file_sizes[request->listIndex], sizeof(UINT64)); + *puSize = sizeof(UINT64); + return TRUE; + } + if (request->dwFlags != FILECONTENTS_RANGE) + return FALSE; + + if (clipboard->context->HandleClipboardFiles && + request->listIndex == (UINT32)list->first_file_index && + request->nPositionLow == 0 && request->nPositionHigh == 0) + { + clipboard->context->HandleClipboardFiles(request->connID, list->nFiles, list->file_names); + } + return wf_cliprdr_get_served_file_contents( + list->file_names[request->listIndex], + &((const FILEGROUPDESCRIPTORW *)list->descriptors)->fgd[request->listIndex], data, + request->nPositionLow, request->nPositionHigh, cbRequested, puSize); +} + /* path_name has a '\' at the end. e.g. c:\newfolder\, file_name is c:\newfolder\new.txt */ static FILEDESCRIPTORW *wf_cliprdr_get_file_descriptor(WCHAR *file_name, size_t pathLen) { @@ -3028,6 +3335,8 @@ wf_cliprdr_server_format_data_request(CliprdrClientContext *context, STGMEDIUM stg_medium; DROPFILES *dropFiles; FILEGROUPDESCRIPTORW *groupDsc; + clipboard->file_list_seq = GetClipboardSequenceNumber(); + clipboard->file_list_id = 0; result = OleGetClipboard(&dataObj); if (FAILED(result)) @@ -3164,6 +3473,10 @@ wf_cliprdr_server_format_data_request(CliprdrClientContext *context, buff = groupDsc; rc = ERROR_SUCCESS; + wf_cliprdr_stamp_file_list(clipboard, groupDsc, size); + clipboard->file_list_id = wf_cliprdr_file_list_id((const BYTE *)groupDsc, size); + wf_cliprdr_remember_served_file_list(clipboard, formatDataRequest->connID, groupDsc, + size); } else { @@ -3376,6 +3689,18 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context, return ERROR_INTERNAL_ERROR; } + // Refuse an unknown request type, or a range over the cap, before allocating for it. Clients + // of this version split larger reads; an older Windows client reading more in one + // IStream::Read fails, as with unix owners. + if ((fileContentsRequest->dwFlags != FILECONTENTS_SIZE && + fileContentsRequest->dwFlags != FILECONTENTS_RANGE) || + (fileContentsRequest->dwFlags == FILECONTENTS_RANGE && + fileContentsRequest->cbRequested > WF_CLIPRDR_MAX_RANGE_READ)) + { + ZeroMemory(&vStgMedium, sizeof(STGMEDIUM)); // read at exit + goto exit; + } + // If the clipboard is set by the instance, or the file descriptor is from remote, // we should not process the request. // Because this may be the following cases: @@ -3401,6 +3726,22 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context, goto exit; } + // A stream whose list is no longer the current one reads by path from the list it came from. + // Only lists sent to this connection are served. + if (fileContentsRequest->haveClipDataId && + fileContentsRequest->clipDataId != clipboard->file_list_id) + { + ZeroMemory(&vStgMedium, sizeof(STGMEDIUM)); // read at exit + cbRequested = fileContentsRequest->dwFlags == FILECONTENTS_SIZE + ? sizeof(UINT64) + : fileContentsRequest->cbRequested; + pData = (BYTE *)calloc(1, cbRequested); + if (pData && wf_cliprdr_read_served_file_list(clipboard, fileContentsRequest, pData, + cbRequested, &uSize)) + rc = CHANNEL_RC_OK; + goto exit; + } + cbRequested = fileContentsRequest->cbRequested; if (fileContentsRequest->dwFlags == FILECONTENTS_SIZE) cbRequested = sizeof(UINT64); @@ -3430,7 +3771,13 @@ wf_cliprdr_server_file_contents_request(CliprdrClientContext *context, vFormatEtc.lindex = fileContentsRequest->listIndex; vFormatEtc.ptd = NULL; - if ((uStreamIdStc != fileContentsRequest->streamId) || + if (GetClipboardSequenceNumber() != clipboard->file_list_seq) + { + // The clipboard changed after the peer got its file list. The live FileContents + // belongs to the new copy, so reading it would splice another file into this one. + bIsStreamFile = FALSE; + } + else if ((uStreamIdStc != fileContentsRequest->streamId) || (uConnIdStc != fileContentsRequest->connID) || !pStreamStc) { LPENUMFORMATETC pEnumFormatEtc; @@ -3842,6 +4189,7 @@ BOOL wf_cliprdr_uninit(wfClipboard *clipboard, CliprdrClientContext *cliprdr) CloseHandle(clipboard->req_fevent); clear_file_array(clipboard); + wf_cliprdr_forget_served_file_lists(clipboard); clear_format_map(clipboard); free(clipboard->format_mappings); return TRUE; diff --git a/libs/enigo/src/win/win_impl.rs b/libs/enigo/src/win/win_impl.rs index 41aca472c..5506b4c95 100644 --- a/libs/enigo/src/win/win_impl.rs +++ b/libs/enigo/src/win/win_impl.rs @@ -39,6 +39,17 @@ fn mouse_event(flags: u32, data: u32, dx: i32, dy: i32) -> DWORD { unsafe { SendInput(1, &mut input as LPINPUT, size_of::() as c_int) } } +// `v` comes from the remote peer unchecked, so scale in i64 to avoid overflow, and clamp to +// the 0..=65535 range MOUSEEVENTF_ABSOLUTE takes. +// `extent` is 0 when the virtual screen metrics are unavailable. +fn to_absolute(v: i32, origin: i32, extent: i32) -> Option { + if extent <= 0 { + return None; + } + let abs = (v as i64 - origin as i64) * 65535 / extent as i64; + Some(abs.clamp(0, 65535) as i32) +} + fn keybd_event(mut flags: u32, vk: u16, scan: u16) -> DWORD { let mut scan = scan; unsafe { @@ -126,13 +137,28 @@ impl MouseControllable for Enigo { } fn mouse_move_to(&mut self, x: i32, y: i32) { + let (left, top, width, height) = unsafe { + ( + GetSystemMetrics(SM_XVIRTUALSCREEN), + GetSystemMetrics(SM_YVIRTUALSCREEN), + GetSystemMetrics(SM_CXVIRTUALSCREEN), + GetSystemMetrics(SM_CYVIRTUALSCREEN), + ) + }; + let (Some(dx), Some(dy)) = (to_absolute(x, left, width), to_absolute(y, top, height)) + else { + hbb_common::throttled_log!( + std::time::Duration::from_secs(60), + warn, + "mouse_move_to skipped: virtual screen size unavailable" + ); + return; + }; mouse_event( MOUSEEVENTF_MOVE | MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESK, 0, - (x - unsafe { GetSystemMetrics(SM_XVIRTUALSCREEN) }) * 65535 - / unsafe { GetSystemMetrics(SM_CXVIRTUALSCREEN) }, - (y - unsafe { GetSystemMetrics(SM_YVIRTUALSCREEN) }) * 65535 - / unsafe { GetSystemMetrics(SM_CYVIRTUALSCREEN) }, + dx, + dy, ); } diff --git a/src/clipboard_file.rs b/src/clipboard_file.rs index b50f3c828..6ec08188a 100644 --- a/src/clipboard_file.rs +++ b/src/clipboard_file.rs @@ -321,7 +321,7 @@ pub mod unix_file_clip { requested_format_id: _requested_format_id, } => { log::debug!("requested format id: {}", _requested_format_id); - let format_data = serv_files::get_file_list_pdu(); + let format_data = serv_files::get_file_list_pdu(conn_id); if !format_data.is_empty() { return vec![clip_2_msg(ClipboardFile::FormatDataResponse { msg_flags: 1, @@ -372,7 +372,8 @@ pub mod unix_file_clip { n_position_low, n_position_high, cb_requested, - .. + have_clip_data_id, + clip_data_id, } => { log::debug!("file contents request: stream_id: {}, list_index: {}, dw_flags: {}, n_position_low: {}, n_position_high: {}, cb_requested: {}", stream_id, list_index, dw_flags, n_position_low, n_position_high, cb_requested); return serv_files::read_file_contents( @@ -383,12 +384,18 @@ pub mod unix_file_clip { n_position_low, n_position_high, cb_requested, + have_clip_data_id.then_some(clip_data_id), ) .into_iter() .map(|res| match res { Ok(data) => clip_2_msg(data), Err(e) => { - log::error!("failed to read file contents: {:?}", e); + hbb_common::throttled_log!( + std::time::Duration::from_secs(5), + error, + "failed to read file contents: {:?}", + e + ); resp_file_contents_fail(stream_id) } })