Repository navigation
Fonts: Keep font names through CSS validation, storage, and output - #13610
matiasbenedetto wants to merge 16 commits into
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Core applied text operations to CSS `font-family` values. Those operations changed a font name or made invalid CSS. A name with an apostrophe produced the invalid declaration `font-family:O'Reilly Sans;`, so the browser did not use the font. A name with a comma became two families. `sanitize_text_field()` also removed percent sequences, collapsed spaces, and stripped markup. Add `WP_CSS_Font_Family`. The class reads the CSS `font-family` grammar and returns the decoded name of each family, with its type. The serializer writes a decoded name back as a quoted CSS string. It escapes the quote character, the backslash, the control characters, and `<`, `>`, and `&`, so that a name survives HTML output and the KSES post filters. Use the class in these places: - `WP_Font_Utils::sanitize_font_family()` replaces `sanitize_text_field()`, `explode( ',' )`, and quote trimming. - `WP_Font_Utils::get_font_face_slug()` compares decoded names, so that equivalent CSS escapes produce one slug. - `WP_Font_Face_Resolver` selects the first family of a list from the parsed entries. - `WP_Font_Face` writes the `@font-face` descriptor as a quoted CSS string. - Both font REST controllers reject a `fontFamily` value that is not valid CSS and not a plain font name. - `WP_Font_Collection` sanitizes the nested `fontFace.fontFamily` value. - `safecss_filter_attr()` splits declarations with quote and escape awareness, and validates `font-family` with the font family grammar. A named family is now always quoted, and a generic family stays a keyword. For compatibility, a plain font name such as `O'Reilly Sans` still works at the font input boundaries. Core does not require a client-side escape scheme. Props matiasbenedetto. See #63568.
a5795c3 to
560da82
Compare
…ter. Use `mb_chr()` to decode a hexadecimal escape, copy the continuation bytes of an escaped character in place, and remove three private helpers. Simplify the identifier loop, and remove guards that cannot fail. In KSES, check a `font-family` value inside the allowed-property branch and clear the test string when the grammar accepts it. Remove the parenthesis depth tracking from the splitter, because a font name with a semicolon is always a quoted string. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Use bounded regular expressions and native string operations to remove duplicate parser and serializer code. Copy complete CSS declarations in the KSES splitter. Reject undefined generic arguments before CSS serialization. Add parser and REST regression tests for escaped declaration delimiters.
# Conflicts: # src/wp-includes/kses.php # tests/phpunit/tests/kses.php
Use one regex to reject CSS syntax characters and control characters in a plain font name. Remove the PLAIN_NAME_REJECTED_CHARACTERS constant. Move the escape check into consume_identifier() and remove is_valid_escape(), which had one caller. Use a regex to copy the escaped UTF-8 character in consume_escape(). The input is valid UTF-8, because parse_list() rejects invalid UTF-8. The behavior does not change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Remove the WP_CSS_Font_Family class. WP_Font_Utils now holds the parser and the serializer as these methods: - parse_font_family_list() - parse_font_family_list_with_plain_names() - parse_font_family_descriptor_name() - serialize_font_family_name() - serialize_font_family_list() The private helper methods and the constants get names that include "css" or "font family", because WP_Font_Utils also holds other methods. Move the parser tests to tests/fonts/font-library/wpFontUtils/. The behavior does not change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n name lists. Split style declarations at each semicolon again in safecss_filter_attr(), and remove _wp_kses_split_css_declarations(). The quote-aware splitter read a quote inside an unquoted url() as the start of a string. A declaration after it, such as `behavior: url(x.htc)`, then got past the allowlist. serialize_font_family_name() now writes a semicolon in a name as a CSS escape, so a split at each semicolon keeps the name. Keep a font name that is one identifier of letters and hyphens unquoted, as WordPress 6.5 did. Safari reads `-apple-system` as a system font only without quotes. Remove the quotes from a font name in get_font_face_slug(), and apply sanitize_text_field() to the other parts of the slug and to a value that the parser rejects, as WordPress 6.5 did. A font face that an earlier version saved keeps its slug, so the duplicate check finds it. Parse each entry of a font family list on its own. An entry that is not valid CSS is a plain name up to the next comma. A comma inside a quoted name no longer splits the name, and an empty entry is ignored. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`WP_Font_Utils::serialize_font_family_name()` now writes a comma as a CSS escape. Some clients split a font family list at each comma and do not read quoted strings. The Gutenberg Font Library preview function `formatFontFamily()` is one example: it read `"ACME, Sans"` as the two names `ACME` and `Sans`, so the preview used a fallback font. The decoded name does not change. The Gutenberg client already escapes the comma in the same way in `createCssString()`. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tion. Keep the original private method name `WP_Font_Face_Resolver::maybe_parse_name_from_comma_separated_list()` and change only its body. In `WP_Font_Utils::get_font_face_slug()`, call `get_font_family_comparison_key()` inside the existing lines, so that the assignment alignment of `$defaults` and `$settings` does not change. The behavior does not change. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/wp-includes/kses.php
The upload client in core sends the name from the font file as it is, for example `Bodoni*` or `Font (Display)`. The plain name path rejected the CSS syntax characters, so the REST API returned a 400 error for these names. trunk accepts them. Now a raw entry that is not valid CSS becomes one font name. Only a value with control characters is an error. The serializer escapes every character that CSS or HTML reads, so the name stays inert. Accept the font name "0" in both font REST controllers. The check for an empty required setting read "0" as a false value. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A font file can name its family with a space at the start or the end. Both upload clients trim that name, and core trims a raw name too. Test that the display name, the preset, and the face descriptor all use the trimmed name, so that the preset selects the face. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Testing evidence: 86 edge-case fonts in Chromium, Chrome, and FirefoxThese videos test all the fonts from trac-63568-edge-case-fonts.zip with the steps of the font name test guide in the description. For each case, the video does these steps:
Case 1 plays at normal speed. Cases 2–87 play at 5× speed, with a scoreboard. Each video ends with a summary table that compares each case with the guide. Setup: wordpress-develop Docker environment, Twenty Twenty-Five, Playwright 1.61.1. The core upload client is the Gutenberg build that
The table shows how many of the 57 cases that must work pass. In all nine runs, each of the 86 cases gives the result that the guide shows.
|
Add these tests for Trac #63568: - A data provider with the cases of the font name test guide in the PR description. Each raw name goes through the REST API, and the test checks the documented result: the exact name, the trimmed name, the known CSS reading, or a 400 error. The cases include non-Latin names and invisible characters. - Slug pairs that must stay different: "Font%2c Sans" and "Font, Sans", and the same letters in Unicode NFC and NFD. - HTML-like names for a user without `unfiltered_html`. KSES filters the post content for that user, so the test checks that the stored name does not change. The test fails if the serializer does not escape "&", "<", and ">". - A literal entity in the shared data set of the data path tests. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replace the five new public methods of WP_Font_Utils with two methods that return plain values: - `is_valid_css_font_family()` returns true for a valid CSS value. KSES uses it. - `get_font_face_family()` returns the quoted `@font-face` name, or an empty string. WP_Font_Face, the resolver, and the font faces controller use it. The parser, the serializers, and the keyword constants are now private, so the array of parsed entries is not a public contract. The font families controller uses `sanitize_font_family()` to check the value. The font faces controller now rejects the empty quoted name `""`. Earlier, WP_Font_Face rejected that face at output time. The tests call the private parser and serializer through reflection. See #63568. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Core changes some font names when it treats CSS values as plain text. For example,
O'Reilly Sansproduces an invalid unquoted@font-facedescriptor. This PR preserves the decoded name through font validation, storage, resolution, and CSS output.Trac ticket: https://core.trac.wordpress.org/ticket/63568
The fix also preserves commas inside quoted names, significant spaces, CSS escapes, and punctuation. It keeps a named family such as
"serif"distinct from the generic keywordserif.Implementation.
WP_Font_Utilsgets a private CSSfont-familyparser and serializer. The class adds only two public methods, and both return plain values:is_valid_css_font_family()returns true if the value is valid CSS. It requires the complete value to be a list of names and generic families, or one CSS-wide keyword.get_font_face_family()returns the first family of the value as a quoted CSS string for an@font-facedescriptor. It returns an empty string if the value names no font.The private parser has two modes:
O'Reilly Sans,O"Reilly Sans,Bodoni*, andFont (Display). A plain name can hold any text except control characters, and the serializer makes it inert. The parser ignores an empty entry, such as the entry after a trailing comma.The private serializer keeps explicit quotes around each named family. An unquoted name stays unquoted if it uses one identifier of letters and hyphens. Each other name gets quotes. The serializer escapes quotes, backslashes, control characters,
<,>,&,;, and,. It keeps the family order and the difference between names and generic keywords.The array of parsed entries is not public, so it is not an API contract.
The parser uses no recursion. It accepts only
kai,fangsong,khmer-mul, andnastaliqasgeneric()arguments. It validates the decoded argument before it emits CSS, so an escaped argument cannot introduce a second declaration.These parts of core use the shared parser:
WP_Font_Utils::sanitize_font_family()andWP_Font_Utils::get_font_face_slug().WP_Font_FaceandWP_Font_Face_Resolver.fontFamilyschema ofWP_Font_Collection."". The controllers accept the name0, which the empty check read as a false value.safecss_filter_attr().An invalid face value produces a
_doing_it_wrong()notice and no face output.KSES.
safecss_filter_attr()still splits declarations at each semicolon. If the CSS font family grammar accepts afont-familyvalue, the function skips the character checks that reject punctuation, such as a parenthesis or a backslash escape. Other values still use the current checks, and thesafecss_filter_attr_allow_cssfilter remains in the path. The serializer writes a semicolon in a name as\3b, so the split at each semicolon keeps the name.An earlier revision of this PR used a quote-aware declaration splitter. I removed it, because it read a quote inside an unquoted
url()as the start of a string. A declaration after it, such asbehavior: url(x.htc), then got past the allowlist.Compatibility.
sanitize_font_family()outputManropeManrope"Manrope""Manrope""-webkit-body""-webkit-body"-webkit-body-webkit-body-apple-system, BlinkMacSystemFont, sans-serif-apple-system, BlinkMacSystemFont, sans-serifOpen Sans, sans-serif"Open Sans", sans-serifO'Reilly Sans"O'Reilly Sans""ACME, Sans", sans-serif"ACME\2c Sans", sans-serif"serif", serif"serif", serifBodoni*"Bodoni*""A"; color:red"\"A\"\3b color:red"(one inert name)A+ U+0001 +B"-webkit-body"selects a named font instead of a browser keyword. An unquoted name stays unquoted if it uses one identifier of letters and hyphens. Each other name gets quotes. The same rule preserves the difference between"-apple-system"and-apple-system.@font-facedescriptor always uses quotes. Thusfont-family:Manropein the face output becomesfont-family:"Manrope". The preset value staysManrope."ACME\2c Sans", sans-serif, while its face descriptor contains only"ACME\2c Sans". The decoded name staysACME, Sans.formatFontFamily()read"ACME, Sans"as the two namesACMEandSans, so the preview used a fallback font. The Gutenberg client functioncreateCssString()already escapes a comma in the same way.<,>, and&so names survive HTML output and KSES post filters. Backslashes use hexadecimal escapes becausewp_kses_no_null()can remove a literal backslash before zeros. Short hexadecimal escapes retain their terminator spaces.get_font_face_slug()compares decoded names, so equivalent CSS escapes identify the same face. It keeps the WordPress 6.5 rule that removes quotation marks and apostrophes from a name. For example,O'Reilly Sansgivesoreilly sans;normal;400;100%;U+0-10FFFF. A record from an earlier version can still have a different slug.%,\,;,,,&,<, and>in a name become percent sequences. They cannot change the slug fields or thepost_title. Ordinary names such asOpen Sansretain their previous slug. If the parser rejects a value, the slug uses the WordPress 6.5 text normalization.Client dependency and limits. The upload client in core sends the raw name from the font file as
fontFamily, for exampleBodoni*. Core reads each entry as CSS first. If the entry is not valid CSS, core stores the raw text as one name. Thus most raw names work with the current client.Some raw names are also valid CSS with a different meaning, and the server cannot tell them apart. CSS reads a comma as a list,
/* */as a comment, a backslash as an escape,seriforinheritas a keyword, and joins spaces between identifiers. These names need an upload client that sends a quoted CSS string, such as Gutenberg PR #76782. Core accepts valid CSS without the special escape scheme of that PR.Display-name storage remains outside this fix. The family controller still applies
sanitize_text_field()to the display name inpost_title. A display name with markup can therefore lose that text, independently of the CSS identity.Font name test guide
This list is the reference for all tests of font names in this PR. Each case has a number. The same number is in the file name of its test font. The PHPUnit test
Tests_Fonts_FontFamilyDataPath::test_raw_name_gives_the_documented_resultsends each raw name of the list through the REST API and checks the expected result, with the case number as the name of the data set. Cases 84 and 85 hold invalid UTF-8, which a JSON request cannot carry.Test fonts. Download trac-63568-edge-case-fonts.zip. It holds one font for each case (
case-NN-….woff2) and a control font (case-00-control.woff2, nameEdge Control Sans). Each font is a copy of DM Sans Regular (SIL OFL 1.1) with a different family name in itsnametable. All the fonts have the same glyphs. Case 84 has no font, because a font name table cannot hold invalid UTF-8.Steps for one case.
O'Reilly SansandO"Reilly Sans.document.fonts.load('16px "<name>"')in the console, with the name as a CSS string. The result must hold one loaded face.fontFamilyinGET /wp/v2/font-families. It must decode to one name with the exact text of the case, so that the preset uses the font.Expected results.
emojiandfangsongas generic keywords yet. So the text renders in the uploaded font, but the stored value is still a generic keyword, and the case fails step 7. A browser that uses these keywords shows a fallback font.kebabCase(), which removes all non-Latin letters. The empty slug gives a 400 error. This needs a client fix.Earlier browser results. I ran each case with Playwright in Chromium, through the Fonts page, as in the steps above. The core upload client is the Gutenberg build that
trunkpins (5715a61), built locally. The last column used the Gutenberg plugin from PR #76782 (86cf7bad0d) with this PR at4d54df7d10, which is before the commit that accepts raw names and the name0. I ran all the cases again with this PR at88508e2613and the same Gutenberg plugin. Each case passed or failed as the last column shows. Case 44 (0) works at that revision because of that commit.The table records results from earlier revisions. I did not repeat the complete upload test at
1a7bae7460. A comparison of all 85 raw-name cases found no change in sanitizer output betweenbe5d8ec126and1a7bae7460. The selected PHPUnit groups also pass at1a7bae7460. These checks support the documented PHP behavior; they do not verify the complete upload flow at the current revision.trunkO'Reilly SansO"Reilly SansO'Reilly "Sans"Suisse BP Int'l‘Curly’ “Quotes”'Leading apostropheLeading apostrop…Trailing quote"Trailing quoteACME, SansACMEACMEA;BAA{B}A=BWhat?A:BFont (Display)Font [Beta]Font !importantDr. FontFont #1Font @HomeFont/SlashA/*c*/BA BA BBodoni*Jost*Rounded M+ 1cC++ Mono50% GrayFont 50%ABFont 50Font%2c SansFont SansFont, SansFontFontA\BA[U+000B]A[U+000B]Trailing\\003000Tom & JerryTom & JerryA<B>ATest </style> SansTest Sans</style><script>alert(1)</script><!-- x -->url(javascript:alert(1))expression(alert(1))A"; color: red; x:"A} body { color: red123450-1 Font1942 reportPress Start 2P--custom-apple-systemserifserifserifSerifserifserifsans-serifsans-serifsans-serifsystem-uisystem-uisystem-uiemojiemojiemojifangsongfangsongfangsonginheritinheritinheritINHERITinheritinheritinitialinitialinitialunsetunsetunsetrevert-layerrevert-layerrevert-layerdefaultdefaultdefaultgeneric(kai)generic(kai)generic(kai)A BA BA BLeading spaceTrailing spaceA[U+0009]BA BA BA[U+000A]BA BA BA[U+00A0]BA[U+3000]BA[U+200B]B日本語 😀微软雅黑MS ゴシックÑandúCaféCaféوزیرمتنA[U+202E]BDev 👩[U+200D]💻AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA…A\A\A\A\A\A\A\A\A\A\A\A\A\A\A\A\A\…A[U+0000]BABA[U+0001]BA\xFFBA[U+D800]BControl and invisible characters appear as
[U+XXXX]. "No face" means that the browser has no face with the name, because the@font-facerule is invalid or missing. In the earlier browser tests, all 57 cases that must work passed with this PR. The 4 cases that must be rejected returned 400. The testedtrunkrevision passed 29 of the 57.¹ The face loaded with the trimmed name, and the preset used the same name. The test measured the text width only with the untrimmed name, so these two cases have no width result. In the last column, case 83 works because that client escapes the control character.
CSS input values (53 cases)
These values come from a theme, a collection, or a REST client that sends CSS. Strict is the strict mode of the private parser, which KSES uses through
is_valid_css_font_family(). Input is the plain name mode, which the REST API, theme input, and font faces use. Each entry shows its type and its decoded value.InterInterInterOpen SansOpen SansOpen Sans"O'Reilly Sans"O'Reilly SansO'Reilly SansO'Reilly SansO'Reilly Sans'O"Reilly Sans'O"Reilly SansO"Reilly Sans"O'Reilly \"Sans\""O'Reilly "Sans"O'Reilly "Sans""ACME, Sans", sans-serifACME, Sans; genericsans-serifACME, Sans; genericsans-serifACME\,Sans, serifACME,Sans; genericserifACME,Sans; genericserif"Tom & Jerry"Tom & JerryTom & Jerry"Tom \26 Jerry"Tom & JerryTom & Jerry"Tom \000026 Jerry"Tom &JerryTom &Jerry"Tom \000026 Jerry"Tom & JerryTom & Jerry"Font 50%AB"Font 50%ABFont 50%AB"A B"A BA BA BA BA B"12345"1234512345"-1 Font"-1 Font-1 Font"What?"What?What?"A;B"A;BA;B"A{B}"A{B}A{B}"A=B"A=BA=B"A\\B"A\BA\B"O\22 Reilly Sans"O"Reilly SansO"Reilly Sans"serif", serifserif; genericserifserif; genericserif"inherit", sans-serifinherit; genericsans-serifinherit; genericsans-serifInter, generic(kai)Inter; genericgeneric(kai)Inter; genericgeneric(kai)"日本語 😀"日本語 😀日本語 😀"A<B>"A<B>A<B>Inter/* comment */, serifInter; genericserifInter; genericserif"Inter"InterInter"""0"00"A\[U+000A]B"ABAB"\41 B"ABAB"\41B"ЛЛ"\0"[U+FFFD][U+FFFD]"\110000"[U+FFFD][U+FFFD]"\D800"[U+FFFD][U+FFFD]"A\""A\"Inter,Inter-apple-system, BlinkMacSystemFont, sans-serif-apple-system; nameBlinkMacSystemFont; genericsans-serif-apple-system; nameBlinkMacSystemFont; genericsans-serif"Inter"InterInter /* openInter /* open"Inter" Bold"Inter" Bold"A"; color:red"A"; color:red"A"} body{color:red"A"} body{color:redurl(javascript:alert(1))url(javascript:alert(1))expression(alert(1))expression(alert(1))generic(\6b ai\29 ;color:red)generic(\6b ai\29 ;color:red)generic(foo)generic(foo)inherit, serifinherit; genericserifInter, , serifInter; genericserif"</style><script>alert(1)</script>"</style><script>alert(1)</script></style><script>alert(1)</script>Current automated results. These local results cover commit
1a7bae7460on PHP 8.3.31. The PHPUnit groups use a separate table prefix in the test database. A temporary test container mapsexample.comto a public address because the local DNS resolver did not resolve it. This permits the URL checks in tests that mock HTTP requests.phpunit --group fonts,kses,restapi-global-stylesphpunit -c tests/phpunit/multisite.xml --group fonts,kses,restapi-global-styles1a7bae7460PHPCompatibilityWP, PHP 7.4 and later) on the 2 source files in that commit-webkit-bodybe5d8ec126and1a7bae7460.The regression tests cover old face records, equivalent CSS escapes, duplicates across families, and records after the first batch of 100. They also permit different weights and distinct names whose old and new titles collide. The quote tests preserve named browser keywords and keep unquoted browser keywords.
GitHub checks at
1a7bae7460. Code style, PHP compatibility, and PHP static analysis passed. The PHPUnit workflow reports failures in 18 PHP 8.5 jobs. The PHP 8.5 / MySQL 8.4 job reports 231 errors from deprecatedReflectionMethod::setAccessible()calls in the font tests. I did not inspect the other failure logs. Thus the local results do not establish a pass for the complete GitHub matrix.Earlier automated results. The following results cover
be5d8ec126, exceptcomposer phpstan, which covers526821468d. These are historical results. I did not rerun the full PHPUnit suites locally at1a7bae7460.composer phpstanphpcompat.xml.dist) on the changed source filesphpunit(full suite)phpunit -c tests/phpunit/multisite.xml(full suite)Browser evidence and limits. The guide above records earlier browser results for 86 font files through the upload flow of the Fonts page. Those results do not cover
1a7bae7460. I ran the earlier complete test in Chromium 149, Chrome 153, and Firefox 151. In all three browsers, each case gave the result in the table. In Firefox, case 67 (A[U+000A]B) gives no REST request with the upload client in core, and the editor shows no notice. Chromium and Chrome send the request, and the font gets a wrong name. Both results fail, as the table shows. I did not test WebKit, including the-apple-systembehavior in Safari. The test checks that the stored value decodes to the exact name, that a face loads, and that its metrics match the control font. It does not compare the rendered glyphs in a screenshot.A comment on this PR has videos of the complete test in the three browsers. An earlier comment has videos of the upload flow at
6913a32d02.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code and Codex
Model(s): Claude Opus 5, Claude Opus 5.5, and Claude Fable 5.1; GPT-6.
Used for: The implementation, code review, code reduction, regression tests, test execution, and PR description.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
🤖 Generated with Claude Code