From fd745a9f4cb3ba81623167c9d1117747353db33a Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sat, 2 Nov 2024 20:05:51 +0000 Subject: [PATCH] Fix racing condition in hint triggering This fixes an issue with hints where it was possible that the terminal content of highlighted hints changed between the highlighted hint update and the activation of the hint. This patch always validates the hint's text content against the hint itself to ensure that the content is still valid for the original hint which triggered the highlight. Closes #8277. --- CHANGELOG.md | 1 + alacritty/src/config/bindings.rs | 11 +++--- alacritty/src/config/font.rs | 2 +- alacritty/src/config/ui_config.rs | 8 ++--- alacritty/src/display/color.rs | 6 ++-- alacritty/src/display/content.rs | 6 ++-- alacritty/src/display/damage.rs | 2 +- alacritty/src/display/hint.rs | 52 +++++++++++++++++++-------- alacritty/src/display/meter.rs | 2 +- alacritty/src/event.rs | 8 ++--- alacritty/src/input/mod.rs | 2 +- alacritty/src/migrate/mod.rs | 2 +- alacritty/src/renderer/text/gles2.rs | 6 ++-- alacritty/src/renderer/text/glsl3.rs | 6 ++-- alacritty/src/renderer/text/mod.rs | 2 +- alacritty/src/string.rs | 2 +- alacritty_terminal/src/grid/mod.rs | 2 +- alacritty_terminal/src/term/mod.rs | 2 +- alacritty_terminal/src/term/search.rs | 2 +- 19 files changed, 74 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c28ddd1..bbe0d2f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Notable changes to the `alacritty_terminal` crate are documented in its ### Fixed - Mouse/Vi cursor hint highlighting broken on the terminal cursor line +- Hint launcher opening arbitrary text, when terminal content changed while opening ## 0.14.0 diff --git a/alacritty/src/config/bindings.rs b/alacritty/src/config/bindings.rs index dfe31853..931b0583 100644 --- a/alacritty/src/config/bindings.rs +++ b/alacritty/src/config/bindings.rs @@ -5,6 +5,7 @@ use std::fmt::{self, Debug, Display}; use bitflags::bitflags; use serde::de::{self, Error as SerdeError, MapAccess, Unexpected, Visitor}; use serde::{Deserialize, Deserializer}; +use std::rc::Rc; use toml::Value as SerdeValue; use winit::event::MouseButton; use winit::keyboard::{ @@ -96,7 +97,7 @@ pub enum Action { /// Regex keyboard hints. #[config(skip)] - Hint(Hint), + Hint(Rc), /// Move vi mode cursor. #[config(skip)] @@ -790,7 +791,7 @@ impl<'a> Deserialize<'a> for ModeWrapper { { struct ModeVisitor; - impl<'a> Visitor<'a> for ModeVisitor { + impl Visitor<'_> for ModeVisitor { type Value = ModeWrapper; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -844,7 +845,7 @@ impl<'a> Deserialize<'a> for MouseButtonWrapper { { struct MouseButtonVisitor; - impl<'a> Visitor<'a> for MouseButtonVisitor { + impl Visitor<'_> for MouseButtonVisitor { type Value = MouseButtonWrapper; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -956,7 +957,7 @@ impl<'a> Deserialize<'a> for RawBinding { { struct FieldVisitor; - impl<'a> Visitor<'a> for FieldVisitor { + impl Visitor<'_> for FieldVisitor { type Value = Field; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -1204,7 +1205,7 @@ impl<'a> de::Deserialize<'a> for ModsWrapper { { struct ModsVisitor; - impl<'a> Visitor<'a> for ModsVisitor { + impl Visitor<'_> for ModsVisitor { type Value = ModsWrapper; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/alacritty/src/config/font.rs b/alacritty/src/config/font.rs index 061c0f42..760c26d5 100644 --- a/alacritty/src/config/font.rs +++ b/alacritty/src/config/font.rs @@ -144,7 +144,7 @@ impl<'de> Deserialize<'de> for Size { D: Deserializer<'de>, { struct NumVisitor; - impl<'v> Visitor<'v> for NumVisitor { + impl Visitor<'_> for NumVisitor { type Value = Size; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/alacritty/src/config/ui_config.rs b/alacritty/src/config/ui_config.rs index 69716dee..b44bda0d 100644 --- a/alacritty/src/config/ui_config.rs +++ b/alacritty/src/config/ui_config.rs @@ -253,7 +253,7 @@ pub struct Hints { alphabet: HintsAlphabet, /// All configured terminal hints. - pub enabled: Vec, + pub enabled: Vec>, } impl Default for Hints { @@ -274,7 +274,7 @@ impl Default for Hints { }); Self { - enabled: vec![Hint { + enabled: vec![Rc::new(Hint { content, action, persist: false, @@ -288,7 +288,7 @@ impl Default for Hints { mods: ModsWrapper(ModifiersState::SHIFT | ModifiersState::CONTROL), mode: Default::default(), }), - }], + })], alphabet: Default::default(), } } @@ -619,7 +619,7 @@ impl SerdeReplace for Program { } pub(crate) struct StringVisitor; -impl<'de> serde::de::Visitor<'de> for StringVisitor { +impl serde::de::Visitor<'_> for StringVisitor { type Value = String; fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/alacritty/src/display/color.rs b/alacritty/src/display/color.rs index 669bf502..2e854a3f 100644 --- a/alacritty/src/display/color.rs +++ b/alacritty/src/display/color.rs @@ -18,7 +18,7 @@ pub const DIM_FACTOR: f32 = 0.66; #[derive(Copy, Clone)] pub struct List([Rgb; COUNT]); -impl<'a> From<&'a Colors> for List { +impl From<&'_ Colors> for List { fn from(colors: &Colors) -> List { // Type inference fails without this annotation. let mut list = List([Rgb::default(); COUNT]); @@ -235,7 +235,7 @@ impl<'de> Deserialize<'de> for Rgb { b: u8, } - impl<'a> Visitor<'a> for RgbVisitor { + impl Visitor<'_> for RgbVisitor { type Value = Rgb; fn expecting(&self, f: &mut Formatter<'_>) -> fmt::Result { @@ -331,7 +331,7 @@ impl<'de> Deserialize<'de> for CellRgb { const EXPECTING: &str = "CellForeground, CellBackground, or hex color like #ff00ff"; struct CellRgbVisitor; - impl<'a> Visitor<'a> for CellRgbVisitor { + impl Visitor<'_> for CellRgbVisitor { type Value = CellRgb; fn expecting(&self, f: &mut Formatter<'_>) -> fmt::Result { diff --git a/alacritty/src/display/content.rs b/alacritty/src/display/content.rs index 2fbbdec4..edd2709f 100644 --- a/alacritty/src/display/content.rs +++ b/alacritty/src/display/content.rs @@ -150,7 +150,7 @@ impl<'a> RenderableContent<'a> { } } -impl<'a> Iterator for RenderableContent<'a> { +impl Iterator for RenderableContent<'_> { type Item = RenderableCell; /// Gets the next renderable cell. @@ -453,7 +453,7 @@ struct Hint<'a> { labels: &'a Vec>, } -impl<'a> Hint<'a> { +impl Hint<'_> { /// Advance the hint iterator. /// /// If the point is within a hint, the keyboard shortcut character that should be displayed at @@ -543,7 +543,7 @@ impl<'a> HintMatches<'a> { } } -impl<'a> Deref for HintMatches<'a> { +impl Deref for HintMatches<'_> { type Target = [Match]; fn deref(&self) -> &Self::Target { diff --git a/alacritty/src/display/damage.rs b/alacritty/src/display/damage.rs index fc6aef39..b0736375 100644 --- a/alacritty/src/display/damage.rs +++ b/alacritty/src/display/damage.rs @@ -251,7 +251,7 @@ impl<'a> RenderDamageIterator<'a> { } } -impl<'a> Iterator for RenderDamageIterator<'a> { +impl Iterator for RenderDamageIterator<'_> { type Item = Rect; fn next(&mut self) -> Option { diff --git a/alacritty/src/display/hint.rs b/alacritty/src/display/hint.rs index a01a1d03..3f10b4e5 100644 --- a/alacritty/src/display/hint.rs +++ b/alacritty/src/display/hint.rs @@ -1,6 +1,8 @@ +use std::borrow::Cow; use std::cmp::Reverse; use std::collections::HashSet; use std::iter; +use std::rc::Rc; use ahash::RandomState; use winit::keyboard::ModifiersState; @@ -23,7 +25,7 @@ const HINT_SPLIT_PERCENTAGE: f32 = 0.5; /// Keyboard regex hint state. pub struct HintState { /// Hint currently in use. - hint: Option, + hint: Option>, /// Alphabet for hint labels. alphabet: String, @@ -56,7 +58,7 @@ impl HintState { } /// Start the hint selection process. - pub fn start(&mut self, hint: Hint) { + pub fn start(&mut self, hint: Rc) { self.hint = Some(hint); } @@ -150,7 +152,7 @@ impl HintState { // Check if the selected label is fully matched. if label.len() == 1 { let bounds = self.matches[index].clone(); - let action = hint.action.clone(); + let hint = hint.clone(); // Exit hint mode unless it requires explicit dismissal. if hint.persist { @@ -161,7 +163,7 @@ impl HintState { // Hyperlinks take precedence over regex matches. let hyperlink = term.grid()[*bounds.start()].hyperlink(); - Some(HintMatch { action, bounds, hyperlink }) + Some(HintMatch { bounds, hyperlink, hint }) } else { // Store character to preserve the selection. self.keys.push(c); @@ -192,13 +194,14 @@ impl HintState { /// Hint match which was selected by the user. #[derive(PartialEq, Eq, Debug, Clone)] pub struct HintMatch { - /// Action for handling the text. - action: HintAction, - /// Terminal range matching the hint. bounds: Match, + /// OSC 8 hyperlink. hyperlink: Option, + + /// Hint which triggered this match. + hint: Rc, } impl HintMatch { @@ -210,7 +213,7 @@ impl HintMatch { #[inline] pub fn action(&self) -> &HintAction { - &self.action + &self.hint.action } #[inline] @@ -221,6 +224,29 @@ impl HintMatch { pub fn hyperlink(&self) -> Option<&Hyperlink> { self.hyperlink.as_ref() } + + /// Get the text content of the hint match. + /// + /// This will always revalidate the hint text, to account for terminal content + /// changes since the [`HintMatch`] was constructed. The text of the hint might + /// be different from its original value, but it will **always** be a valid + /// match for this hint. + pub fn text(&self, term: &Term) -> Option> { + // Revalidate hyperlink match. + if let Some(hyperlink) = &self.hyperlink { + let (validated, bounds) = hyperlink_at(term, *self.bounds.start())?; + return (&validated == hyperlink && bounds == self.bounds) + .then(|| hyperlink.uri().into()); + } + + // Revalidate regex match. + let regex = self.hint.content.regex.as_ref()?; + let bounds = regex.with_compiled(|regex| { + regex_match_at(term, *self.bounds.start(), regex, self.hint.post_processing) + })??; + (bounds == self.bounds) + .then(|| term.bounds_to_string(*bounds.start(), *bounds.end()).into()) + } } /// Generator for creating new hint labels. @@ -382,18 +408,14 @@ pub fn highlighted_at( if let Some((hyperlink, bounds)) = hint.content.hyperlinks.then(|| hyperlink_at(term, point)).flatten() { - return Some(HintMatch { - bounds, - action: hint.action.clone(), - hyperlink: Some(hyperlink), - }); + return Some(HintMatch { bounds, hyperlink: Some(hyperlink), hint: hint.clone() }); } let bounds = hint.content.regex.as_ref().and_then(|regex| { regex.with_compiled(|regex| regex_match_at(term, point, regex, hint.post_processing)) }); if let Some(bounds) = bounds.flatten() { - return Some(HintMatch { bounds, action: hint.action.clone(), hyperlink: None }); + return Some(HintMatch { bounds, hint: hint.clone(), hyperlink: None }); } None @@ -554,7 +576,7 @@ impl<'a, T> HintPostProcessor<'a, T> { } } -impl<'a, T> Iterator for HintPostProcessor<'a, T> { +impl Iterator for HintPostProcessor<'_, T> { type Item = Match; fn next(&mut self) -> Option { diff --git a/alacritty/src/display/meter.rs b/alacritty/src/display/meter.rs index e263100f..20cbbd1b 100644 --- a/alacritty/src/display/meter.rs +++ b/alacritty/src/display/meter.rs @@ -57,7 +57,7 @@ impl<'a> Sampler<'a> { } } -impl<'a> Drop for Sampler<'a> { +impl Drop for Sampler<'_> { fn drop(&mut self) { self.meter.add_sample(self.alive_duration()); } diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs index f0159060..e82d7974 100644 --- a/alacritty/src/event.rs +++ b/alacritty/src/event.rs @@ -1141,16 +1141,16 @@ impl<'a, N: Notify + 'a, T: EventListener> input::ActionContext for ActionCon } let hint_bounds = hint.bounds(); - let text = match hint.hyperlink() { - Some(hyperlink) => hyperlink.uri().to_owned(), - None => self.terminal.bounds_to_string(*hint_bounds.start(), *hint_bounds.end()), + let text = match hint.text(self.terminal) { + Some(text) => text, + None => return, }; match &hint.action() { // Launch an external program. HintAction::Command(command) => { let mut args = command.args().to_vec(); - args.push(text); + args.push(text.into()); self.spawn_daemon(command.program(), &args); }, // Copy the text to the clipboard. diff --git a/alacritty/src/input/mod.rs b/alacritty/src/input/mod.rs index bbd8673f..1c9d6008 100644 --- a/alacritty/src/input/mod.rs +++ b/alacritty/src/input/mod.rs @@ -1136,7 +1136,7 @@ mod tests { inline_search_state: &'a mut InlineSearchState, } - impl<'a, T: EventListener> super::ActionContext for ActionContext<'a, T> { + impl super::ActionContext for ActionContext<'_, T> { fn search_next( &mut self, _origin: Point, diff --git a/alacritty/src/migrate/mod.rs b/alacritty/src/migrate/mod.rs index 2f806d36..42b51d27 100644 --- a/alacritty/src/migrate/mod.rs +++ b/alacritty/src/migrate/mod.rs @@ -244,7 +244,7 @@ enum Migration<'a> { Yaml((&'a Path, String)), } -impl<'a> Migration<'a> { +impl Migration<'_> { /// Get the success message for this migration. fn success_message(&self, import: bool) -> String { match self { diff --git a/alacritty/src/renderer/text/gles2.rs b/alacritty/src/renderer/text/gles2.rs index 427a60e3..8756ea80 100644 --- a/alacritty/src/renderer/text/gles2.rs +++ b/alacritty/src/renderer/text/gles2.rs @@ -346,7 +346,7 @@ pub struct RenderApi<'a> { dual_source_blending: bool, } -impl<'a> Drop for RenderApi<'a> { +impl Drop for RenderApi<'_> { fn drop(&mut self) { if !self.batch.is_empty() { self.render_batch(); @@ -354,7 +354,7 @@ impl<'a> Drop for RenderApi<'a> { } } -impl<'a> LoadGlyph for RenderApi<'a> { +impl LoadGlyph for RenderApi<'_> { fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph { Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized) } @@ -364,7 +364,7 @@ impl<'a> LoadGlyph for RenderApi<'a> { } } -impl<'a> TextRenderApi for RenderApi<'a> { +impl TextRenderApi for RenderApi<'_> { fn batch(&mut self) -> &mut Batch { self.batch } diff --git a/alacritty/src/renderer/text/glsl3.rs b/alacritty/src/renderer/text/glsl3.rs index 7c32bf9f..fee95ce3 100644 --- a/alacritty/src/renderer/text/glsl3.rs +++ b/alacritty/src/renderer/text/glsl3.rs @@ -215,7 +215,7 @@ pub struct RenderApi<'a> { program: &'a mut TextShaderProgram, } -impl<'a> TextRenderApi for RenderApi<'a> { +impl TextRenderApi for RenderApi<'_> { fn batch(&mut self) -> &mut Batch { self.batch } @@ -261,7 +261,7 @@ impl<'a> TextRenderApi for RenderApi<'a> { } } -impl<'a> LoadGlyph for RenderApi<'a> { +impl LoadGlyph for RenderApi<'_> { fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph { Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized) } @@ -271,7 +271,7 @@ impl<'a> LoadGlyph for RenderApi<'a> { } } -impl<'a> Drop for RenderApi<'a> { +impl Drop for RenderApi<'_> { fn drop(&mut self) { if !self.batch.is_empty() { self.render_batch(); diff --git a/alacritty/src/renderer/text/mod.rs b/alacritty/src/renderer/text/mod.rs index 886b7f8b..acd1b521 100644 --- a/alacritty/src/renderer/text/mod.rs +++ b/alacritty/src/renderer/text/mod.rs @@ -186,7 +186,7 @@ pub struct LoaderApi<'a> { current_atlas: &'a mut usize, } -impl<'a> LoadGlyph for LoaderApi<'a> { +impl LoadGlyph for LoaderApi<'_> { fn load_glyph(&mut self, rasterized: &RasterizedGlyph) -> Glyph { Atlas::load_glyph(self.active_tex, self.atlas, self.current_atlas, rasterized) } diff --git a/alacritty/src/string.rs b/alacritty/src/string.rs index a7af4394..b8c47d3b 100644 --- a/alacritty/src/string.rs +++ b/alacritty/src/string.rs @@ -106,7 +106,7 @@ impl<'a> StrShortener<'a> { } } -impl<'a> Iterator for StrShortener<'a> { +impl Iterator for StrShortener<'_> { type Item = char; fn next(&mut self) -> Option { diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index 278bea3b..fbd2c79e 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -615,7 +615,7 @@ pub trait BidirectionalIterator: Iterator { fn prev(&mut self) -> Option; } -impl<'a, T> BidirectionalIterator for GridIterator<'a, T> { +impl BidirectionalIterator for GridIterator<'_, T> { fn prev(&mut self) -> Option { let topmost_line = self.grid.topmost_line(); let last_column = self.grid.last_column(); diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index 8a0410dc..1f1e52c1 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -199,7 +199,7 @@ impl<'a> TermDamageIterator<'a> { } } -impl<'a> Iterator for TermDamageIterator<'a> { +impl Iterator for TermDamageIterator<'_> { type Item = LineDamageBounds; fn next(&mut self) -> Option { diff --git a/alacritty_terminal/src/term/search.rs b/alacritty_terminal/src/term/search.rs index 33f6ee05..20a427b7 100644 --- a/alacritty_terminal/src/term/search.rs +++ b/alacritty_terminal/src/term/search.rs @@ -656,7 +656,7 @@ impl<'a, T> RegexIter<'a, T> { } } -impl<'a, T> Iterator for RegexIter<'a, T> { +impl Iterator for RegexIter<'_, T> { type Item = Match; fn next(&mut self) -> Option {