From 603b80100909b4a6cc41824bfa43c4b0bdd1100e Mon Sep 17 00:00:00 2001 From: gravitypdf-bot Date: Thu, 12 Nov 2026 11:33:44 +1100 Subject: [PATCH] Keep another font's file when deleting a core font Before the sync saves a core font under its hashed name, another font's file of a known size at the font's name passes the size check as the font's own copy, so deleting the family in the Font Manager deleted it. Hash a family file at its plain name before deleting it, and keep it when its SHA-1 isn't a known build. The E2E stand-ins for core fonts are truncated copies of other fonts, so they now go under the hashed name, which is trusted by size. The Multisite test that deletes the file at the plain name copies the manifest's own build there. Refs #2036 Co-Authored-By: Claude Opus 5.5 --- src/Model/Model_Font_Downloader.php | 13 +++++-- .../Model/Test_Downloaded_Fonts.php | 9 +++++ .../core/managers/font-manager.spec.ts | 4 +-- tests/playwright/multisite/site-fonts.spec.ts | 14 +++++--- tools/playwright/utils/core-fonts.ts | 35 ++++++++++++++----- 5 files changed, 58 insertions(+), 17 deletions(-) diff --git a/src/Model/Model_Font_Downloader.php b/src/Model/Model_Font_Downloader.php index 4039902fb..a1308c9e8 100644 --- a/src/Model/Model_Font_Downloader.php +++ b/src/Model/Model_Font_Downloader.php @@ -802,8 +802,8 @@ protected function get_size_fields( array $files, string $dir ): array { /** * Delete an unused downloaded family's own copies (see Model_Font_Sources::get_own_copy()), keeping the licence files - * and any other font's file at a family file's name. A file at the name another site's custom font uses is kept too, - * as every site shares the fonts folder. + * and any other font's file at a family file's name (a file of a known size is hashed to tell). A file at the name + * another site's custom font uses is kept too, as every site shares the fonts folder. * * @return array{id: string, removed: string[], bytes: int, font: array|null}|WP_Error `font` is its row now, if it can * be installed again. 404 when @@ -813,6 +813,15 @@ protected function get_size_fields( array $files, string $dir ): array { */ public function delete_downloaded_font( string $key ) { $family = $this->get_core_families()[ $key ] ?? null; + + /* Before the sync saves the font under its hashed name, another font's file of a known size passes for its own */ + foreach ( $family['paths'] ?? [] as $file => $path ) { + if ( $path === $this->font_sources->get_plain_path( $file ) && $this->is_unknown_build( $file ) ) { + unset( $family['paths'][ $file ] ); + $this->log->notice( 'Kept another font\'s file at a font file\'s name', [ 'name' => $file ] ); + } + } + if ( $family === null || count( $family['paths'] ) === 0 ) { return new WP_Error( 'font_not_downloaded', __( 'The font is not downloaded.', 'gravity-pdf' ), [ 'status' => 404 ] ); } diff --git a/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php b/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php index 3e49ed9db..840ef3240 100644 --- a/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php +++ b/tests/phpunit/integration/Model/Test_Downloaded_Fonts.php @@ -381,6 +381,15 @@ public function test_delete_removes_only_the_hashed_copy_and_never_the_file_at_t $this->assertFileEquals( $this->font_fixture( 'Chewy.ttf' ), $this->font_path( 'DejaVuSerifCondensed.ttf' ) ); } + public function test_delete_keeps_another_fonts_file_of_a_known_size_at_the_name() { + $this->put_same_size_font_at( 'DejaVuSerifCondensed.ttf', 'DejaVuSerifCondensed.ttf' ); + $bytes = (string) file_get_contents( $this->font_path( 'DejaVuSerifCondensed.ttf' ) ); + + $this->assert_font_error( $this->downloader()->delete_downloaded_font( 'dejavuserifcondensed' ), 'font_not_downloaded', 404 ); + $this->assertStringEqualsFile( $this->font_path( 'DejaVuSerifCondensed.ttf' ), $bytes ); + $this->assertCount( 1, $this->log_messages( 'Kept another font\'s file at a font file\'s name' ) ); + } + public function test_a_family_sharing_a_name_with_a_record_lists_and_deletes_only_its_hashed_copy() { $sources = GPDFAPI::get_mvc_class( 'Model_Font_Sources' ); $hashed = $sources->get_hashed_name( 'DejaVuSerifCondensed.ttf' ); diff --git a/tests/playwright/core/managers/font-manager.spec.ts b/tests/playwright/core/managers/font-manager.spec.ts index 7b70de063..6c9f60cfa 100644 --- a/tests/playwright/core/managers/font-manager.spec.ts +++ b/tests/playwright/core/managers/font-manager.spec.ts @@ -4,7 +4,7 @@ import type { Page } from '@playwright/test'; import { test, resourcesPath } from '@self:playwright/fixtures/test'; import Pdf, { type FontUpload } from '@self:playwright/utils/gravitypdf'; import { wpCli } from '@self:playwright/utils/wp-cli'; -import { coreFontStandIn } from '@self:playwright/utils/core-fonts'; +import { coreFontStandIn, hashedName } from '@self:playwright/utils/core-fonts'; import * as fs from 'fs'; import * as path from 'path'; @@ -379,7 +379,7 @@ test.describe('Font Manager', () => { // Seeds a core font no other spec uses const seedCoreFont = (...files: string[]) => { - seededFiles.push(...files); + seededFiles.push(...files.map(hashedName)); wpCli( ...files.map((file) => coreFontStandIn( diff --git a/tests/playwright/multisite/site-fonts.spec.ts b/tests/playwright/multisite/site-fonts.spec.ts index 21238e7ce..7a56961cb 100644 --- a/tests/playwright/multisite/site-fonts.spec.ts +++ b/tests/playwright/multisite/site-fonts.spec.ts @@ -6,7 +6,7 @@ import { logInToSubsite as logIn, } from '@self:playwright/utils/multisite'; import { wpCli } from '@self:playwright/utils/wp-cli'; -import { coreFontStandIn } from '@self:playwright/utils/core-fonts'; +import { coreFontStandIn, hashedName } from '@self:playwright/utils/core-fonts'; import * as fs from 'fs'; import * as path from 'path'; @@ -17,6 +17,10 @@ const FILE = 'NetworkRoboto.ttf'; const CORE_NAMED_FONT = 'Core Named Roboto'; const CORE_FILE = 'DejaVuSerifCondensed.ttf'; const CORE_BOLD_FILE = 'DejaVuSerifCondensed-Bold.ttf'; +const CORE_BOLD_COPY = hashedName(CORE_BOLD_FILE); +// The manifest's own build, which the plugin hashes before deleting it from the core font's name +const CORE_FILE_IN_CONTAINER = + 'wp-content/plugins/gravity-pdf/tools/phpunit/data/fonts/DejaVuSerifCondensed.ttf'; const ROBOTO_IN_CONTAINER = 'wp-content/plugins/gravity-pdf/tools/playwright/data/fonts/Roboto-Regular.ttf'; @@ -83,7 +87,7 @@ test.describe('Fonts of a Multisite subsite', () => { } await admin.context().close(); - wpCli(`rm -f ${FONTS}/${CORE_FILE} ${FONTS}/${CORE_BOLD_FILE}`); + wpCli(`rm -f ${FONTS}/${CORE_FILE} ${FONTS}/${CORE_BOLD_COPY}`); }; test.beforeAll(deleteFonts); @@ -207,7 +211,7 @@ test.describe('Fonts of a Multisite subsite', () => { (row: { id: string }) => row.id === 'dejavuserifcondensed' ); expect(core.regular).toBe(''); - expect(core.bold).toBe(CORE_BOLD_FILE); + expect(core.bold).toBe(CORE_BOLD_COPY); const deleted = await send( page, @@ -222,7 +226,7 @@ test.describe('Fonts of a Multisite subsite', () => { const files = fontFileList(); expect(files).toContain(CORE_FILE); - expect(files).not.toContain(CORE_BOLD_FILE); + expect(files).not.toContain(CORE_BOLD_COPY); // Throws if the subsite's font no longer has its own bytes wpCli(`cmp ${ROBOTO_IN_CONTAINER} ${FONTS}/${CORE_FILE}`); @@ -249,7 +253,7 @@ test.describe('Fonts of a Multisite subsite', () => { await subsite.context().close(); // The custom font's delete removed its file, so put one back at the core font's name - wpCli(coreFontStandIn(ROBOTO_IN_CONTAINER, `${FONTS}/${CORE_FILE}`)); + wpCli(`cp ${CORE_FILE_IN_CONTAINER} ${FONTS}/${CORE_FILE}`); await page.goto('/wp-admin/index.php'); const deleted = await send( diff --git a/tools/playwright/utils/core-fonts.ts b/tools/playwright/utils/core-fonts.ts index 8f9062b8f..fb55d6159 100644 --- a/tools/playwright/utils/core-fonts.ts +++ b/tools/playwright/utils/core-fonts.ts @@ -1,26 +1,45 @@ import * as fs from 'fs'; import * as path from 'path'; -const manifest: { name: string; size: number }[] = JSON.parse( +const manifest: { name: string; size: number; sha: string }[] = JSON.parse( fs.readFileSync( path.join(__dirname, '..', '..', '..', 'fonts', 'core-fonts.json'), 'utf8' ) ); +const entryFor = (file: string) => { + const entry = manifest.find((row) => row.name === file); + if (entry === undefined) { + throw new Error(`${file} is not on the core font manifest`); + } + + return entry; +}; + +/** + * The name a core font file takes when another file holds its own: `{stem}-{first 8 of its SHA-1}{ext}` + * + * @param file The core font file's name + */ +export function hashedName(file: string) { + const { ext, name } = path.parse(file); + + return `${name}-${entryFor(file).sha.slice(0, 8)}${ext}`; +} + /** - * A shell command putting a stand-in for a core font file at `dest`: a copy of `source` at the core file's size on the - * manifest, which the plugin takes as the core font's own copy (downloads are off on the E2E sites) + * A shell command putting a stand-in for a core font file under its hashed name (see hashedName()) beside `dest`: a + * copy of `source` at the core file's size on the manifest. The plugin takes a hashed copy of that size as the core + * font's own without hashing it, while one at the plain name is hashed before it's deleted (downloads are off on the + * E2E sites). * * @param source A font file path in the container * @param dest The core font file's path in the container */ export function coreFontStandIn(source: string, dest: string) { const file = path.basename(dest); - const size = manifest.find((entry) => entry.name === file)?.size; - if (size === undefined) { - throw new Error(`${file} is not on the core font manifest`); - } + const target = path.join(path.dirname(dest), hashedName(file)); - return `cp ${source} ${dest} && truncate -s ${size} ${dest}`; + return `cp ${source} ${target} && truncate -s ${entryFor(file).size} ${target}`; }