From 64d45afd8a650981d4dbab98aa49d47629568f5b Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@gmail.com>
Date: Tue, 23 Jul 2024 16:14:26 +0100
Subject: [PATCH] LibWeb: Check CanvasTextDrawingStyles.font assignment is
 valid

Checking that the string parsed for the `font` property is not enough,
the spec also wants to rule out CSS-wide keywords like `inherit`. The
simplest way to do so is to check if it's a ShorthandStyleValue, which
also rules out use of `var()`; this matches other browsers' behaviour.

The newly-added test would previously crash, and now doesn't. :^)
---
 .../expected/canvas/invalid-font-property.txt     |  4 ++++
 .../Text/input/canvas/invalid-font-property.html  | 15 +++++++++++++++
 .../LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h  |  5 ++++-
 3 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibWeb/Text/expected/canvas/invalid-font-property.txt
 create mode 100644 Tests/LibWeb/Text/input/canvas/invalid-font-property.html

diff --git a/Tests/LibWeb/Text/expected/canvas/invalid-font-property.txt b/Tests/LibWeb/Text/expected/canvas/invalid-font-property.txt
new file mode 100644
index 00000000000..9db99cf0bf0
--- /dev/null
+++ b/Tests/LibWeb/Text/expected/canvas/invalid-font-property.txt
@@ -0,0 +1,4 @@
+normal normal 20px SerenitySans
+normal normal 20px SerenitySans
+normal normal 20px SerenitySans
+normal normal 20px SerenitySans
diff --git a/Tests/LibWeb/Text/input/canvas/invalid-font-property.html b/Tests/LibWeb/Text/input/canvas/invalid-font-property.html
new file mode 100644
index 00000000000..ee94df86c46
--- /dev/null
+++ b/Tests/LibWeb/Text/input/canvas/invalid-font-property.html
@@ -0,0 +1,15 @@
+<script src="../include.js"></script>
+<script>
+    test(() => {
+        let canvas = document.createElement("canvas");
+        let context = canvas.getContext("2d");
+        context.font = '20px SerenitySans';
+        println(context.font);
+        context.font = 'inherit'; // CSS-wide keywords are not allowed
+        println(context.font);
+        context.font = 'var(--font-size, 30px) Arial'; // Variables are not allowed here
+        println(context.font);
+        context.font = '!!!'; // Invalid value, should be ignored.
+        println(context.font);
+    });
+</script>
diff --git a/Userland/Libraries/LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h b/Userland/Libraries/LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h
index 6e945a82c9f..7e449848efd 100644
--- a/Userland/Libraries/LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h
+++ b/Userland/Libraries/LibWeb/HTML/Canvas/CanvasTextDrawingStyles.h
@@ -41,11 +41,14 @@ public:
         // The font IDL attribute, on setting, must be parsed as a CSS <'font'> value (but without supporting property-independent style sheet syntax like 'inherit'),
         // and the resulting font must be assigned to the context, with the 'line-height' component forced to 'normal', with the 'font-size' component converted to CSS pixels,
         // and with system fonts being computed to explicit values.
+        // FIXME: with the 'line-height' component forced to 'normal'
+        // FIXME: with the 'font-size' component converted to CSS pixels
         auto parsing_context = CSS::Parser::ParsingContext { reinterpret_cast<IncludingClass&>(*this).realm() };
         auto font_style_value_result = parse_css_value(parsing_context, font, CSS::PropertyID::Font);
 
         // If the new value is syntactically incorrect (including using property-independent style sheet syntax like 'inherit' or 'initial'), then it must be ignored, without assigning a new font value.
-        if (!font_style_value_result) {
+        // NOTE: ShorthandStyleValue should be the only valid option here. We implicitly VERIFY this below.
+        if (!font_style_value_result || !font_style_value_result->is_shorthand()) {
             return;
         }
         my_drawing_state().font_style_value = font_style_value_result.release_nonnull();