From 4029f023a615fe6995a37898d89dbf7390bf4ad5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 23:05:04 -0500 Subject: [PATCH] ui: check floating destinations and integrity-matched font bytes (#567) - docking: apply_with_permissions refuses a move or open into a floating window for a panel that is not floatable (Move, OpenAt, MovePanel and group/column/window moves), and a Group placement at a floating anchor, which becomes a split of its frame, needs accepts_splits. - fonts: the served-font loader keys fetched files, in-flight requests, failures and pending UI faces by (url, integrity), and installs bytes only into faces requiring the integrity they were fetched under (Stack::provide_fetched); the public Stack::provide is unchanged. --- crates/ui/src/docking/model.rs | 21 ++++++-- crates/ui/src/docking/model_tests.rs | 28 +++++++++++ crates/ui/src/fonts/served.rs | 13 ++++- crates/ui/src/fonts/served/loader.rs | 72 +++++++++++++++------------- crates/ui/src/fonts/tests.rs | 14 ++++++ 5 files changed, 109 insertions(+), 39 deletions(-) diff --git a/crates/ui/src/docking/model.rs b/crates/ui/src/docking/model.rs index 0c90c140..26c3eaf4 100644 --- a/crates/ui/src/docking/model.rs +++ b/crates/ui/src/docking/model.rs @@ -941,6 +941,9 @@ impl Layout

{ } action => action, }; + // A destination in a floating window takes only floatable panels, and a group placed + // there becomes a split of the anchor's frame. + let floats = |anchor: &P| self.floating.iter().any(|w| w.contains(anchor)); match &action { Action::Move { panel, anchor, placement } | Action::OpenAt { panel, anchor, placement } => { // Opening an absent panel moves nothing, so it works in a locked workspace too. @@ -948,7 +951,7 @@ impl Layout

{ if !opens && !permissions(panel).movable { return Err(DockError::NotAllowed); } - if !placement_allowed(placement, permissions(anchor)) { + if !placement_allowed(placement, permissions(anchor), floats(anchor)) || floats(anchor) && !permissions(panel).floatable { return Err(DockError::NotAllowed); } } @@ -967,7 +970,9 @@ impl Layout

{ let allowed = policy.movable && match to { Target::Float { .. } => policy.floatable, - Target::At { anchor, placement } => placement_allowed(placement, permissions(anchor)), + Target::At { anchor, placement } => { + placement_allowed(placement, permissions(anchor), floats(anchor)) && (!floats(anchor) || policy.floatable) + } Target::NewDock { .. } => true, }; if !allowed { @@ -981,7 +986,11 @@ impl Layout

{ } match to { Target::Float { .. } if members.iter().any(|p| !permissions(p).floatable) => return Err(DockError::NotAllowed), - Target::At { anchor, placement } if !members.contains(anchor) && !placement_allowed(placement, permissions(anchor)) => { + Target::At { anchor, placement } + if !members.contains(anchor) + && (!placement_allowed(placement, permissions(anchor), floats(anchor)) + || floats(anchor) && members.iter().any(|p| !permissions(p).floatable)) => + { return Err(DockError::NotAllowed); } _ => {} @@ -1329,11 +1338,13 @@ impl Layout

{ } /// Whether the anchor's permissions take this placement: tabs need `accepts_tabs`, side -/// splits `accepts_splits`. -fn placement_allowed

(placement: &Placement

, anchor: Permissions) -> bool { +/// splits `accepts_splits`, and so does a group placed at an anchor in a floating window +/// (`floating`), which splits the anchor's frame. +fn placement_allowed

(placement: &Placement

, anchor: Permissions, floating: bool) -> bool { match placement { Placement::Tab { .. } | Placement::Split(Zone::Center) => anchor.accepts_tabs, Placement::Split(_) | Placement::SplitSized { .. } => anchor.accepts_splits, + Placement::Group { .. } if floating => anchor.accepts_splits, _ => true, } } diff --git a/crates/ui/src/docking/model_tests.rs b/crates/ui/src/docking/model_tests.rs index 4df96d21..3c541f10 100644 --- a/crates/ui/src/docking/model_tests.rs +++ b/crates/ui/src/docking/model_tests.rs @@ -118,6 +118,34 @@ fn group_merge_into_tabs_inserts_every_member_before_the_anchor_tab() { assert_eq!(layout, before); } +#[test] +fn a_floating_destination_takes_only_floatable_panels_and_a_group_there_needs_splits() { + let before = docked(); + // Panel 4 may move but not float; panel 6 (floating) takes no splits. + let policy = |p: &u32| match p { + 4 => Permissions { floatable: false, ..Permissions::default() }, + 6 => Permissions { accepts_splits: false, ..Permissions::default() }, + _ => Permissions::default(), + }; + let tab = Placement::Tab { before: None }; + let group = Placement::Group { below: true, height: None }; + for action in [ + Action::Move { panel: 4, anchor: 8, placement: tab.clone() }, + Action::MovePanel { panel: 4, to: Target::At { anchor: 8, placement: tab.clone() } }, + Action::MoveGroup { panel: 4, to: Target::At { anchor: 8, placement: tab.clone() } }, + Action::MoveGroup { panel: 11, to: Target::At { anchor: 6, placement: group.clone() } }, + ] { + let mut layout = before.clone(); + assert_eq!(layout.apply_with_permissions(action.clone(), policy), Err(DockError::NotAllowed), "{action:?}"); + assert_eq!(layout, before); + } + // Floatable panels still join the window, and a group still goes below a docked anchor. + let mut layout = before.clone(); + layout.apply_with_permissions(Action::Move { panel: 5, anchor: 8, placement: tab }, policy).unwrap(); + let mut layout = before.clone(); + layout.apply_with_permissions(Action::MoveGroup { panel: 11, to: Target::At { anchor: 2, placement: group } }, policy).unwrap(); +} + #[test] fn column_moves_and_floats_keep_every_group() { let mut layout = docked(); diff --git a/crates/ui/src/fonts/served.rs b/crates/ui/src/fonts/served.rs index d1cbcd41..e17a563b 100644 --- a/crates/ui/src/fonts/served.rs +++ b/crates/ui/src/fonts/served.rs @@ -116,7 +116,18 @@ impl Stack { /// [`super::UiFonts::stack`] would have. The web build does this for each file it fetches; an /// app that gets the files some other way can call it itself. pub fn provide(&mut self, url: &str, file: &Blob) { - let faces: Vec = self.served.iter().filter(|face| face.url == url).copied().collect(); + self.provide_to(file, |face| face.url == url); + } + + /// [`Self::provide`] for bytes fetched and checked against `integrity`: only the faces that + /// require that integrity (or none, when it is empty) take them. + #[cfg(any(test, target_arch = "wasm32"))] + pub(crate) fn provide_fetched(&mut self, url: &str, integrity: &str, file: &Blob) { + self.provide_to(file, |face| face.url == url && face.integrity == integrity); + } + + fn provide_to(&mut self, file: &Blob, takes: impl Fn(&ServedFace) -> bool) { + let faces: Vec = self.served.iter().filter(|face| takes(face)).copied().collect(); for face in faces { let name = face.name(); let data = baseline_fallback(egui::FontData::from_blob(Arc::clone(file), face.index)); diff --git a/crates/ui/src/fonts/served/loader.rs b/crates/ui/src/fonts/served/loader.rs index b8de5dc3..17ebb5cd 100644 --- a/crates/ui/src/fonts/served/loader.rs +++ b/crates/ui/src/fonts/served/loader.rs @@ -37,26 +37,27 @@ pub(in crate::fonts) fn install(ctx: &egui::Context, mut stack: Stack, fetcher: let fetch = { let mut state = loader.lock(); let now = loader.fetcher.now(); - for (url, file) in &state.files { - stack.provide(url, file); + for (&(url, integrity), file) in &state.files { + stack.provide_fetched(url, integrity, file); } state.ui_pending.clear(); - let mut fetch: Vec<(&'static str, &'static str)> = Vec::new(); + let mut fetch: Vec = Vec::new(); for face in stack.ui_downloads() { - if state.files.contains_key(face.url) || !state.due(face.url, now) { + let key = (face.url, face.integrity); + if state.files.contains_key(&key) || !state.due(key, now) { continue; } - state.ui_pending.insert(face.url); - if !fetch.iter().any(|(url, _)| *url == face.url) { - fetch.push((face.url, face.integrity)); + state.ui_pending.insert(key); + if !fetch.contains(&key) { + fetch.push(key); } } state.stack = Some(stack.clone()); fetch }; stack.install_faces(ctx, Some(Arc::clone(&loader) as Arc)); - for (url, integrity) in fetch { - loader.fetch(url, integrity); + for key in fetch { + loader.fetch(key); } } @@ -92,16 +93,20 @@ struct Loader { state: Mutex, } +/// A file as one face asks for it: its URL and the integrity the bytes were checked against. +/// Bytes fetched for one integrity (or none) are never handed to a face that requires another. +type Key = (&'static str, &'static str); + #[derive(Default)] struct State { /// The installed stack, without the files' data. stack: Option, - /// Fetched files by URL; the browser's HTTP cache keeps them across page loads. - files: BTreeMap<&'static str, Blob>, - in_flight: BTreeSet<&'static str>, - failed: BTreeMap<&'static str, Failure>, + /// Fetched files; the browser's HTTP cache keeps them across page loads. + files: BTreeMap, + in_flight: BTreeSet, + failed: BTreeMap, /// The UI language's files not fetched yet. - ui_pending: BTreeSet<&'static str>, + ui_pending: BTreeSet, } struct Failure { @@ -110,9 +115,9 @@ struct Failure { } impl State { - /// Whether `url` may be fetched: it never failed, or its retry time has come. - fn due(&self, url: &str, now: f64) -> bool { - self.failed.get(url).is_none_or(|failure| now >= failure.retry_at) + /// Whether `key` may be fetched: it never failed, or its retry time has come. + fn due(&self, key: Key, now: f64) -> bool { + self.failed.get(&key).is_none_or(|failure| now >= failure.retry_at) } } @@ -130,34 +135,35 @@ impl Loader { self.state.lock().unwrap_or_else(PoisonError::into_inner) } - fn fetch(&self, url: &'static str, integrity: &'static str) { - if !self.lock().in_flight.insert(url) { + fn fetch(&self, key: Key) { + if !self.lock().in_flight.insert(key) { return; } let Some(me) = self.me.upgrade() else { return }; - self.fetcher.fetch(url, integrity, Box::new(move |result| me.arrived(url, result))); + let (url, integrity) = key; + self.fetcher.fetch(url, integrity, Box::new(move |result| me.arrived(key, result))); } - fn arrived(&self, url: &'static str, result: Result, String>) { + fn arrived(&self, key: Key, result: Result, String>) { let (stack, retry) = { let mut state = self.lock(); - state.in_flight.remove(url); - state.ui_pending.remove(url); + state.in_flight.remove(&key); + state.ui_pending.remove(&key); match result { Ok(bytes) => { - state.failed.remove(url); + state.failed.remove(&key); let file: Blob = Arc::new(bytes); - state.files.insert(url, Arc::clone(&file)); + state.files.insert(key, Arc::clone(&file)); let stack = state.stack.as_mut().map(|stack| { - stack.provide(url, &file); + stack.provide_fetched(key.0, key.1, &file); stack.clone() }); (stack, None) } Err(_) => { - let attempts = state.failed.get(url).map_or(1, |failure| failure.attempts + 1); + let attempts = state.failed.get(&key).map_or(1, |failure| failure.attempts + 1); let delay = (RETRY_FIRST * 2f64.powi(attempts.saturating_sub(1).min(16) as i32)).min(RETRY_MAX); - state.failed.insert(url, Failure { attempts, retry_at: self.fetcher.now() + delay }); + state.failed.insert(key, Failure { attempts, retry_at: self.fetcher.now() + delay }); (None, Some(delay)) } } @@ -196,18 +202,18 @@ impl FontProvider for Loader { let state = self.lock(); let mut next = None; for face in state.stack.as_ref()?.downloads(request.family, request.cluster) { - if state.in_flight.contains(face.url) || state.files.contains_key(face.url) { + let key = (face.url, face.integrity); + if state.in_flight.contains(&key) || state.files.contains_key(&key) { // It draws the character once it arrives, or already does. return None; } - if state.due(face.url, now) { - next = Some((face.url, face.integrity)); + if state.due(key, now) { + next = Some(key); break; } } drop(state); - let (url, integrity) = next?; - self.fetch(url, integrity); + self.fetch(next?); None } } diff --git a/crates/ui/src/fonts/tests.rs b/crates/ui/src/fonts/tests.rs index 06439c79..6e0443ad 100644 --- a/crates/ui/src/fonts/tests.rs +++ b/crates/ui/src/fonts/tests.rs @@ -679,6 +679,20 @@ fn a_fetched_file_reaches_every_face_and_family_that_uses_it() { } } +#[test] +fn bytes_fetched_under_one_integrity_never_reach_a_face_requiring_another() { + const STRICT: &[ServedFace] = &[ServedFace { + integrity: "sha256-strict", + ..served("Noto Sans Arabic", "Regular", &["Arab"], "fonts/noto-sans-arabic/NotoSansArabic.ttf", 0) + }]; + let mut stack = web_fonts("en", STRICT).stack(); + let file: egui::epaint::text::Blob = Arc::new(ARABIC.to_vec()); + stack.provide_fetched("fonts/noto-sans-arabic/NotoSansArabic.ttf", "", &file); + assert!(!stack.definitions.font_data.contains_key("Noto Sans Arabic Regular"), "fetched without an integrity check"); + stack.provide_fetched("fonts/noto-sans-arabic/NotoSansArabic.ttf", "sha256-strict", &file); + assert!(stack.definitions.font_data.contains_key("Noto Sans Arabic Regular")); +} + #[test] fn fetched_cjk_faces_fit_the_latin_rows_as_embedded_ones_do() { const SC: &[ServedFace] = &[served("Noto Sans CJK SC", "Regular", &["Hans", "Latn"], "fonts/sc.otf", 0)];