From a05bb9287c04e0eb34dbb7318cf771c193103aab Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 25 Sep 2026 19:46:01 +0800 Subject: [PATCH] fix(fonts): validate font set queries with Stylo --- moli-core/tests/fixtures/font-query.js | 134 ++++++++++++++++++ moli-core/tests/web_apis.rs | 3 + moli-core/tests/web_apis/font_queries.rs | 26 ++++ moli-css-parse/src/font_face.rs | 106 +++++++++++++- moli-css-parse/src/lib.rs | 2 +- .../css_fontface_runtime/font_face_set.rs | 2 +- .../font_face_set/loading.rs | 13 +- .../css_fontface_runtime/query.rs | 4 +- 8 files changed, 280 insertions(+), 10 deletions(-) create mode 100644 moli-core/tests/fixtures/font-query.js create mode 100644 moli-core/tests/web_apis/font_queries.rs diff --git a/moli-core/tests/fixtures/font-query.js b/moli-core/tests/fixtures/font-query.js new file mode 100644 index 0000000000..4fb56b99ef --- /dev/null +++ b/moli-core/tests/fixtures/font-query.js @@ -0,0 +1,134 @@ +async function() { + const rows = [], failures = []; + const cases = [ + [ + "", + "SyntaxError", + "SyntaxError" + ], + [ + "inherit", + "SyntaxError", + "SyntaxError" + ], + [ + "default", + "SyntaxError", + "SyntaxError" + ], + [ + "12px inherit", + "SyntaxError", + "SyntaxError" + ], + [ + "12px default", + "SyntaxError", + "SyntaxError" + ], + [ + "\"inherit\"", + "SyntaxError", + "SyntaxError" + ], + [ + "\"default\"", + "SyntaxError", + "SyntaxError" + ], + [ + "12px", + "SyntaxError", + "SyntaxError" + ], + [ + "serif", + "SyntaxError", + "SyntaxError" + ], + [ + "12px serif; color: red", + "SyntaxError", + "SyntaxError" + ], + [ + "-1px serif", + "SyntaxError", + "SyntaxError" + ], + [ + "var(--x) serif", + "SyntaxError", + "SyntaxError" + ], + [ + "var(--x, 10px) serif", + "SyntaxError", + "SyntaxError" + ], + [ + "env(size) serif", + "SyntaxError", + "SyntaxError" + ], + [ + "12px serif !important", + "SyntaxError", + "SyntaxError" + ], + [ + "12px serif junk,", + "SyntaxError", + "SyntaxError" + ], + [ + "normal normal normal normal 12px serif", + "loaded", + true + ], + [ + "12px \"inherit\"", + "loaded", + true + ], + [ + "12px \"default\"", + "loaded", + true + ], + [ + "12px \"revert\"", + "loaded", + true + ], + [ + "italic 700 16px/1.2 \"A B\", serif", + "loaded", + true + ], + [ + "calc(1em + 2px) serif", + "loaded", + true + ], + [ + "caption", + "loaded", + true + ], + [ + "12px serif", + "loaded", + true + ] +]; + for (const [query, expectedLoad, expectedCheck] of cases) { + const load = await document.fonts.load(query).then(() => 'loaded', error => error.name); + let check; + try { check = document.fonts.check(query); } catch (error) { check = error.name; } + const row = [query, load, check]; + rows.push(row); + if (load !== expectedLoad || check !== expectedCheck) failures.push({query, load, check, expectedLoad, expectedCheck}); + } + return {rows, failures}; +} diff --git a/moli-core/tests/web_apis.rs b/moli-core/tests/web_apis.rs index f7e364a791..9ec9d20d17 100644 --- a/moli-core/tests/web_apis.rs +++ b/moli-core/tests/web_apis.rs @@ -1,3 +1,6 @@ +#[path = "web_apis/font_queries.rs"] +mod font_queries; + #[path = "web_apis/indexed_db_transaction.rs"] mod indexed_db_transaction; diff --git a/moli-core/tests/web_apis/font_queries.rs b/moli-core/tests/web_apis/font_queries.rs new file mode 100644 index 0000000000..1f0b26bcf2 --- /dev/null +++ b/moli-core/tests/web_apis/font_queries.rs @@ -0,0 +1,26 @@ +use super::pipe_disturbed::run_probe; +use super::*; + +#[tokio::test(flavor = "multi_thread")] +async fn font_face_sets_validate_load_and_check_queries_with_css_shorthand_syntax() -> Result<()> { + let server = FixtureServer::spawn().await?; + let browser = Browser::new(AppConfig::default())?; + let fixture = include_str!("../fixtures/font-query.js"); + for target in ["window", "child"] { + let source = + format!("({fixture})().then(finish, error => finish({{error: String(error)}}));"); + let result = run_probe(&browser, &server, target, &source).await?; + assert_eq!( + result["failures"], + serde_json::json!([]), + "{target}: {result}" + ); + assert_eq!( + result["rows"].as_array().unwrap().len(), + 24, + "{target}: {result}" + ); + } + server.shutdown().await; + Ok(()) +} diff --git a/moli-css-parse/src/font_face.rs b/moli-css-parse/src/font_face.rs index f128ee77a0..88239222d0 100644 --- a/moli-css-parse/src/font_face.rs +++ b/moli-css-parse/src/font_face.rs @@ -9,8 +9,8 @@ pub fn font_load_query_contains_css_wide_keyword(query: &str) -> bool { let mut input = Parser::new(&mut input); while let Ok(token) = input.next() { match token { - Token::Ident(value) | Token::QuotedString(value) - if is_css_wide_keyword(value.as_ref()) => + Token::Ident(value) + if is_css_wide_keyword(value.as_ref()) || value.eq_ignore_ascii_case("default") => { return true; } @@ -20,6 +20,33 @@ pub fn font_load_query_contains_css_wide_keyword(query: &str) -> bool { false } +/// FontFaceSet queries use the font shorthand grammar, without cascade or +/// custom-property substitution. Keep parsing shared with CSS declarations. +pub fn font_load_query_is_valid(query: &str) -> bool { + // Servo's Stylo configuration does not parse system fonts. Their entire + // shorthand syntax is one of these six identifiers; use CSS tokens so + // escapes, comments and case folding work without accepting trailing input. + let mut input = ParserInput::new(query); + let mut parser = Parser::new(&mut input); + if let Ok(name) = parser.expect_ident_cloned() + && matches!( + name.to_ascii_lowercase().as_str(), + "caption" | "icon" | "menu" | "message-box" | "small-caption" | "status-bar" + ) + && parser.is_exhausted() + { + return true; + } + if font_load_query_contains_css_wide_keyword(query) { + return false; + } + let mut block = crate::CssDeclarationBlock::default(); + let projection = block.set_property_with_projection("font", query, false); + projection.set_result != crate::CssSetResult::ParseError + && !projection.has_unresolved_value + && !block.is_empty() +} + pub fn font_load_query_family(query: &str) -> Option { let trimmed = query.trim(); if trimmed.is_empty() { @@ -51,8 +78,8 @@ fn is_css_wide_keyword(value: &str) -> bool { #[cfg(test)] mod tests { use super::{ - font_load_query_contains_css_wide_keyword, font_load_query_family, normalize_font_face_src, - parse_font_faces, + font_load_query_contains_css_wide_keyword, font_load_query_family, + font_load_query_is_valid, normalize_font_face_src, parse_font_faces, }; #[test] @@ -105,6 +132,77 @@ mod tests { ); } + #[test] + fn font_load_query_validates_the_complete_shorthand_without_substitution() { + for query in [ + "", + "inherit", + "default", + "12px inherit", + "12px default", + r#""inherit""#, + "12px", + "serif", + "12px serif; color: red", + "-1px serif", + "var(--x) serif", + "var(--x, 10px) serif", + "env(size) serif", + "12px serif !important", + "12px serif,", + "caption garbage", + r#""caption""#, + ] { + assert!(!font_load_query_is_valid(query), "{query}"); + } + for query in [ + "12px serif", + r#"12px "inherit""#, + r#"12px "default""#, + r#"italic 700 16px/1.2 "A B", serif"#, + "calc(1em + 2px) serif", + "caption", + "ICON", + "menu", + "message-box", + "small-caption", + "status-bar", + r"c\61 ption", + "caption/**/", + "normal normal normal normal 12px serif", + ] { + assert!(font_load_query_is_valid(query), "{query}"); + } + } + + #[test] + fn font_load_query_distinguishes_reserved_identifiers_from_quoted_families() { + for keyword in [ + "inherit", + "initial", + "unset", + "default", + "revert", + "revert-layer", + ] { + for query in [keyword.to_owned(), format!("medium {keyword}")] { + assert!(font_load_query_contains_css_wide_keyword(&query), "{query}"); + } + for query in [format!("12px \"{keyword}\""), format!("12px '{keyword}'")] { + assert!( + !font_load_query_contains_css_wide_keyword(&query), + "{query}" + ); + } + } + assert!(font_load_query_contains_css_wide_keyword( + r"12px \64 efault" + )); + assert!(!font_load_query_contains_css_wide_keyword( + r#"12px "\69 nherit""# + )); + } + #[test] fn font_load_query_uses_css_tokens_for_family_and_keywords() { assert!(font_load_query_contains_css_wide_keyword( diff --git a/moli-css-parse/src/lib.rs b/moli-css-parse/src/lib.rs index eb0616dee4..bb8f207118 100644 --- a/moli-css-parse/src/lib.rs +++ b/moli-css-parse/src/lib.rs @@ -21,7 +21,7 @@ pub use color::{ pub use declaration::{CssDeclaration, DeclarationParseOptions, parse_declaration_list}; pub use font_face::{ CssFontFace, font_load_query_contains_css_wide_keyword, font_load_query_family, - normalize_font_face_src, parse_font_faces, + font_load_query_is_valid, normalize_font_face_src, parse_font_faces, }; pub use font_palette::{ CssFontPaletteValuesProperty, parse_font_palette_values_property_with_stylo, diff --git a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set.rs b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set.rs index 51232433cc..55cfe9a0be 100644 --- a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set.rs +++ b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set.rs @@ -1,6 +1,6 @@ use super::events::dispatch_font_face_set_event; use super::query::{ - font_face_set_matching_faces_array, font_load_query_contains_css_wide_keyword, + font_face_set_matching_faces_array, font_load_query_is_valid, make_rejected_dom_exception_promise, }; use super::storage::{ diff --git a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set/loading.rs b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set/loading.rs index 3165e93635..a1fa7ab8e2 100644 --- a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set/loading.rs +++ b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/font_face_set/loading.rs @@ -28,7 +28,16 @@ pub(in crate::context_bootstrap) fn font_face_set_check_callback<'s>( let Some(parsed) = webidl::parse_args::(scope, &args) else { return; }; - let _ = (&parsed.font, &parsed.text); + let _ = &parsed.text; + if !font_load_query_is_valid(&parsed.font) { + let error = new_dom_exception_value( + scope, + "The provided font shorthand is invalid.", + "SyntaxError", + ); + scope.throw_exception(error); + return; + } rv.set(v8::Boolean::new(scope, true).into()); } @@ -43,7 +52,7 @@ pub(in crate::context_bootstrap) fn font_face_set_load_callback<'s>( return; }; let _ = &parsed.text; - if font_load_query_contains_css_wide_keyword(&parsed.font) { + if !font_load_query_is_valid(&parsed.font) { rv.set( make_rejected_dom_exception_promise( scope, diff --git a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/query.rs b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/query.rs index b25ed8632f..f359f9840a 100644 --- a/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/query.rs +++ b/moli-renderer-v8/src/context_bootstrap/css_fontface_runtime/query.rs @@ -2,8 +2,8 @@ use super::storage::font_face_set_faces_array; use super::*; use crate::util::serialize_v8_iter_array; -pub(super) fn font_load_query_contains_css_wide_keyword(query: &str) -> bool { - moli_css_parse::font_load_query_contains_css_wide_keyword(query) +pub(super) fn font_load_query_is_valid(query: &str) -> bool { + moli_css_parse::font_load_query_is_valid(query) } fn font_face_matches_query(