Repository navigation
Save a core font under a hashed name when another font's file holds its name - #2016
Merged
Merged
Conversation
Contributor
|
Coverage report for commit: 38e02af Summary - Lines: 93.99% | Methods: 90.96% | Branches: 85.66%
🤖 Jest coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
|
Coverage report for commit: 38e02af Summary - Lines: 89.64% | Methods: 95.78%
🤖 PHPUnit coverage report |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gravitypdf-bot
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 5, 2026 23:59
8b50cd8 to
f7acf3e
Compare
This was referenced Nov 6, 2026
Merged
jakejackson1
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 6, 2026 03:06
f7acf3e to
74c3b06
Compare
Base automatically changed from
refactor/unify-multisite-fonts
to
development
November 9, 2026 05:21
gravitypdf-bot
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 9, 2026 05:30
74c3b06 to
1ad0a3c
Compare
gravitypdf-bot
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 9, 2026 05:45
1ad0a3c to
62cbe50
Compare
…ts name
download() returned true for any file at a manifest name, so a family could silently use another font's file, and
merging two sources' manifests with += gave the later source the first's file for a name they use for different files.
A file at a manifest name is another font's when its size matches no build the sources know (the manifest's, the
legacy manifest's or a source's `legacy` hashes). The family's font is then saved as {stem}-{first 8 of the sha}{ext},
derived from the manifest with nothing stored, and mPDF looks for that name first. The file at the name is never
touched. A size mismatch is a stat call, so it is also found while rendering; a file of a known size that is not the
manifest's is found by the background sync, the only place a file already on disk is hashed. With downloads off, or a
download failing or backing off, the file at the name stays in use, so a conflict never renders worse than before.
Two sources that use one name for different files get it at merge time: the later source's manifest entry and family
file references take the hashed name, so nothing downstream sees a duplicate.
A font upload can't take a name a core font's copy could have, as that copy is used before the file at the plain name,
and a file of the wrong size at the hashed name isn't treated as the copy.
Every reader of a font file's path now agrees: the finder, the downloaded fonts list and delete (a hashed copy is
deleted alone), the installed counts and needs-download checks, the sync's file list and retries, prune and adoption
(which read the file at the name), and the unregistered fonts auto-loader (which skips hashed copies). Saving a hashed
copy flushes the font metrics and removes a copy left by an older release of the font.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
gravitypdf-bot
force-pushed
the
feature/font-file-conflict-rename
branch
from
November 9, 2026 06:09
62cbe50 to
38e02af
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Model_Font_Downloader::download()treated any file at a manifest name as installed, so a font family could silently use another font's file, andModel_Font_Sources::get_manifest()merged sources with+=, so a later source could silently get an earlier source's file. Now a file at a manifest name that is another font's is left alone, and the family's own font is downloaded under{stem}-{first 8 of the sha}{ext}(for exampleGaruda-0abddd35.ttf), worked out from the manifest with nothing stored. mPDF looks for that name first, then the plain name.legacyhashes.Suggested changelog line:
* Fix: a core font is now saved under a new name when a different file already has its name, instead of silently using that file. A core font replaced by hand is no longer used while downloads are on.Try it
wp-content/uploads/PDF_EXTENDED_TEMPLATES/fonts/DejaVuSansCondensed.ttfwith any other.ttf(or one byte longer), and make a PDF in DejaVu Sans Condensed.DejaVuSansCondensed-<8 hex>.ttfappears beside your file, the PDF is set in the real DejaVu Sans Condensed, and your file is untouched.define( 'GPDF_AUTO_INSTALL_FONTS', false );), delete the new copy and render again: your file is used and nothing is requested.Test plan
php -lon PHP 7.4 for the changedsrc/filesyarn test:e2e: 127 of 127 passed before the rebase onto Keep font state in one place for every site, and keep every site's fonts in the 6.19 cleanup #2004 and List custom, core and bundled fonts together in the Font Manager #2015, not re-run sinceMore info
Detection
On_Demand_Font_File_Finder)is_file()on the hashed name, then the plain name, then a size compare against the manifest, and only on a mismatch the legacy manifest andlegacysizes. A size conflict downloads the font to the hashed name (or keeps the plain file when that can't happen)includeof the manifest for a font in the fonts foldersync())download( $file, true ), which hashes it once and, if its SHA-1 isn't a known build's, downloads the real font to the hashed name. Skipped with downloads off, as nothing could be done about the result. Files that need a download go first, so hashing the rest can't push one into a paused runget_retries())A sync runs on install, a version change, a language pack install and a saved default font or PDF that needs a download. A same-size, different-hash file is therefore found at the next of those, not when it's first put there. Fonts that aren't in the required set (a substitute downloaded mid-render) are only checked by size.
An existing file is never hashed to detect a conflict during a render. The one hash that can happen there is after a conflict download, when an older release's hashed copy is checked before it's removed.
Behaviour beyond the brief
Model_Font_Sources::is_downloaded()is still "the file at its name exists" (it runs on every render for Sun-ExtA/ExtB). A newis_installed()("its own copy is on disk") drives needs-download, the System Report's installed counts and the sync.FlushCache::flush()(metrics and the PDF cache generation), as the file the metrics were built from has changed.{stem}-{8 hex}{ext}files whose own SHA-1 starts with the 8 hex in their name (a font someone else put there won't). Until the new copy downloads, a render uses the file at the name.{manifest stem}-{8 hex}{ext}in any case, so a later release's hashed name is covered too. A retry of a reserved name uses_before its number, as digits alone could complete an 8-hex suffix.Model_Font_Downloader::download()takes an optional$verifysecond argument. New public methods onModel_Font_Sources(get_plain_path,get_hashed_name,parse_hashed_name,is_reserved_name,is_hashed_copy,has_hashed_copy,is_installed,is_size_conflict,get_known_builds,get_record_collisions,is_record_file,use_hashed_names) andModel_Font_Downloader::is_in_fonts_folder(). No constructor changes.list_fixture_as()), as it would otherwise be a conflict. The "render with fonts on disk doesn't load the manifest" test is replaced: the manifest is read when a font in the fonts folder is requested, and not for the bundled fonts.Font Manager records
get_record_collisions()(per blog, memoised): a record's file whose basename is a downloadable family's file name, and whose size matches no known build of it, maps to the hashed name. Records are read from the settings already in memory; the manifest is read and a file stat'd only when a record's file has a family's name, and nothing is hashed.Known limits
is_downloaded()(and so the Sun-ExtA substitution check) only sees the file at the name.rename_colliding_files()needs two sources with manifests, which only exist after Let plugins add font sources through the gfpdf_font_sources filter #2013, so it's exercised in tests with a test subclass until then.Open PRs based on this branch
#2008, #2010, #2011, #2012, #2013 and #2014 target this branch and need rebasing onto the new commit.
Baseline failures
test_an_unwritable_folder_fails_without_a_request(x2) needs chmod to refuse the owner, and the container runs as root, so it cannot pass in it.