From a1a4be2a38cd3178274a76d452a67dd15e269a38 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 14 Feb 2023 18:47:04 -0600 Subject: Newtype PangoUnits(i32): used to check for overflow when converting to i32 pango units Part-of: --- src/text.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/text.rs b/src/text.rs index a02b5077..3ca8ec9f 100644 --- a/src/text.rs +++ b/src/text.rs @@ -340,6 +340,21 @@ impl Span { } } +/// Use as `PangoUnits::from_pixels()` so that we can check for overflow. +struct PangoUnits(i32); + +impl PangoUnits { + fn from_pixels(v: f64) -> Option { + // We want (v * f64::from(pango::SCALE) + 0.5) as i32 + // + // But check for overflow. + + cast::i32(v * f64::from(pango::SCALE) + 0.5) + .ok() + .map(PangoUnits) + } +} + impl MeasuredSpan { fn from_span(layout_context: &LayoutContext, span: &Span) -> MeasuredSpan { let values = span.values.clone(); @@ -1410,4 +1425,14 @@ mod tests { (0.0, -4.0) ); } + + #[test] + fn pango_units_works() { + assert_eq!(PangoUnits::from_pixels(10.0).unwrap().0, pango::SCALE * 10); + } + + #[test] + fn pango_units_detects_overflow() { + assert!(PangoUnits::from_pixels(1e7).is_none()); + } } -- cgit v1.2.1 From be70ba1e182b425f711b308bf30e6ecccde7163d Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 14 Feb 2023 19:09:34 -0600 Subject: MeasuredSpan::from_span(): Check that the font-size and letter-spacing are convertible to Pango units Otherwise, ignore the span - return None and have the caller ignore it. Now, this is a good place to actually impose a limit on font sizes. What should those be...? Part-of: --- src/text.rs | 94 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 62 insertions(+), 32 deletions(-) diff --git a/src/text.rs b/src/text.rs index 3ca8ec9f..24fa12d1 100644 --- a/src/text.rs +++ b/src/text.rs @@ -144,7 +144,7 @@ impl MeasuredChunk { let mut measured_spans: Vec = chunk .spans .iter() - .map(|span| MeasuredSpan::from_span(layout_context, span)) + .filter_map(|span| MeasuredSpan::from_span(layout_context, span)) .collect(); // The first span contains the (dx, dy) that will be applied to the whole chunk. @@ -356,7 +356,7 @@ impl PangoUnits { } impl MeasuredSpan { - fn from_span(layout_context: &LayoutContext, span: &Span) -> MeasuredSpan { + fn from_span(layout_context: &LayoutContext, span: &Span) -> Option { let values = span.values.clone(); let params = NormalizeParams::new(&values, &layout_context.view_params); @@ -369,30 +369,35 @@ impl MeasuredSpan { ); let with_control_chars = wrap_with_direction_control_chars(&span.text, &bidi_control); - let layout = create_pango_layout(layout_context, &properties, &with_control_chars); - let (w, h) = layout.size(); - let w = f64::from(w) / f64::from(pango::SCALE); - let h = f64::from(h) / f64::from(pango::SCALE); + if let Some(layout) = create_pango_layout(layout_context, &properties, &with_control_chars) + { + let (w, h) = layout.size(); - // This is the logical size of the layout, regardless of text direction, so it's always positive. - assert!(w >= 0.0); - assert!(h >= 0.0); + let w = f64::from(w) / f64::from(pango::SCALE); + let h = f64::from(h) / f64::from(pango::SCALE); - let advance = if layout_context.writing_mode.is_horizontal() { - (w, 0.0) - } else { - (0.0, w) - }; + // This is the logical size of the layout, regardless of text direction, so it's always positive. + assert!(w >= 0.0); + assert!(h >= 0.0); - MeasuredSpan { - values, - layout, - layout_size: (w, h), - advance, - dx: span.dx, - dy: span.dy, - link_target: span.link_target.clone(), + let advance = if layout_context.writing_mode.is_horizontal() { + (w, 0.0) + } else { + (0.0, w) + }; + + Some(MeasuredSpan { + values, + layout, + layout_size: (w, h), + advance, + dx: span.dx, + dy: span.dy, + link_target: span.link_target.clone(), + }) + } else { + None } } } @@ -1206,11 +1211,12 @@ fn wrap_with_direction_control_chars(s: &str, bidi_control: &BidiControl) -> Str res } +/// Returns `None` if the layout would be invalid due to, for example, out-of-bounds font sizes. fn create_pango_layout( layout_context: &LayoutContext, props: &FontProperties, text: &str, -) -> pango::Layout { +) -> Option { let pango_context = create_pango_context(&layout_context.font_options, &layout_context.transform); @@ -1236,14 +1242,37 @@ fn create_pango_layout( let layout = pango::Layout::new(&pango_context); - let attr_list = pango::AttrList::new(); - add_pango_attributes(&attr_list, props, 0, text.len()); + let font_size = PangoUnits::from_pixels(props.font_size); + let letter_spacing = PangoUnits::from_pixels(props.letter_spacing); - layout.set_attributes(Some(&attr_list)); - layout.set_text(text); - layout.set_auto_dir(false); + if font_size.is_none() { + rsvg_log!( + &layout_context.session, + "font-size {} is out of bounds; ignoring span", + props.font_size + ); + } + + if letter_spacing.is_none() { + rsvg_log!( + &layout_context.session, + "letter-spacing {} is out of bounds; ignoring span", + props.letter_spacing + ); + } + + if let (Some(font_size), Some(letter_spacing)) = (font_size, letter_spacing) { + let attr_list = pango::AttrList::new(); + add_pango_attributes(&attr_list, props, 0, text.len(), font_size, letter_spacing); + + layout.set_attributes(Some(&attr_list)); + layout.set_text(text); + layout.set_auto_dir(false); - layout + Some(layout) + } else { + None + } } /// Adds Pango attributes, suitable for a span of text, to an `AttrList`. @@ -1252,6 +1281,8 @@ fn add_pango_attributes( props: &FontProperties, start_index: usize, end_index: usize, + font_size: PangoUnits, + letter_spacing: PangoUnits, ) { let start_index = u32::try_from(start_index).expect("Pango attribute index must fit in u32"); let end_index = u32::try_from(end_index).expect("Pango attribute index must fit in u32"); @@ -1270,12 +1301,11 @@ fn add_pango_attributes( font_desc.set_weight(pango::Weight::from(props.font_weight)); font_desc.set_stretch(pango::Stretch::from(props.font_stretch)); - font_desc.set_size(to_pango_units(props.font_size)); + font_desc.set_size(font_size.0); attributes.push(pango::AttrFontDesc::new(&font_desc).upcast()); - attributes - .push(pango::AttrInt::new_letter_spacing(to_pango_units(props.letter_spacing)).upcast()); + attributes.push(pango::AttrInt::new_letter_spacing(letter_spacing.0).upcast()); if props.text_decoration.overline { attributes.push(pango::AttrInt::new_overline(pango::Overline::Single).upcast()); -- cgit v1.2.1 From fd9f7dbb495b282b1a295cc57c9a66cb2f1a703d Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 14 Feb 2023 19:10:44 -0600 Subject: Remove unused function Part-of: --- src/text.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/text.rs b/src/text.rs index 24fa12d1..e64fdf04 100644 --- a/src/text.rs +++ b/src/text.rs @@ -1025,10 +1025,6 @@ impl SetAttributes for TSpan { impl Draw for TSpan {} -fn to_pango_units(v: f64) -> i32 { - (v * f64::from(pango::SCALE) + 0.5) as i32 -} - impl From for pango::Style { fn from(s: FontStyle) -> pango::Style { match s { -- cgit v1.2.1 From ec25e6bc37bfd0b4ed1976f7c9ebab6b17330470 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 14 Feb 2023 19:19:33 -0600 Subject: Remove assertion that I used to check my understanding of Pango's sizes Yes, the sizes returned from pango_layout_get_size() are positive if Pango doesn't overflow its i32 values. The assertions were there from e4dc9bacdb528ce03109d8f98d1d7a7c88d6416e when I was checking what Pango does for RTL text. Part-of: --- src/text.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/text.rs b/src/text.rs index e64fdf04..e209d8b2 100644 --- a/src/text.rs +++ b/src/text.rs @@ -377,10 +377,6 @@ impl MeasuredSpan { let w = f64::from(w) / f64::from(pango::SCALE); let h = f64::from(h) / f64::from(pango::SCALE); - // This is the logical size of the layout, regardless of text direction, so it's always positive. - assert!(w >= 0.0); - assert!(h >= 0.0); - let advance = if layout_context.writing_mode.is_horizontal() { (w, 0.0) } else { -- cgit v1.2.1 From 965ecc6c8a79f7724c6f1c2163f8d5fafd5ce3c4 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Tue, 14 Feb 2023 19:21:25 -0600 Subject: Add test for #932 Part-of: --- tests/fixtures/render-crash/bug932-too-big-font-size.svg | 9 +++++++++ tests/src/render_crash.rs | 1 + 2 files changed, 10 insertions(+) create mode 100644 tests/fixtures/render-crash/bug932-too-big-font-size.svg diff --git a/tests/fixtures/render-crash/bug932-too-big-font-size.svg b/tests/fixtures/render-crash/bug932-too-big-font-size.svg new file mode 100644 index 00000000..14796be6 --- /dev/null +++ b/tests/fixtures/render-crash/bug932-too-big-font-size.svg @@ -0,0 +1,9 @@ + + + + + + A + + A + diff --git a/tests/src/render_crash.rs b/tests/src/render_crash.rs index da58192a..9a2923c9 100644 --- a/tests/src/render_crash.rs +++ b/tests/src/render_crash.rs @@ -63,6 +63,7 @@ mod tests { t!(bug721_pattern_cycle_from_other_child_svg, "bug721-pattern-cycle-from-other-child.svg"); t!(bug777155_zero_sized_pattern_svg, "bug777155-zero-sized-pattern.svg"); t!(bug928_empty_fetile_bounds_svg, "bug928-empty-feTile-bounds.svg"); + t!(bug932_too_big_font_size, "bug932-too-big-font-size.svg"); t!(femerge_color_interpolation_srgb_svg, "feMerge-color-interpolation-srgb.svg"); t!(filters_non_invertible_paffine_svg, "filters-non-invertible-paffine.svg"); t!(gradient_with_empty_bbox_svg, "gradient-with-empty-bbox.svg"); -- cgit v1.2.1