Prevent stack overflow from circular composite glyph references by fdb · Pull Request #826 · opentypejs/opentype.js

fdb · 2026-04-03T17:21:40Z

Description

This change adds protection against stack overflow errors when loading TrueType fonts that contain circular references in composite glyphs. The fix tracks which glyphs are currently being resolved and skips any circular references encountered during path building.

Changes Made:

src/tables/glyf.mjs: Added a module-scoped _resolving Set to track glyph indices currently being processed. When building composite glyph paths, the code now checks if a component glyph is already being resolved and skips it if a circular reference is detected. The tracking is properly cleaned up using try/finally to ensure the Set is maintained correctly across recursive calls.
test/generate-circular-ref-font.mjs: Added a utility script that generates a minimal test font (circular-composite.ttf) containing two composite glyphs that reference each other circularly. This font is used to verify the fix works correctly.
test/glyph.spec.mjs: Added a test case that loads the circular composite font and calls getPath() on both glyphs to verify they return valid Path objects without causing a stack overflow.

Motivation and Context

Fonts with circular composite glyph references (where glyph A references glyph B and glyph B references glyph A) would previously cause infinite recursion and stack overflow errors when calling getPath(). While such fonts are malformed, the library should handle them gracefully rather than crashing.

How Has This Been Tested?

Added a new test case circular composite glyph references that loads a specially crafted font with circular references and verifies that getPath() returns valid Path objects without throwing errors
The test font is generated by the new generate-circular-ref-font.mjs script
All existing tests continue to pass with the circular reference detection in place

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

I have added tests to cover my changes
The fix handles the edge case gracefully without breaking existing functionality

https://claude.ai/code/session_014E56osqBcVfPwdgHHtrMBJ

Add cycle detection to buildPath() in glyf.mjs using a module-scoped Set that tracks glyph indices currently being resolved. When a circular reference is detected (e.g. glyph A → B → A), the component is skipped instead of recursing infinitely. This prevents denial-of-service via crafted TrueType fonts with circular composite glyph references. Includes a POC font generator and regression test with a minimal TTF containing two mutually-referencing composite glyphs. https://claude.ai/code/session_014E56osqBcVfPwdgHHtrMBJ

Copilot

Pull request overview

This pull request hardens TrueType composite glyph path building against infinite recursion/stack overflows caused by circular component references, and adds a regression fixture + test to ensure getPath() remains safe on malformed fonts.

Changes:

Add circular-reference detection during composite glyph path resolution.
Add a generator script and a minimal TTF fixture containing circular composite references.
Add a regression test that loads the fixture and calls getPath() on the involved glyphs.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
`src/tables/glyf.mjs`	Tracks glyph indices during composite resolution to skip circular references and prevent recursion overflow.
`test/glyph.spec.mjs`	Adds a regression test ensuring circular composites don’t crash `getPath()`.
`test/generate-circular-ref-font.mjs`	Adds a utility to generate a minimal circular-composite TTF fixture for testing.
`test/fonts/circular-composite.ttf`	Commits the generated test fixture font used by the new test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T17:26:20Z

src/tables/glyf.mjs

    if (glyph.isComposite) {
-        for (let j = 0; j < glyph.components.length; j += 1) {
-            const component = glyph.components[j];
-            const componentGlyph = glyphs.get(component.glyphIndex);
-            // Force the ttfGlyphLoader to parse the glyph.
-            componentGlyph.getPath();
-            if (componentGlyph.points) {
-                let transformedPoints;
-                if (component.matchedPoints === undefined) {
-                    // component positioned by offset
-                    transformedPoints = transformPoints(componentGlyph.points, component);
-                } else {
-                    // component positioned by matched points
-                    if ((component.matchedPoints[0] > glyph.points.length - 1) ||
-                        (component.matchedPoints[1] > componentGlyph.points.length - 1)) {
-                        throw Error('Matched points out of range in ' + glyph.name);
+        _resolving.add(glyph.index);
+        try {
+            for (let j = 0; j < glyph.components.length; j += 1) {
+                const component = glyph.components[j];
+                if (_resolving.has(component.glyphIndex)) {
+                    continue; // skip circular reference
+                }
+                const componentGlyph = glyphs.get(component.glyphIndex);
+                // Force the ttfGlyphLoader to parse the glyph.
+                componentGlyph.getPath();
+                if (componentGlyph.points) {
+                    let transformedPoints;
+                    if (component.matchedPoints === undefined) {
+                        // component positioned by offset
+                        transformedPoints = transformPoints(componentGlyph.points, component);
+                    } else {
+                        // component positioned by matched points
+                        if ((component.matchedPoints[0] > glyph.points.length - 1) ||
+                            (component.matchedPoints[1] > componentGlyph.points.length - 1)) {
+                            throw Error('Matched points out of range in ' + glyph.name);
+                        }
+                        const firstPt = glyph.points[component.matchedPoints[0]];
+                        let secondPt = componentGlyph.points[component.matchedPoints[1]];
+                        const transform = {
+                            xScale: component.xScale, scale01: component.scale01,
+                            scale10: component.scale10, yScale: component.yScale,
+                            dx: 0, dy: 0
+                        };
+                        secondPt = transformPoints([secondPt], transform)[0];
+                        transform.dx = firstPt.x - secondPt.x;
+                        transform.dy = firstPt.y - secondPt.y;
+                        transformedPoints = transformPoints(componentGlyph.points, transform);
                    }
-                    const firstPt = glyph.points[component.matchedPoints[0]];
-                    let secondPt = componentGlyph.points[component.matchedPoints[1]];
-                    const transform = {
-                        xScale: component.xScale, scale01: component.scale01,
-                        scale10: component.scale10, yScale: component.yScale,
-                        dx: 0, dy: 0
-                    };
-                    secondPt = transformPoints([secondPt], transform)[0];
-                    transform.dx = firstPt.x - secondPt.x;
-                    transform.dy = firstPt.y - secondPt.y;
-                    transformedPoints = transformPoints(componentGlyph.points, transform);
+                    glyph.points = glyph.points.concat(transformedPoints);
                }
-                glyph.points = glyph.points.concat(transformedPoints);
            }
+        } finally {
+            _resolving.delete(glyph.index);
        }


_resolving is module-scoped and keyed only by glyph index. If two different fonts (or separate GlyphSet instances) resolve glyphs with the same index interleaved in the same process, this can cause false circular-reference detection and silently drop components. Also, using a plain Set can be unsafe under re-entrant calls: adding an already-present index and then unconditionally deleting it in finally can clear the marker while an outer frame is still resolving. Consider scoping the tracking to the font/glyphset (e.g., store a Set/Map on glyphs/glyph.font, or use a WeakMap<GlyphSet, Map<number, count>>) and only remove the marker if this frame added it (or decrement a refcount).

Copilot · 2026-04-03T17:26:20Z

test/generate-circular-ref-font.mjs

+        ...i16(0),      ...i16(1),         // idDelta[]: 1-0x41=-0x40... let's use glyph offset instead
+        ...u16(0),      ...u16(0),         // idRangeOffset[]: 0, 0
+    ];
+    // Fix idDelta: to map 0x41 → glyph 1, delta = 1 - 0x41 = -0x40 = 0xFFC0
+    subtable[20] = 0xFF; subtable[21] = 0xC0;
+    // sentinel delta
+    subtable[22] = 0x00; subtable[23] = 0x01;


The cmap generator looks incorrect here: subtable is a byte array, and indices 20–23 correspond to startCode[], not idDelta[]. As written, this overwrites the startCode for the segment and the font will not map U+0041 ('A') to glyph 1 as described. The idDelta bytes should be patched at the correct offsets (immediately after the startCode[] array) and the inline comment about “let's use glyph offset instead” should be updated to match the chosen approach. After fixing, regenerate the committed test/fonts/circular-composite.ttf so the fixture matches the script/output description.

Suggested change

...i16(0), ...i16(1), // idDelta[]: 1-0x41=-0x40... let's use glyph offset instead

...u16(0), ...u16(0), // idRangeOffset[]: 0, 0

];

// Fix idDelta: to map 0x41 → glyph 1, delta = 1 - 0x41 = -0x40 = 0xFFC0

subtable[20] = 0xFF; subtable[21] = 0xC0;

// sentinel delta

subtable[22] = 0x00; subtable[23] = 0x01;

...i16(0), ...i16(1), // idDelta[]: patched below for 'A' and sentinel

...u16(0), ...u16(0), // idRangeOffset[]: 0, 0

];

// Fix idDelta: to map 0x41 → glyph 1, delta = 1 - 0x41 = -0x40 = 0xFFC0

subtable[24] = 0xFF; subtable[25] = 0xC0;

// Sentinel delta keeps 0xFFFF mapping to glyph 0.

subtable[26] = 0x00; subtable[27] = 0x01;

Copilot · 2026-04-03T17:26:21Z

test/glyph.spec.mjs

+    describe('circular composite glyph references', function() {
+        it('does not crash on fonts with circular composite references', function() {
+            const font = loadSync('./test/fonts/circular-composite.ttf');
+            const glyph1 = font.glyphs.get(1);
+            const glyph2 = font.glyphs.get(2);
+            // Should return paths without stack overflow
+            const path1 = glyph1.getPath();
+            const path2 = glyph2.getPath();
+            assert.ok(path1 instanceof Path);
+            assert.ok(path2 instanceof Path);
+        });


This test only exercises glyphs.get(1/2) and doesn't validate the cmap mapping that generate-circular-ref-font.mjs claims (U+0041 → glyph 1). Once the test font’s cmap is corrected, it would be more robust to fetch glyph 1 via font.charToGlyph('A') (and optionally assert its index is 1) before calling getPath(), so the fixture covers both cmap parsing and the circular-composite recursion guard.

… offsets - Use a WeakMap<GlyphSet, Set> instead of a module-scoped Set to avoid theoretical cross-font collisions on shared glyph indices - Fix cmap byte offsets in POC generator (indices 24-27 for idDelta, not 20-23 which are startCode) - Test now uses charToGlyph('A') to also verify cmap mapping https://claude.ai/code/session_014E56osqBcVfPwdgHHtrMBJ

fdb requested a review from Copilot April 3, 2026 17:21

Copilot started reviewing on behalf of fdb April 3, 2026 17:22 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

fdb merged commit f95c049 into master Apr 3, 2026
1 check passed

fdb deleted the claude/fix-glyph-circular-reference-4ADE7 branch April 3, 2026 17:37

fdb mentioned this pull request Apr 3, 2026

Fix circular composite glyph path resolution #823

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent stack overflow from circular composite glyph references#826

Prevent stack overflow from circular composite glyph references#826
fdb merged 2 commits intomasterfrom
claude/fix-glyph-circular-reference-4ADE7

fdb commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

fdb commented Apr 3, 2026

Description

Changes Made:

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants