diff --git a/src/lib.rs b/src/lib.rs index c1c0fa1..8e2147a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,9 @@ pub trait RunData { None } fn xwayland_ready(&self, _display: String, _pid: u32) {} + fn max_req_len_bytes(&self) -> Option { + None + } } pub const fn timespec_from_millis(millis: u64) -> Timespec { @@ -186,6 +189,10 @@ pub fn main(mut data: impl RunData) -> Option<()> { } let mut xstate = XState::new(xsock_wl.as_fd()); + if let Some(bytes) = data.max_req_len_bytes() { + xstate.set_max_req_bytes(bytes); + } + let mut reader = BufReader::new(&ready_rx); { let mut display = String::new(); diff --git a/src/xstate/mod.rs b/src/xstate/mod.rs index 823a310..2e0b78a 100644 --- a/src/xstate/mod.rs +++ b/src/xstate/mod.rs @@ -119,6 +119,7 @@ pub struct XState { wm_window: x::Window, selection_state: SelectionState, settings: Settings, + max_req_bytes: usize, } impl XState { @@ -226,6 +227,9 @@ impl XState { let selection_state = SelectionState::new(&connection, root, &atoms); let window_atoms = WindowTypes::intern_all(&connection).unwrap(); let settings = Settings::new(&connection, &atoms, root); + // maximum-request-length is returned in units of 4 bytes. + // Additionally, requests use 32 bytes of metadata which cannot store arbitrary data + let max_req_bytes = (connection.get_maximum_request_length() * 4 - 32) as usize; let mut r = Self { connection, @@ -235,12 +239,21 @@ impl XState { window_atoms, selection_state, settings, + max_req_bytes, }; r.create_ewmh_window(); r.set_xsettings_owner(); r } + pub(super) fn set_max_req_bytes(&mut self, max_req_bytes: usize) { + // `max_req_bytes` is initialized to the largest possible value before transfer problems + // would begin to occur. This function is called once during initialization only in + // integration tests, so this overly simple check is fine. + assert!(self.max_req_bytes >= max_req_bytes); + self.max_req_bytes = max_req_bytes; + } + pub fn server_state_setup( &self, server_state: super::EarlyServerState, @@ -883,9 +896,12 @@ impl XState { event: x::PropertyNotifyEvent, server_state: &mut super::RealServerState, ) { - if event.state() != x::Property::NewValue { + if self.handle_selection_property_change(&event) { + return; + } + if event.state() == x::Property::Delete { debug!( - "ignoring non newvalue for property {:?}", + "ignoring delete for property {:?}", get_atom_name(&self.connection, event.atom()) ); return; @@ -929,9 +945,7 @@ impl XState { } } _ => { - if !self.handle_selection_property_change(&event) - && log::log_enabled!(log::Level::Debug) - { + if log::log_enabled!(log::Level::Debug) { debug!( "changed property {:?} for {:?}", get_atom_name(&self.connection, event.atom()), diff --git a/src/xstate/selection.rs b/src/xstate/selection.rs index 884f091..8736109 100644 --- a/src/xstate/selection.rs +++ b/src/xstate/selection.rs @@ -162,12 +162,59 @@ impl Selection { } } +pub struct WaylandIncrInfo { + data: Vec, + range: std::ops::Range, + property: x::Atom, + target_window: x::Window, + target_type: x::Atom, + max_req_bytes: usize, +} + +pub struct WaylandSelection { + mimes: Vec, + inner: ForeignSelection, + incr_data: Option, +} + +impl WaylandSelection { + fn check_for_incr(&mut self, connection: &xcb::Connection) -> Option { + let incr_data = self.incr_data.as_mut()?; + let range_start = incr_data.range.start; + let range_len = std::cmp::min(incr_data.max_req_bytes, incr_data.range.end - range_start); + + if let Err(e) = connection.send_and_check_request(&x::ChangeProperty { + mode: x::PropMode::Append, + window: incr_data.target_window, + property: incr_data.property, + r#type: incr_data.target_type, + data: &incr_data.data[range_start..][..range_len], + }) { + warn!("failed to write selection data: {e:?}"); + self.incr_data = None; + return Some(true); + } + + incr_data.range.start += range_len; + if range_len == 0 { + debug!( + "completed incr for mime {}", + get_atom_name(connection, incr_data.target_type) + ); + self.incr_data = None; + } else { + debug!( + "received some incr data for {}", + get_atom_name(connection, incr_data.target_type) + ); + } + Some(true) + } +} + enum CurrentSelection { X11(Rc), - Wayland { - mimes: Vec, - inner: ForeignSelection, - }, + Wayland(WaylandSelection), } struct SelectionData { @@ -198,10 +245,11 @@ trait SelectionDataImpl { ); fn x11_selection(&self) -> Option<&Selection>; fn handle_selection_request( - &self, + &mut self, connection: &xcb::Connection, atoms: &super::Atoms, request: &x::SelectionRequestEvent, + max_req_bytes: usize, success: &dyn Fn(), refuse: &dyn Fn(), server_state: &mut RealServerState, @@ -217,6 +265,12 @@ impl SelectionData { current_selection: None, } } + fn wayland_selection_mut(&mut self) -> Option<&mut WaylandSelection> { + match &mut self.current_selection { + Some(CurrentSelection::Wayland(sel)) => Some(sel), + _ => None, + } + } } impl SelectionDataImpl for SelectionData { @@ -359,15 +413,21 @@ impl SelectionDataImpl for SelectionData { } fn handle_selection_request( - &self, + &mut self, connection: &xcb::Connection, atoms: &super::Atoms, request: &x::SelectionRequestEvent, + max_req_bytes: usize, success: &dyn Fn(), refuse: &dyn Fn(), server_state: &mut RealServerState, ) { - let Some(CurrentSelection::Wayland { mimes, inner }) = &self.current_selection else { + let Some(CurrentSelection::Wayland(WaylandSelection { + mimes, + inner, + ref mut incr_data, + })) = &mut self.current_selection + else { warn!("Got selection request, but we don't seem to be the selection owner"); refuse(); return; @@ -405,17 +465,52 @@ impl SelectionDataImpl for SelectionData { .cloned() .unwrap_or_else(|| target.name.clone()); let data = inner.receive(mime_name, server_state); - match connection.send_and_check_request(&x::ChangeProperty { - mode: x::PropMode::Replace, - window: request.requestor(), - property: request.property(), - r#type: target.atom, - data: &data, - }) { - Ok(_) => success(), - Err(e) => { - warn!("Failed setting selection property: {e:?}"); + if data.len() > max_req_bytes { + if let Err(e) = connection.send_and_check_request(&x::ChangeWindowAttributes { + window: request.requestor(), + value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)], + }) { + warn!("Failed to set up property change notifications: {e:?}"); refuse(); + return; + } + if let Err(e) = connection.send_and_check_request(&x::ChangeProperty { + mode: x::PropMode::Replace, + window: request.requestor(), + property: request.property(), + r#type: atoms.incr, + data: &[data.len() as u32], + }) { + warn!("Failed to set incr property for large transfer: {e:?}"); + refuse(); + return; + } + debug!( + "beginning incr for {}", + get_atom_name(connection, target.atom) + ); + *incr_data = Some(WaylandIncrInfo { + range: 0..data.len(), + data, + target_window: request.requestor(), + property: request.property(), + target_type: target.atom, + max_req_bytes, + }); + success() + } else { + match connection.send_and_check_request(&x::ChangeProperty { + mode: x::PropMode::Replace, + window: request.requestor(), + property: request.property(), + r#type: target.atom, + data: &data, + }) { + Ok(_) => success(), + Err(e) => { + warn!("Failed setting selection property: {e:?}"); + refuse(); + } } } } @@ -503,10 +598,12 @@ impl XState { }); } - self.selection_state.clipboard.current_selection = Some(CurrentSelection::Wayland { - mimes, - inner: selection, - }); + self.selection_state.clipboard.current_selection = + Some(CurrentSelection::Wayland(WaylandSelection { + mimes, + inner: selection, + incr_data: None, + })); self.selection_state .clipboard .set_owner(&self.connection, self.wm_window); @@ -559,10 +656,12 @@ impl XState { }); } - self.selection_state.primary.current_selection = Some(CurrentSelection::Wayland { - mimes, - inner: selection, - }); + self.selection_state.primary.current_selection = + Some(CurrentSelection::Wayland(WaylandSelection { + mimes, + inner: selection, + incr_data: None, + })); self.selection_state .primary .set_owner(&self.connection, self.wm_window); @@ -676,6 +775,7 @@ impl XState { &self.connection, &self.atoms, e, + self.max_req_bytes, &success, &refuse, server_state, @@ -725,15 +825,28 @@ impl XState { &mut self, event: &x::PropertyNotifyEvent, ) -> bool { - for data in [ - &self.selection_state.primary as &dyn SelectionDataImpl, - &self.selection_state.clipboard as _, - ] { - if let Some(selection) = &data.x11_selection() { - return selection.check_for_incr(event); + fn inner( + connection: &xcb::Connection, + event: &x::PropertyNotifyEvent, + data: &mut SelectionData, + ) -> Option { + match event.state() { + x::Property::NewValue => { + if let Some(selection) = &data.x11_selection() { + return Some(selection.check_for_incr(event)); + } + } + x::Property::Delete => { + if let Some(selection) = data.wayland_selection_mut() { + return selection.check_for_incr(connection); + } + } } + None } - false + inner(&self.connection, event, &mut self.selection_state.primary) + .or_else(|| inner(&self.connection, event, &mut self.selection_state.clipboard)) + .unwrap_or(false) } } diff --git a/tests/integration.rs b/tests/integration.rs index df844fa..4f422eb 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -82,6 +82,10 @@ impl xwls::RunData for TestData { assert!(server.is_some()); server.take() } + + fn max_req_len_bytes(&self) -> Option { + Some(500) + } } struct Fixture { @@ -551,6 +555,23 @@ impl Connection { .unwrap(); } + #[track_caller] + fn await_property_notify(&mut self) -> x::PropertyNotifyEvent { + match self.await_event() { + xcb::Event::X(x::Event::PropertyNotify(r)) => r, + other => panic!("Didn't get property notify event, instead got {other:?}"), + } + } + + #[track_caller] + fn get_property_change_events(&self, window: x::Window) { + self.send_and_check_request(&x::ChangeWindowAttributes { + window, + value_list: &[x::Cw::EventMask(x::EventMask::PROPERTY_CHANGE)], + }) + .unwrap(); + } + #[track_caller] fn verify_clipboard_owner(&self, window: x::Window) { let owner = self.get_reply(&x::GetSelectionOwner { @@ -1163,17 +1184,14 @@ fn copy_from_wayland() { #[test] fn incr_copy_from_wayland() { - // After a little binary searching, the test passes on less than 16,777,184 bytes, fails due to - // a failed assert on matching `dest1_atom` starting at 16,777,185 bytes, and starts crashing - // instead at 16,777,189 bytes. - const BYTES: usize = 16_777_189; + const BYTES: usize = 3000; let mut f = Fixture::new(); let mut connection = Connection::new(&f.display); let window = connection.new_window(connection.root, 0, 0, 20, 20, false); connection.get_selection_owner_change_events(true, window); f.map_as_toplevel(&mut connection, window); - let offer = vec![testwl::PasteData { + let mut offer = vec![testwl::PasteData { mime_type: "text/plain".into(), data: std::iter::successors(Some(0u8), |n| Some(n.wrapping_add(1))) .take(BYTES) @@ -1219,49 +1237,70 @@ fn incr_copy_from_wayland() { let targets: &[x::Atom] = reply.value(); assert_eq!(targets.len(), 1); - for testwl::PasteData { mime_type, data } in offer { - let atom = connection - .get_reply(&x::InternAtom { - only_if_exists: true, - name: mime_type.as_bytes(), - }) - .atom(); - assert_ne!(atom, x::ATOM_NONE); - assert!(targets.contains(&atom)); + let offer_data = offer.pop().unwrap(); + let (mime_type, data) = (offer_data.mime_type, offer_data.data); + let atom = connection + .get_reply(&x::InternAtom { + only_if_exists: true, + name: mime_type.as_bytes(), + }) + .atom(); + assert_ne!(atom, x::ATOM_NONE); + assert!(targets.contains(&atom)); - std::thread::sleep(std::time::Duration::from_millis(50)); - connection - .send_and_check_request(&x::ConvertSelection { - requestor: window, - selection: connection.atoms.clipboard, - target: atom, - property: dest1_atom, - time: x::CURRENT_TIME, - }) - .unwrap(); + connection + .send_and_check_request(&x::ConvertSelection { + requestor: window, + selection: connection.atoms.clipboard, + target: atom, + property: dest1_atom, + time: x::CURRENT_TIME, + }) + .unwrap(); - f.wait_and_dispatch(); - let request = connection.await_selection_notify(); - assert_eq!(request.requestor(), window); - assert_eq!(request.selection(), connection.atoms.clipboard); - assert_eq!(request.target(), atom); - // The particular assert which fails in the narrow byte range as mentioned above. - assert_eq!(request.property(), dest1_atom); + f.wait_and_dispatch(); + let request = connection.await_selection_notify(); + assert_eq!(request.requestor(), window); + assert_eq!(request.selection(), connection.atoms.clipboard); + assert_eq!(request.target(), atom); + assert_eq!(request.property(), dest1_atom); - let val: Vec = connection - .get_reply(&x::GetProperty { - delete: true, - window, - property: dest1_atom, - r#type: x::ATOM_ANY, - long_offset: 0, - long_length: (BYTES / 4 + BYTES % 4) as u32, - }) - .value() - .to_vec(); + connection.get_property_change_events(window); + let reply = connection.get_reply(&x::GetProperty { + delete: true, + window, + property: dest1_atom, + r#type: x::ATOM_ANY, + long_offset: 0, + long_length: (BYTES / 4 + BYTES % 4) as u32, + }); + assert_eq!(reply.r#type(), connection.atoms.incr); + assert_eq!(reply.value::().len(), 1); + assert!(reply.value::()[0] <= data.len() as u32); - assert_eq!(val.len(), data.len()); - assert_eq!(val, data); + let delete_property = connection.await_property_notify(); + assert_eq!(delete_property.state(), x::Property::Delete); + assert_eq!(delete_property.atom(), dest1_atom); + + for (idx, chunk) in data.chunks(500).chain([]).enumerate() { + let new_property = connection.await_property_notify(); + assert_eq!(new_property.state(), x::Property::NewValue, "chunk {idx}"); + assert_eq!(new_property.atom(), dest1_atom, "chunk {idx}"); + + let incr_reply = connection.get_reply(&x::GetProperty { + delete: true, + window, + property: dest1_atom, + r#type: x::ATOM_ANY, + long_offset: 0, + long_length: (BYTES / 4 + BYTES % 4) as u32, + }); + assert_eq!(incr_reply.r#type(), atom, "chunk {idx}"); + assert_eq!(incr_reply.value::(), chunk, "chunk {idx}"); + + let delete_property = connection.await_property_notify(); + assert_eq!(delete_property.state(), x::Property::Delete, "chunk {idx}"); + assert_eq!(delete_property.atom(), dest1_atom, "chunk {idx}"); } }