From 52bb95fe572b7a295861f94295336b571f2ce7b9 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Fri, 21 Mar 2025 01:24:34 +0000 Subject: [PATCH] LibWeb: Don't crash when `border-spacing` is set to a `calc()` value Previously, the browser would crash if the `border-spacing` property had 2 lengths and either one of these was set to a `calc()` value. --- Libraries/LibWeb/CSS/ComputedProperties.cpp | 50 +++++++++++++------ .../Crash/CSS/table-border-spacing-calc.html | 7 +++ 2 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 Tests/LibWeb/Crash/CSS/table-border-spacing-calc.html diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index ad4c21b06ae..96b24795d9c 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -488,24 +488,46 @@ ImageRendering ComputedProperties::image_rendering() const Length ComputedProperties::border_spacing_horizontal(Layout::Node const& layout_node) const { - auto const& value = property(PropertyID::BorderSpacing); - if (value.is_length()) - return value.as_length().length(); - if (value.is_calculated()) - return value.as_calculated().resolve_length({ .length_resolution_context = Length::ResolutionContext::for_layout_node(layout_node) }).value_or(Length(0, Length::Type::Px)); - auto const& list = value.as_value_list(); - return list.value_at(0, false)->as_length().length(); + auto resolve_value = [&](auto const& style_value) -> Optional { + if (style_value.is_length()) + return style_value.as_length().length(); + if (style_value.is_calculated()) + return style_value.as_calculated().resolve_length({ .length_resolution_context = Length::ResolutionContext::for_layout_node(layout_node) }).value_or(Length(0, Length::Type::Px)); + return {}; + }; + + auto const& style_value = property(PropertyID::BorderSpacing); + auto resolved_value = resolve_value(style_value); + if (!resolved_value.has_value()) { + auto const& list = style_value.as_value_list(); + VERIFY(list.size() > 0); + resolved_value = resolve_value(*list.value_at(0, false)); + } + + VERIFY(resolved_value.has_value()); + return *resolved_value; } Length ComputedProperties::border_spacing_vertical(Layout::Node const& layout_node) const { - auto const& value = property(PropertyID::BorderSpacing); - if (value.is_length()) - return value.as_length().length(); - if (value.is_calculated()) - return value.as_calculated().resolve_length({ .length_resolution_context = Length::ResolutionContext::for_layout_node(layout_node) }).value_or(Length(0, Length::Type::Px)); - auto const& list = value.as_value_list(); - return list.value_at(1, false)->as_length().length(); + auto resolve_value = [&](auto const& style_value) -> Optional { + if (style_value.is_length()) + return style_value.as_length().length(); + if (style_value.is_calculated()) + return style_value.as_calculated().resolve_length({ .length_resolution_context = Length::ResolutionContext::for_layout_node(layout_node) }).value_or(Length(0, Length::Type::Px)); + return {}; + }; + + auto const& style_value = property(PropertyID::BorderSpacing); + auto resolved_value = resolve_value(style_value); + if (!resolved_value.has_value()) { + auto const& list = style_value.as_value_list(); + VERIFY(list.size() > 1); + resolved_value = resolve_value(*list.value_at(1, false)); + } + + VERIFY(resolved_value.has_value()); + return *resolved_value; } CaptionSide ComputedProperties::caption_side() const diff --git a/Tests/LibWeb/Crash/CSS/table-border-spacing-calc.html b/Tests/LibWeb/Crash/CSS/table-border-spacing-calc.html new file mode 100644 index 00000000000..f44eb8e6ad0 --- /dev/null +++ b/Tests/LibWeb/Crash/CSS/table-border-spacing-calc.html @@ -0,0 +1,7 @@ + + +