Bug 261508

Summary: Loading disallowed FontFace from ArrayBuffer in Lockdown Mode doesn't reject promise
Product: WebKit Reporter: pajowu
Component: TextAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: bfulgham, mmaxfield, ntim, pajowu, vitor.roriz, webkit-bug-importer, wenson_hsieh, wilander
Priority: P2 Keywords: InRadar
Version: Safari 16   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Minimal Reproducing Example none

Description pajowu 2023-09-13 06:24:07 PDT 
Created attachment 467671 [details]
Minimal Reproducing Example

The CSS Font Loading API can be used to load fonts via JS using the `FontFace` constructor. The constructor is passed a source, which can either be a css string or `BinaryData` (an ArrayBuffer or ArrayBufferView). According to the spec[1], if BinaryData is passed, the data is directly stored in the FontFaces internal `[[Data]]` slot and then parsed asynchronously. After this parsing finished, the fonts `loading` promise shall be resolved (either fulfilled if the parsing was successful or rejected otherwise). According to the spec, the `load()` method of a `FontFace` is a no-op if the font-face is constructed from BinaryData[2].

In conclusion: According to the spec, if a FontFace is constructed from BinaryData, its `loaded` property can be used immediately without calling the `load()` method and is guaranteed to be resolved.

However, when loading a custom FontFace from BinaryData in Lockdown Mode, the `loaded` promise is never resolved until `load()` is called.

A simple example to reproduce this is attached as `mre.html`. The page should show "font.loaded fulfilled" or "font.loaded rejected" almost immediately after loading. However in current Safari, it is stock on `Loading font ...` forever. Adding a `font.load()` at the end fixes this, making the promise fail as expected immediately.

----

The following is educated speculation, as I'm not familiar with the webkit codebase.

The problem seems to be the code in [3]/[4], which checks if the loaded font is within a number of allow-listed webfonts[5] and otherwise returns. My impression is that instead of returning empty, this should reject the `loaded` promise (`return Exception { NetworkError }`)

[1]: https://www.w3.org/TR/css-font-loading/#font-face-constructor
[2]: https://www.w3.org/TR/css-font-loading/#font-face-load
[3]: https://github.com/WebKit/WebKit/blob/d68d5c148eb3616448ae8d8e28408c0340823181/Source/WebCore/css/FontFace.cpp#L88-L90
[3]: https://github.com/WebKit/WebKit/blob/d68d5c148eb3616448ae8d8e28408c0340823181/Source/WebCore/css/FontFace.cpp#L82-L83
[5]: https://github.com/WebKit/WebKit/blob/main/Source/WebCore/loader/cache/AllowedFonts.cpp#L41
Comment 1 pajowu 2023-09-13 06:30:35 PDT 
Comment on attachment 467671 [details]
Minimal Reproducing Example

The mre should of course be the following (only change is the `font.load()` at the end of the file commented out)

><html>
>  <body>
>    <div id="status">Page is loading...</div>
>  </body>
>  <script>
>    // Taken from https://github.com/mozilla/pdf.js/blob/920e51a1e6050ed9b8ddad51a3fb0a39dc4fa5ee/src/display/font_loader.js#L228
>    // This is a CFF font with 1 glyph for '.' that fills its entire width
>    // and height.
>    const testFont = atob(
>      "T1RUTwALAIAAAwAwQ0ZGIDHtZg4AAAOYAAAAgUZGVE1lkzZwAAAEHAAAABxHREVGABQA" +
>        "FQAABDgAAAAeT1MvMlYNYwkAAAEgAAAAYGNtYXABDQLUAAACNAAAAUJoZWFk/xVFDQAA" +
>        "ALwAAAA2aGhlYQdkA+oAAAD0AAAAJGhtdHgD6AAAAAAEWAAAAAZtYXhwAAJQAAAAARgA" +
>        "AAAGbmFtZVjmdH4AAAGAAAAAsXBvc3T/hgAzAAADeAAAACAAAQAAAAEAALZRFsRfDzz1" +
>        "AAsD6AAAAADOBOTLAAAAAM4KHDwAAAAAA+gDIQAAAAgAAgAAAAAAAAABAAADIQAAAFoD" +
>        "6AAAAAAD6AABAAAAAAAAAAAAAAAAAAAAAQAAUAAAAgAAAAQD6AH0AAUAAAKKArwAAACM" +
>        "AooCvAAAAeAAMQECAAACAAYJAAAAAAAAAAAAAQAAAAAAAAAAAAAAAFBmRWQAwAAuAC4D" +
>        "IP84AFoDIQAAAAAAAQAAAAAAAAAAACAAIAABAAAADgCuAAEAAAAAAAAAAQAAAAEAAAAA" +
>        "AAEAAQAAAAEAAAAAAAIAAQAAAAEAAAAAAAMAAQAAAAEAAAAAAAQAAQAAAAEAAAAAAAUA" +
>        "AQAAAAEAAAAAAAYAAQAAAAMAAQQJAAAAAgABAAMAAQQJAAEAAgABAAMAAQQJAAIAAgAB" +
>        "AAMAAQQJAAMAAgABAAMAAQQJAAQAAgABAAMAAQQJAAUAAgABAAMAAQQJAAYAAgABWABY" +
>        "AAAAAAAAAwAAAAMAAAAcAAEAAAAAADwAAwABAAAAHAAEACAAAAAEAAQAAQAAAC7//wAA" +
>        "AC7////TAAEAAAAAAAABBgAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
>        "AAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
>        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
>        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
>        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" +
>        "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAMAAAAAAAD/gwAyAAAAAQAAAAAAAAAAAAAAAAAA" +
>        "AAABAAQEAAEBAQJYAAEBASH4DwD4GwHEAvgcA/gXBIwMAYuL+nz5tQXkD5j3CBLnEQAC" +
>        "AQEBIVhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYWFhYAAABAQAADwACAQEEE/t3" +
>        "Dov6fAH6fAT+fPp8+nwHDosMCvm1Cvm1DAz6fBQAAAAAAAABAAAAAMmJbzEAAAAAzgTj" +
>        "FQAAAADOBOQpAAEAAAAAAAAADAAUAAQAAAABAAAAAgABAAAAAAAAAAAD6AAAAAAAAA=="
>    );
>    const fontBuffer = Uint8Array.from(testFont, (c) => c.charCodeAt(0));
>    const statusElem = document.getElementById("status");
>
>    const font = new FontFace("test", fontBuffer);
>    statusElem.innerText = "Loading font ...";
>
>    font.loaded.then(
>      () => (statusElem.innerText = "font.loaded fulfilled"),
>      () => (statusElem.innerText = "font.loaded rejected")
>    );
>    // font.load();
>  </script>
></html>
Comment 2 Radar WebKit Bug Importer 2023-09-13 07:42:31 PDT 
<rdar://problem/115429609>