From e917c43ca9c9038007b1d81074db1211f7395f45 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 21 Jun 2022 14:03:20 -0600 Subject: [PATCH 1/7] [varLib.varStore] Add operator __bool__ and use it Part of https://github.com/fonttools/fonttools/issues/2211#issuecomment-790125437 --- Lib/fontTools/feaLib/builder.py | 2 +- Lib/fontTools/varLib/__init__.py | 4 ++-- Lib/fontTools/varLib/varStore.py | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index a16448759..0601a57c1 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -764,7 +764,7 @@ class Builder(object): gdef.Version = 0x00010002 if gdef.MarkGlyphSetsDef else 0x00010000 if self.varstorebuilder: store = self.varstorebuilder.finish() - if store.VarData: + if store: gdef.Version = 0x00010003 gdef.VarStore = store varidx_map = store.optimize() diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 4029a107e..b28e62608 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -606,7 +606,7 @@ def _add_BASE(font, masterModel, master_ttfs, axisTags): merger.mergeTables(font, master_ttfs, ['BASE']) store = merger.store_builder.finish() - if not store.VarData: + if not store: return base = font['BASE'].table assert base.Version == 0x00010000 @@ -621,7 +621,7 @@ def _merge_OTL(font, model, master_fonts, axisTags): merger.mergeTables(font, master_fonts, ['GSUB', 'GDEF', 'GPOS']) store = merger.store_builder.finish() - if not store.VarData: + if not store: return try: GDEF = font['GDEF'].table diff --git a/Lib/fontTools/varLib/varStore.py b/Lib/fontTools/varLib/varStore.py index bcf81b39d..9f9ead2dd 100644 --- a/Lib/fontTools/varLib/varStore.py +++ b/Lib/fontTools/varLib/varStore.py @@ -135,6 +135,11 @@ def VarRegion_get_support(self, fvar_axes): ot.VarRegion.get_support = VarRegion_get_support +def VarStore___bool__(self): + return bool(self.VarData) + +ot.VarStore.__bool__ = VarStore___bool__ + class VarStoreInstancer(object): def __init__(self, varstore, fvar_axes, location={}): From 83f69028a319c89d6202216d40aed1e1a36d89c7 Mon Sep 17 00:00:00 2001 From: nathannaveen <42319948+nathannaveen@users.noreply.github.com> Date: Thu, 23 Jun 2022 00:58:26 +0000 Subject: [PATCH 2/7] chore: Set permissions for GitHub actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restrict the GitHub token permissions only to the required ones; this way, even if the attackers will succeed in compromising your workflow, they won’t be able to do much. - Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs [Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com> --- .github/workflows/publish.yml | 3 +++ .github/workflows/test.yml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 0b358052d..1eec08484 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -9,6 +9,9 @@ on: tags: - '*.*.*' # e.g. 1.0.0 or 20.15.10 +permissions: + contents: read + jobs: deploy: runs-on: ubuntu-latest diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 25be0c7a3..252e5e93f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: lint: runs-on: ubuntu-latest From d224e1f73d3683d91e610d26751f1d32661490a5 Mon Sep 17 00:00:00 2001 From: Simon Cozens Date: Thu, 23 Jun 2022 15:04:59 +0100 Subject: [PATCH 3/7] [feaLib] show all missing glyphs at once (#2665) --- Lib/fontTools/feaLib/parser.py | 28 +++++++++++++++++++--------- Tests/feaLib/parser_test.py | 2 +- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py index fd53573da..6897c41be 100644 --- a/Lib/fontTools/feaLib/parser.py +++ b/Lib/fontTools/feaLib/parser.py @@ -73,6 +73,7 @@ class Parser(object): self.next_token_location_ = None lexerClass = IncludingLexer if followIncludes else NonIncludingLexer self.lexer_ = lexerClass(featurefile, includeDir=includeDir) + self.missing = {} self.advance_lexer_(comments=True) def parse(self): @@ -125,6 +126,16 @@ class Parser(object): ), self.cur_token_location_, ) + # Report any missing glyphs at the end of parsing + if self.missing: + error = [ + " %s (first found at %s)" % (name, loc) + for name, loc in self.missing.items() + ] + raise FeatureLibError( + "The following glyph names are referenced but are missing from the " + "glyph set:\n" + ("\n".join(error)), None + ) return self.doc_ def parse_anchor_(self): @@ -2073,19 +2084,18 @@ class Parser(object): raise FeatureLibError("Expected a glyph name or CID", self.cur_token_location_) def check_glyph_name_in_glyph_set(self, *names): - """Raises if glyph name (just `start`) or glyph names of a - range (`start` and `end`) are not in the glyph set. + """Adds a glyph name (just `start`) or glyph names of a + range (`start` and `end`) which are not in the glyph set + to the "missing list" for future error reporting. If no glyph set is present, does nothing. """ if self.glyphNames_: - missing = [name for name in names if name not in self.glyphNames_] - if missing: - raise FeatureLibError( - "The following glyph names are referenced but are missing from the " - f"glyph set: {', '.join(missing)}", - self.cur_token_location_, - ) + for name in names: + if name in self.glyphNames_: + continue + if name not in self.missing: + self.missing[name] = self.cur_token_location_ def expect_markClass_reference_(self): name = self.expect_class_name_() diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py index fd9dea700..b281e8ac3 100644 --- a/Tests/feaLib/parser_test.py +++ b/Tests/feaLib/parser_test.py @@ -316,7 +316,7 @@ class ParserTest(unittest.TestCase): def test_strict_glyph_name_check(self): self.parse("@bad = [a b ccc];", glyphNames=("a", "b", "ccc")) - with self.assertRaisesRegex(FeatureLibError, "missing from the glyph set: ccc"): + with self.assertRaisesRegex(FeatureLibError, "(?s)missing from the glyph set:.*ccc"): self.parse("@bad = [a b ccc];", glyphNames=("a", "b")) def test_glyphclass(self): From 0e6a4ec581cd59351f9b0de175ad9d9891f8233b Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 23 Jun 2022 16:34:12 +0100 Subject: [PATCH 4/7] [otBase] temporarily disable extension sharing during harfbuzz repacking If extension sharing is enabled some fonts may fail to repack due to fontTools repacker getting stuck in a unrecoverable error. https://github.com/fonttools/fonttools/issues/2661 --- Lib/fontTools/ttLib/tables/otBase.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/ttLib/tables/otBase.py b/Lib/fontTools/ttLib/tables/otBase.py index 76c4766a4..77ae67866 100644 --- a/Lib/fontTools/ttLib/tables/otBase.py +++ b/Lib/fontTools/ttLib/tables/otBase.py @@ -533,7 +533,9 @@ class OTTableWriter(object): https://github.com/harfbuzz/uharfbuzz/blob/main/src/uharfbuzz/_harfbuzz.pyx#L1149 """ internedTables = {} - self._doneWriting(internedTables, shareExtension=True) + # TODO: Restore shareExtension=True after we fix + # https://github.com/fonttools/fonttools/issues/2661 + self._doneWriting(internedTables, shareExtension=False) tables = [] obj_list = [] done = {} From cac724151456a601f33e978e667f773cfdc0e4cc Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 21 Jun 2022 14:08:34 -0600 Subject: [PATCH 5/7] [varLib.varStore] Add NO_VARIATION_INDEX Part of https://github.com/fonttools/fonttools/issues/2211 --- Lib/fontTools/varLib/varStore.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/fontTools/varLib/varStore.py b/Lib/fontTools/varLib/varStore.py index 9f9ead2dd..b4c92052f 100644 --- a/Lib/fontTools/varLib/varStore.py +++ b/Lib/fontTools/varLib/varStore.py @@ -7,6 +7,10 @@ from functools import partial from collections import defaultdict +NO_VARIATION_INDEX = 0xFFFFFFFF +ot.VarStore.NO_VARIATION_INDEX = NO_VARIATION_INDEX + + def _getLocationKey(loc): return tuple(sorted(loc.items(), key=lambda kv: kv[0])) From e01f643a8e41eacded24b67adb841eb0c217f43f Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 21 Jun 2022 14:27:32 -0600 Subject: [PATCH 6/7] [varLib.varStore] Support NO_VARIATION_INDEX in optimizer & instancer Fixes https://github.com/fonttools/fonttools/issues/2211 --- Lib/fontTools/varLib/__init__.py | 2 +- Lib/fontTools/varLib/instancer/__init__.py | 1 + Lib/fontTools/varLib/varStore.py | 11 ++++++++--- Tests/varLib/instancer/instancer_test.py | 8 ++++---- Tests/varLib/varStore_test.py | 2 +- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index b28e62608..eb8a3a9bc 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -486,7 +486,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs, vOrigMap[glyphName] = storeBuilder.storeDeltas(deltas, round=noRound) indirectStore = storeBuilder.finish() - mapping2 = indirectStore.optimize() + mapping2 = indirectStore.optimize(use_NO_VARIATION_INDEX=False) advMapping = [mapping2[advMapping[g]] for g in glyphOrder] advanceMapping = builder.buildVarIdxMap(advMapping, glyphOrder) diff --git a/Lib/fontTools/varLib/instancer/__init__.py b/Lib/fontTools/varLib/instancer/__init__.py index 881a1c989..91dd586ff 100644 --- a/Lib/fontTools/varLib/instancer/__init__.py +++ b/Lib/fontTools/varLib/instancer/__init__.py @@ -633,6 +633,7 @@ def instantiateItemVariationStore(itemVarStore, fvarAxes, axisLimits): for major, deltas in enumerate(defaultDeltaArray) for minor, delta in enumerate(deltas) } + defaultDeltas[itemVarStore.NO_VARIATION_INDEX] = 0 return defaultDeltas diff --git a/Lib/fontTools/varLib/varStore.py b/Lib/fontTools/varLib/varStore.py index b4c92052f..5e2155870 100644 --- a/Lib/fontTools/varLib/varStore.py +++ b/Lib/fontTools/varLib/varStore.py @@ -178,6 +178,7 @@ class VarStoreInstancer(object): def __getitem__(self, varidx): major, minor = varidx >> 16, varidx & 0xFFFF + if varidx == NO_VARIATION_INDEX: return 0. varData = self._varData scalars = [self._getScalar(ri) for ri in varData[major].VarRegionIndex] deltas = varData[major].Item[minor] @@ -440,7 +441,7 @@ class _EncodingDict(dict): return chars -def VarStore_optimize(self): +def VarStore_optimize(self, use_NO_VARIATION_INDEX=True): """Optimize storage. Returns mapping from old VarIdxes to new ones.""" # TODO @@ -464,6 +465,10 @@ def VarStore_optimize(self): row[regionIdx] += v row = tuple(row) + if use_NO_VARIATION_INDEX and not any(row): + front_mapping[(major<<16)+minor] = None + continue + encodings.add_row(row) front_mapping[(major<<16)+minor] = row @@ -546,9 +551,9 @@ def VarStore_optimize(self): back_mapping[item] = (major<<16)+minor # Compile final mapping. - varidx_map = {} + varidx_map = {NO_VARIATION_INDEX:NO_VARIATION_INDEX} for k,v in front_mapping.items(): - varidx_map[k] = back_mapping[v] + varidx_map[k] = back_mapping[v] if v is not None else NO_VARIATION_INDEX # Remove unused regions. self.prune_regions() diff --git a/Tests/varLib/instancer/instancer_test.py b/Tests/varLib/instancer/instancer_test.py index b9d4ffe9b..c3b6f71d9 100644 --- a/Tests/varLib/instancer/instancer_test.py +++ b/Tests/varLib/instancer/instancer_test.py @@ -360,7 +360,7 @@ class InstantiateHVARTest(object): assert support == pytest.approx(expectedRegion[axisTag]) assert len(varStore.VarData) == 1 - assert varStore.VarData[0].ItemCount == 2 + assert varStore.VarData[0].ItemCount == 1 assert hvar.AdvWidthMap is not None advWithMap = hvar.AdvWidthMap.mapping @@ -368,9 +368,7 @@ class InstantiateHVARTest(object): assert advWithMap[".notdef"] == advWithMap["space"] varIdx = advWithMap[".notdef"] # these glyphs have no metrics variations in the test font - assert varStore.VarData[varIdx >> 16].Item[varIdx & 0xFFFF] == ( - [0] * varStore.VarData[0].VarRegionCount - ) + assert varIdx == varStore.NO_VARIATION_INDEX varIdx = advWithMap["hyphen"] assert varStore.VarData[varIdx >> 16].Item[varIdx & 0xFFFF] == expectedDeltas @@ -458,6 +456,8 @@ class InstantiateItemVariationStoreTest(object): defaultDeltaArray = [] for varidx, delta in sorted(defaultDeltas.items()): + if varidx == varStore.NO_VARIATION_INDEX: + continue major, minor = varidx >> 16, varidx & 0xFFFF if major == len(defaultDeltaArray): defaultDeltaArray.append([]) diff --git a/Tests/varLib/varStore_test.py b/Tests/varLib/varStore_test.py index a1bdd1237..d89c76034 100644 --- a/Tests/varLib/varStore_test.py +++ b/Tests/varLib/varStore_test.py @@ -13,7 +13,7 @@ from fontTools.ttLib.tables.otTables import VarStore ( [{}, {"a": 1}], [ - [10, 20], + [10, 10], [100, 2000], [100, 22000], ], From 5d70109645bd42d456dae2d8d97747aa60d180fd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Thu, 23 Jun 2022 11:58:50 -0600 Subject: [PATCH 7/7] [varStore] Comment --- Tests/varLib/varStore_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/varLib/varStore_test.py b/Tests/varLib/varStore_test.py index d89c76034..cad8ac735 100644 --- a/Tests/varLib/varStore_test.py +++ b/Tests/varLib/varStore_test.py @@ -13,7 +13,7 @@ from fontTools.ttLib.tables.otTables import VarStore ( [{}, {"a": 1}], [ - [10, 10], + [10, 10], # Test NO_VARIATION_INDEX [100, 2000], [100, 22000], ],