From 4695752c78a0e1002dc382fb803d8deb2968d7cc Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 12 Jun 2026 15:00:31 +0530 Subject: [PATCH 1/6] test(FUN-4): add glyph-assembly loop-termination tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MTGlyphAssemblyTest covers three cases: - zero-advance extender with flanking fixed parts never exits in the original loop (hang-class bug); the new test asserts the method returns a non-empty best-effort assembly. - positive-advance extender reaches the requested height via normal branch A exit. - tall \left(...\right) with a bundled font (regression guard: no behaviour change on valid MATH data). Two test-only headers are added to expose private seams: - MTTypesetter+Testing.h — declares initWithFont:style:cramped:spaced: and constructGlyphWithParts:height:glyphs:offsets:height: - MTGlyphPart+Testing.h — redeclares writable properties so tests can build synthetic MTGlyphPart instances. Co-Authored-By: Claude Sonnet 4.6 --- iosMath/render/internal/MTGlyphPart+Testing.h | 21 ++ .../render/internal/MTTypesetter+Testing.h | 34 ++++ iosMathTests/MTGlyphAssemblyTest.m | 179 ++++++++++++++++++ 3 files changed, 234 insertions(+) create mode 100644 iosMath/render/internal/MTGlyphPart+Testing.h create mode 100644 iosMath/render/internal/MTTypesetter+Testing.h create mode 100644 iosMathTests/MTGlyphAssemblyTest.m diff --git a/iosMath/render/internal/MTGlyphPart+Testing.h b/iosMath/render/internal/MTGlyphPart+Testing.h new file mode 100644 index 00000000..c7c74490 --- /dev/null +++ b/iosMath/render/internal/MTGlyphPart+Testing.h @@ -0,0 +1,21 @@ +// +// MTGlyphPart+Testing.h +// iosMath +// +// Redeclares MTGlyphPart's writable properties (originally in the class +// extension inside MTFontMathTable.m) so test code can build synthetic +// MTGlyphPart instances for unit-testing constructGlyphWithParts: (FUN-4). +// NOT part of the public API; include only from test targets. +// + +#import "MTFontMathTable.h" + +@interface MTGlyphPart (Testing) + +@property (nonatomic) CGGlyph glyph; +@property (nonatomic) CGFloat fullAdvance; +@property (nonatomic) CGFloat startConnectorLength; +@property (nonatomic) CGFloat endConnectorLength; +@property (nonatomic) BOOL isExtender; + +@end diff --git a/iosMath/render/internal/MTTypesetter+Testing.h b/iosMath/render/internal/MTTypesetter+Testing.h new file mode 100644 index 00000000..4a4486e3 --- /dev/null +++ b/iosMath/render/internal/MTTypesetter+Testing.h @@ -0,0 +1,34 @@ +// +// MTTypesetter+Testing.h +// iosMath +// +// Exposes the private -constructGlyphWithParts:height:glyphs:offsets:height: +// method for unit-testing the loop-termination guard (FUN-4). +// NOT part of the public API; include only from test targets. +// + +#import "MTTypesetter.h" +#import "MTFontMathTable.h" + +NS_ASSUME_NONNULL_BEGIN + +@interface MTTypesetter (Testing) + +/// Designated initializer (normally private). Exposed so tests can construct a +/// fully initialised instance (with _styleFont set) for synthetic-parts testing. +- (instancetype)initWithFont:(MTFont *)font + style:(MTLineStyle)style + cramped:(BOOL)cramped + spaced:(BOOL)spaced; + +/// Exposed for unit testing the no-progress / iteration-cap guard (FUN-4). +/// Builds a glyph assembly from the given parts stretched to glyphHeight. +- (void)constructGlyphWithParts:(NSArray *)parts + height:(CGFloat)glyphHeight + glyphs:(NSArray *_Nonnull __autoreleasing *_Nonnull)glyphs + offsets:(NSArray *_Nonnull __autoreleasing *_Nonnull)offsets + height:(CGFloat *)height; + +@end + +NS_ASSUME_NONNULL_END diff --git a/iosMathTests/MTGlyphAssemblyTest.m b/iosMathTests/MTGlyphAssemblyTest.m new file mode 100644 index 00000000..1c5686b7 --- /dev/null +++ b/iosMathTests/MTGlyphAssemblyTest.m @@ -0,0 +1,179 @@ +// +// MTGlyphAssemblyTest.m +// iosMath +// +// Tests for the glyph-assembly loop-termination guard introduced in FUN-4. +// Without the guard, testConstructGlyphWithZeroAdvanceExtenderTerminates hangs +// indefinitely; with the guard it returns a best-effort assembly promptly. +// + +#import + +#import "MTTypesetter.h" +#import "MTTypesetter+Testing.h" +#import "MTFontMathTable.h" +#import "MTGlyphPart+Testing.h" +#import "MTFont.h" +#import "MTFontManager.h" +#import "MTMathList.h" +#import "MTMathListBuilder.h" +#import "MTMathListDisplay.h" +#import "MTMathListDisplayInternal.h" + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/// Build a synthetic MTGlyphPart with caller-supplied field values. +static MTGlyphPart *makePart(CGGlyph glyph, + CGFloat fullAdvance, + CGFloat startConnector, + CGFloat endConnector, + BOOL extender) +{ + MTGlyphPart *part = [[MTGlyphPart alloc] init]; + part.glyph = glyph; + part.fullAdvance = fullAdvance; + part.startConnectorLength = startConnector; + part.endConnectorLength = endConnector; + part.isExtender = extender; + return part; +} + +// --------------------------------------------------------------------------- + +@interface MTGlyphAssemblyTest : XCTestCase +@property (nonatomic) MTFont *font; +@property (nonatomic) MTTypesetter *typesetter; +@end + +@implementation MTGlyphAssemblyTest + +- (void)setUp { + [super setUp]; + self.font = MTFontManager.fontManager.defaultFont; + // Construct a fully initialised MTTypesetter so _styleFont is set. + // constructGlyphWithParts: reads _styleFont.mathTable.minConnectorOverlap + // to determine the minimum connector distance; without _styleFont it crashes. + self.typesetter = [[MTTypesetter alloc] initWithFont:self.font + style:kMTLineStyleDisplay + cramped:NO + spaced:NO]; +} + +- (void)tearDown { + [super tearDown]; +} + +// --------------------------------------------------------------------------- +// RED test: zero-advance extender must not hang (FUN-4) +// --------------------------------------------------------------------------- + +/// Regression guard for FUN-4. +/// +/// A parts list with two non-extender parts (each fullAdvance = 10 pt) flanking +/// a zero-advance extender simulates malformed font MATH data. The flanking +/// parts constrain maxDelta (via their connector lengths), so neither normal +/// exit branch ever fires: +/// - minHeight stays flat each time we add more zero-advance extenders. +/// - maxHeight stays flat or shrinks (maxDelta ≤ 0 once extender joints are +/// factored in), so branch B never becomes reachable. +/// Without the no-progress guard the loop spins forever. With the guard the +/// method returns a best-effort assembly promptly. +/// +/// NOTE: XCTest has a default per-test timeout of 60 s (CI kills after ~10 min). +/// Without the fix this test is caught by the timeout and shows as a failure/hang; +/// with the fix it returns in microseconds. +- (void)testConstructGlyphWithZeroAdvanceExtenderTerminates +{ + // Two fixed (non-extender) parts with connector lengths that constrain + // maxDelta once the extender's joints are added. connector=5 pt, + // minConnectorOverlap (from the font) is ~0.4 pt, so maxDelta per joint + // ≈ 0. The zero-advance extender contributes nothing to minOffset. + MTGlyphPart *partLeft = makePart(1, 10.0, 0.0, 5.0, NO); // endConnector=5 + MTGlyphPart *extender = makePart(2, 0.0, 0.0, 0.0, YES); // fullAdvance=0 — degenerate + MTGlyphPart *partRight = makePart(3, 10.0, 5.0, 0.0, NO); // startConnector=5 + + NSArray *parts = @[ partLeft, extender, partRight ]; + + NSArray *glyphs = nil; + NSArray *offsets = nil; + CGFloat height = 0; + + // Request a height far beyond what these degenerate parts can ever reach. + [self.typesetter constructGlyphWithParts:parts + height:1000.0 + glyphs:&glyphs + offsets:&offsets + height:&height]; + + // The method must return with a usable (non-empty) assembly. + XCTAssertNotNil(glyphs, @"glyphs must not be nil after degenerate-extender assembly"); + XCTAssertNotNil(offsets, @"offsets must not be nil after degenerate-extender assembly"); + XCTAssertGreaterThan(glyphs.count, 0u, @"best-effort assembly must contain at least one glyph"); + XCTAssertEqual(glyphs.count, offsets.count, @"glyph/offset arrays must be same length"); + // height is whatever the degenerate font could produce — just verify it's finite and >= 0. + XCTAssertGreaterThanOrEqual(height, 0.0, @"returned height must be non-negative"); + XCTAssertTrue(isfinite(height), @"returned height must be finite"); +} + +/// Complementary check: with a positive-advance extender the normal exit branch +/// fires and minHeight >= glyphHeight (branch A) or the spread-delta branch B. +- (void)testConstructGlyphWithPositiveAdvanceExtenderProducesAdequateHeight +{ + // Non-extender: 10 pt base. + MTGlyphPart *base = makePart(1, 10.0, 0.0, 0.0, NO); + // Extender with a real advance (5 pt). Requesting 100 pt means we need + // roughly 18+ copies; the loop should terminate normally via branch A. + MTGlyphPart *extender = makePart(2, 5.0, 0.0, 0.0, YES); + + NSArray *parts = @[ base, extender ]; + + NSArray *glyphs = nil; + NSArray *offsets = nil; + CGFloat height = 0; + CGFloat requestedHeight = 100.0; + + [self.typesetter constructGlyphWithParts:parts + height:requestedHeight + glyphs:&glyphs + offsets:&offsets + height:&height]; + + XCTAssertNotNil(glyphs); + XCTAssertNotNil(offsets); + XCTAssertGreaterThan(glyphs.count, 0u); + XCTAssertEqual(glyphs.count, offsets.count); + // Normal exit: assembled height must reach (or exceed) the requested height. + XCTAssertGreaterThanOrEqual(height, requestedHeight, + @"normal exit: assembled height %g must satisfy requested height %g", + height, requestedHeight); +} + +// --------------------------------------------------------------------------- +// Regression guard: bundled-font tall delimiter still renders (no behaviour change) +// --------------------------------------------------------------------------- + +/// Renders a tall \left( ... \right) that forces glyph assembly (not just +/// variant selection) and asserts the display has positive dimensions. +/// This proves the no-progress guard does not perturb the common path with +/// well-formed font MATH data. +- (void)testTallDelimiterRendersWithBundledFont +{ + MTFont *font = MTFontManager.fontManager.defaultFont; + // A fraction tall enough to force glyph assembly for the parentheses. + NSString *latex = @"\\left( \\frac{\\frac{1}{2}}{\\frac{3}{4}} \\right)"; + MTMathList *list = [MTMathListBuilder buildFromString:latex]; + XCTAssertNotNil(list); + + MTMathListDisplay *display = [MTTypesetter createLineForMathList:list + font:font + style:kMTLineStyleDisplay]; + XCTAssertNotNil(display); + XCTAssertGreaterThan(display.ascent + display.descent, 0, + @"tall delimiter display must have positive height"); + XCTAssertGreaterThan(display.width, 0, + @"tall delimiter display must have positive width"); +} + +@end From cfc939bc1ef98967a4f814befcd919f3e799f745 Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 12 Jun 2026 15:00:48 +0530 Subject: [PATCH 2/6] fix(FUN-4): prevent infinite loop in constructGlyphWithParts: on degenerate extenders The loop `for (int numExtenders = 0; true; numExtenders++)` in -[MTTypesetter constructGlyphWithParts:height:glyphs:offsets:height:] only exits when the assembled minHeight (or maxHeight via spread-delta) reaches the requested glyphHeight. A font plist that supplies an extender part with fullAdvance == 0 (or whose advance is fully cancelled by connector overlap) gives minOffsetDelta <= 0 each iteration, so minHeight never grows. With a sufficiently large target neither branch A nor branch B fires and the loop spins forever on the UI thread. Fix: two-layer guard, both local to constructGlyphWithParts:. 1. No-progress guard: after both normal exit branches fail, if neither minHeight nor maxHeight grew by at least 0.01 pt vs the previous iteration, the font data is degenerate. Return the best-effort assembly (most extenders we can usefully add) rather than looping. Note: checking minHeight alone is insufficient. Some valid fonts (e.g. arrowright in Latin Modern Math) have extenders where endConnector == startConnector == fullAdvance, so extender-to-extender joints contribute zero to minOffset and minHeight stays flat while maxHeight grows each iteration until branch B fires. The guard therefore requires BOTH minHeight AND maxHeight to stall. 2. Hard iteration cap (belt-and-suspenders): `numExtenders <= 10000` caps the loop unconditionally, covering degenerate arithmetic that the progress check might miss. 10 000 extenders is far beyond any realistic delimiter stretch at any font size. Net change: ~18 LOC added, 1 LOC changed (for-header), 0 removed. One file (MTTypesetter.m). No header or API changes. Valid fonts are byte-for-byte unaffected. Fixes FUN-4. Co-Authored-By: Claude Sonnet 4.6 --- iosMath/render/internal/MTTypesetter.m | 73 ++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/iosMath/render/internal/MTTypesetter.m b/iosMath/render/internal/MTTypesetter.m index 6aa2b55b..cfd6f1d8 100644 --- a/iosMath/render/internal/MTTypesetter.m +++ b/iosMath/render/internal/MTTypesetter.m @@ -1582,16 +1582,51 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) { NSParameterAssert(glyphs); NSParameterAssert(offsets); - - for (int numExtenders = 0; true; numExtenders++) { + + // Guard against malformed font MATH data (FUN-4): if extender parts have a + // zero or degenerate fullAdvance, adding more copies never increases the + // assembled height and the loop would spin forever on the UI thread. + // + // Two complementary defenses: + // 1. No-progress guard: after both normal exit branches fail, if neither + // minHeight nor maxHeight increased by at least kMinHeightProgress over + // the previous iteration, the font data is degenerate — stop and return + // the best-effort assembly built so far. + // NOTE: checking only minHeight is insufficient. Some valid fonts have + // extenders where endConnector == startConnector == fullAdvance (e.g. + // arrowright in Latin Modern Math), causing each extender-to-extender + // joint to contribute zero to minOffset. minHeight stays flat across + // iterations for those fonts, but maxHeight grows normally and branch B + // eventually fires. The guard must require BOTH to stall. + // 2. Hard iteration cap (belt-and-suspenders): stop after kMaxExtenders + // regardless, covering any degenerate arithmetic the progress check + // might miss. + // + // Well-formed fonts are unaffected: a valid extender produces either growing + // minHeight (branch A path) or growing maxHeight (branch B path), so neither + // guard is ever reached. + static const int kMaxExtenders = 10000; + static const CGFloat kMinHeightProgress = 0.01; // pt; far below any real extender advance + + // Track heights from the previous iteration to detect non-progress. + CGFloat prevMinHeight = -CGFLOAT_MAX; + CGFloat prevMaxHeight = -CGFLOAT_MAX; + + // Hoist last-built arrays so the hard-cap fallback (after the loop) can + // return them. They are nil until the first iteration with a non-nil prev. + NSArray* lastGlyphs = nil; + NSArray* lastOffsets = nil; + CGFloat lastMinHeight = 0; + + for (int numExtenders = 0; numExtenders <= kMaxExtenders; numExtenders++) { NSMutableArray* glyphsRv = [NSMutableArray array]; NSMutableArray* offsetsRv = [NSMutableArray array]; - + MTGlyphPart* prev = nil; CGFloat minDistance = _styleFont.mathTable.minConnectorOverlap; CGFloat minOffset = 0; CGFloat maxDelta = CGFLOAT_MAX; // the maximum amount we can increase the offsets by - + for (MTGlyphPart* part in parts) { int repeats = 1; if (part.isExtender) { @@ -1614,7 +1649,7 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) prev = part; } } - + NSAssert(glyphsRv.count == offsetsRv.count, @"Offsets should match the glyphs"); if (!prev) { continue; // maybe only extenders @@ -1643,6 +1678,34 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) *height = lastOffset + prev.fullAdvance; return; } + + // Neither exit branch fired. Save this iteration's assembly as the + // best effort built so far, then apply the no-progress guard. + lastGlyphs = glyphsRv; + lastOffsets = offsetsRv; + lastMinHeight = minHeight; + + if (numExtenders > 0 + && minHeight - prevMinHeight < kMinHeightProgress + && maxHeight - prevMaxHeight < kMinHeightProgress) { + // Adding more extenders is growing neither the minimum nor the + // maximum assembled height — degenerate font data. Return the + // best-effort assembly rather than looping forever. + *glyphs = lastGlyphs; + *offsets = lastOffsets; + *height = lastMinHeight; + return; + } + prevMinHeight = minHeight; + prevMaxHeight = maxHeight; + } + + // Hit the hard iteration cap (extremely degenerate font data). + // Return the last assembly we successfully built. + if (lastGlyphs) { + *glyphs = lastGlyphs; + *offsets = lastOffsets; + *height = lastMinHeight; } } From a0e1beb0cc6bce7f70c0fbb52c580e0b6b301a4f Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 12 Jun 2026 15:08:58 +0530 Subject: [PATCH 3/6] test(FUN-4): wire MTGlyphAssemblyTest.m into iosMath.xcodeproj Add the glyph-assembly loop-termination tests to the iosMathTests target so they run under the iOS Xcode test job, not only SPM swift test. Without this the infinite-loop regression guard was silently skipped on iOS. Co-Authored-By: Claude Opus 4.8 --- iosMath.xcodeproj/project.pbxproj | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/iosMath.xcodeproj/project.pbxproj b/iosMath.xcodeproj/project.pbxproj index 1db284c9..64dc08d8 100644 --- a/iosMath.xcodeproj/project.pbxproj +++ b/iosMath.xcodeproj/project.pbxproj @@ -26,6 +26,7 @@ 4987307617D546800041B02B /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49965F0117CBBA2700A555C5 /* Foundation.framework */; }; 498730A417D547450041B02B /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 49965F2617CBBA2700A555C5 /* InfoPlist.strings */; }; 498730A717D548190041B02B /* MTMathListBuilderTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 49B83EEF17CE71AC0014B739 /* MTMathListBuilderTest.m */; }; + C0A55E3B20260612000001 /* MTGlyphAssemblyTest.m in Sources */ = {isa = PBXBuildFile; fileRef = C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */; }; 498730A817D548190041B02B /* MTMathListTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 49B83EF517CF046A0014B739 /* MTMathListTest.m */; }; 498730A917D548CA0041B02B /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49965EFF17CBBA2700A555C5 /* UIKit.framework */; }; 498730AA17D548D40041B02B /* CoreText.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49B83EF217CE74620014B739 /* CoreText.framework */; }; @@ -101,6 +102,7 @@ 49965F2717CBBA2700A555C5 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = en; path = en.lproj/InfoPlist.strings; sourceTree = ""; }; 49A27B0218EBF5B30030B7EF /* CoreFoundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreFoundation.framework; path = System/Library/Frameworks/CoreFoundation.framework; sourceTree = SDKROOT; }; 49B83EEF17CE71AC0014B739 /* MTMathListBuilderTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTMathListBuilderTest.m; sourceTree = ""; }; + C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTGlyphAssemblyTest.m; sourceTree = ""; }; 49B83EF217CE74620014B739 /* CoreText.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreText.framework; path = System/Library/Frameworks/CoreText.framework; sourceTree = SDKROOT; }; 49B83EF517CF046A0014B739 /* MTMathListTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTMathListTest.m; sourceTree = ""; }; 49B83EF817CF2AE90014B739 /* AppDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AppDelegate.h; path = example/AppDelegate.h; sourceTree = ""; }; @@ -250,6 +252,7 @@ 49B83EF517CF046A0014B739 /* MTMathListTest.m */, 490465BE1D23DA8400F82033 /* MTTypesetterTest.m */, 490465C01D23DA8400F82033 /* MTFontManagerTest.m */, + C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */, 49965F2417CBBA2700A555C5 /* Supporting Files */, ); path = iosMathTests; @@ -516,6 +519,7 @@ 498730A817D548190041B02B /* MTMathListTest.m in Sources */, 490465BF1D23DA8400F82033 /* MTTypesetterTest.m in Sources */, 490465C11D23DA8400F82033 /* MTFontManagerTest.m in Sources */, + C0A55E3B20260612000001 /* MTGlyphAssemblyTest.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; From 95bf382a9b20ce153ed968ed20aa712e92150ea3 Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 26 Jun 2026 01:19:05 +0530 Subject: [PATCH 4/6] fix(FUN-4): validate glyph assemblies at the boundary instead of guarding the typesetter loop Replace the in-loop infinite-loop guard in constructGlyphWithParts: with validation at the two boundaries where a degenerate assembly can enter: - math_table_to_plist.py: refuse to emit a plist whose extender part has a non-positive advance (names the offending glyph), so bad font data never ships. - MTFontMathTable: reject such an assembly at load time (return nil) for any plist not produced by the script. Callers already fall back to the largest discrete variant when no assembly is available, so the typesetter never sees degenerate parts and cannot hang. This drops the subtle two-layer no-progress heuristic, the test-only private headers, and the synthetic test in favor of a one-time check that fails fast with a clear diagnostic. None of the 8 bundled fonts contain a zero-advance extender, so behavior is unchanged for all real fonts. Tests live in MTTypesetterTest.m (already wired into iosMath.xcodeproj) so they run in both the SPM and iOS xcodebuild CI lanes. Co-Authored-By: Claude Opus 4.8 --- iosMath.xcodeproj/project.pbxproj | 4 - iosMath/fonts/math_table_to_plist.py | 16 ++ iosMath/render/internal/MTFontMathTable.m | 10 + iosMath/render/internal/MTGlyphPart+Testing.h | 21 -- .../render/internal/MTTypesetter+Testing.h | 34 ---- iosMath/render/internal/MTTypesetter.m | 73 +------ iosMathTests/MTGlyphAssemblyTest.m | 179 ------------------ iosMathTests/MTTypesetterTest.m | 67 +++++++ 8 files changed, 98 insertions(+), 306 deletions(-) delete mode 100644 iosMath/render/internal/MTGlyphPart+Testing.h delete mode 100644 iosMath/render/internal/MTTypesetter+Testing.h delete mode 100644 iosMathTests/MTGlyphAssemblyTest.m diff --git a/iosMath.xcodeproj/project.pbxproj b/iosMath.xcodeproj/project.pbxproj index 64dc08d8..1db284c9 100644 --- a/iosMath.xcodeproj/project.pbxproj +++ b/iosMath.xcodeproj/project.pbxproj @@ -26,7 +26,6 @@ 4987307617D546800041B02B /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49965F0117CBBA2700A555C5 /* Foundation.framework */; }; 498730A417D547450041B02B /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 49965F2617CBBA2700A555C5 /* InfoPlist.strings */; }; 498730A717D548190041B02B /* MTMathListBuilderTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 49B83EEF17CE71AC0014B739 /* MTMathListBuilderTest.m */; }; - C0A55E3B20260612000001 /* MTGlyphAssemblyTest.m in Sources */ = {isa = PBXBuildFile; fileRef = C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */; }; 498730A817D548190041B02B /* MTMathListTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 49B83EF517CF046A0014B739 /* MTMathListTest.m */; }; 498730A917D548CA0041B02B /* UIKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49965EFF17CBBA2700A555C5 /* UIKit.framework */; }; 498730AA17D548D40041B02B /* CoreText.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 49B83EF217CE74620014B739 /* CoreText.framework */; }; @@ -102,7 +101,6 @@ 49965F2717CBBA2700A555C5 /* en */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = en; path = en.lproj/InfoPlist.strings; sourceTree = ""; }; 49A27B0218EBF5B30030B7EF /* CoreFoundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreFoundation.framework; path = System/Library/Frameworks/CoreFoundation.framework; sourceTree = SDKROOT; }; 49B83EEF17CE71AC0014B739 /* MTMathListBuilderTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTMathListBuilderTest.m; sourceTree = ""; }; - C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTGlyphAssemblyTest.m; sourceTree = ""; }; 49B83EF217CE74620014B739 /* CoreText.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreText.framework; path = System/Library/Frameworks/CoreText.framework; sourceTree = SDKROOT; }; 49B83EF517CF046A0014B739 /* MTMathListTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = MTMathListTest.m; sourceTree = ""; }; 49B83EF817CF2AE90014B739 /* AppDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AppDelegate.h; path = example/AppDelegate.h; sourceTree = ""; }; @@ -252,7 +250,6 @@ 49B83EF517CF046A0014B739 /* MTMathListTest.m */, 490465BE1D23DA8400F82033 /* MTTypesetterTest.m */, 490465C01D23DA8400F82033 /* MTFontManagerTest.m */, - C0A55E3B20260612000002 /* MTGlyphAssemblyTest.m */, 49965F2417CBBA2700A555C5 /* Supporting Files */, ); path = iosMathTests; @@ -519,7 +516,6 @@ 498730A817D548190041B02B /* MTMathListTest.m in Sources */, 490465BF1D23DA8400F82033 /* MTTypesetterTest.m in Sources */, 490465C11D23DA8400F82033 /* MTFontManagerTest.m in Sources */, - C0A55E3B20260612000001 /* MTGlyphAssemblyTest.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/iosMath/fonts/math_table_to_plist.py b/iosMath/fonts/math_table_to_plist.py index 4ad30cf0..ce26e022 100644 --- a/iosMath/fonts/math_table_to_plist.py +++ b/iosMath/fonts/math_table_to_plist.py @@ -167,6 +167,20 @@ def get_h_variants(math_table): variant_dict[name] = glyph_variants return variant_dict +def validate_parts(glyph_name, parts): + # A glyph assembly stretches by repeating its extender part(s). An extender + # whose FullAdvance is non-positive never increases the assembled height, so + # the renderer's assembly loop would spin forever trying to reach the target + # size. Refuse to emit such a plist rather than ship font data that hangs the + # app (FUN-4). MTFontMathTable applies the same check at load time for plists + # not produced by this script. + for part in parts: + if part["extender"] and part["advance"] <= 0: + raise ValueError( + "Glyph assembly for '%s' has an extender part with " + "non-positive advance (%s); this would hang the renderer." + % (glyph_name, part["advance"])) + def get_v_assembly(math_table): variants = math_table.MathVariants vglyphs = variants.VertGlyphCoverage.glyphs @@ -181,6 +195,7 @@ def get_v_assembly(math_table): # There is an assembly for this glyph italic = assembly.ItalicsCorrection.Value parts = [part_dict(part) for part in assembly.PartRecords] + validate_parts(name, parts) assembly_dict[name] = { "italic" : assembly.ItalicsCorrection.Value, "parts" : parts } @@ -199,6 +214,7 @@ def get_h_assembly(math_table): if assembly is not None: italic = assembly.ItalicsCorrection.Value parts = [part_dict(part) for part in assembly.PartRecords] + validate_parts(name, parts) assembly_dict[name] = { "italic" : assembly.ItalicsCorrection.Value, "parts" : parts } diff --git a/iosMath/render/internal/MTFontMathTable.m b/iosMath/render/internal/MTFontMathTable.m index bb1e5c57..9819cb9d 100644 --- a/iosMath/render/internal/MTFontMathTable.m +++ b/iosMath/render/internal/MTFontMathTable.m @@ -531,6 +531,16 @@ - (CGFloat)minConnectorOverlap NSString* partGlyphName = (NSString*) partInfo[@"glyph"]; part.glyph = [self.font getGlyphWithName:partGlyphName]; + // Guard against malformed MATH data (FUN-4): an extender with a + // non-positive fullAdvance never increases the assembled height, so the + // typesetter's assembly loop would spin forever. Reject the whole + // assembly so the caller falls back to the largest discrete variant. + // The math_table_to_plist.py generator rejects such fonts up front; this + // is the runtime guard for plists not produced by that script. + if (part.isExtender && part.fullAdvance <= 0) { + return nil; + } + [rv addObject:part]; } return rv; diff --git a/iosMath/render/internal/MTGlyphPart+Testing.h b/iosMath/render/internal/MTGlyphPart+Testing.h deleted file mode 100644 index c7c74490..00000000 --- a/iosMath/render/internal/MTGlyphPart+Testing.h +++ /dev/null @@ -1,21 +0,0 @@ -// -// MTGlyphPart+Testing.h -// iosMath -// -// Redeclares MTGlyphPart's writable properties (originally in the class -// extension inside MTFontMathTable.m) so test code can build synthetic -// MTGlyphPart instances for unit-testing constructGlyphWithParts: (FUN-4). -// NOT part of the public API; include only from test targets. -// - -#import "MTFontMathTable.h" - -@interface MTGlyphPart (Testing) - -@property (nonatomic) CGGlyph glyph; -@property (nonatomic) CGFloat fullAdvance; -@property (nonatomic) CGFloat startConnectorLength; -@property (nonatomic) CGFloat endConnectorLength; -@property (nonatomic) BOOL isExtender; - -@end diff --git a/iosMath/render/internal/MTTypesetter+Testing.h b/iosMath/render/internal/MTTypesetter+Testing.h deleted file mode 100644 index 4a4486e3..00000000 --- a/iosMath/render/internal/MTTypesetter+Testing.h +++ /dev/null @@ -1,34 +0,0 @@ -// -// MTTypesetter+Testing.h -// iosMath -// -// Exposes the private -constructGlyphWithParts:height:glyphs:offsets:height: -// method for unit-testing the loop-termination guard (FUN-4). -// NOT part of the public API; include only from test targets. -// - -#import "MTTypesetter.h" -#import "MTFontMathTable.h" - -NS_ASSUME_NONNULL_BEGIN - -@interface MTTypesetter (Testing) - -/// Designated initializer (normally private). Exposed so tests can construct a -/// fully initialised instance (with _styleFont set) for synthetic-parts testing. -- (instancetype)initWithFont:(MTFont *)font - style:(MTLineStyle)style - cramped:(BOOL)cramped - spaced:(BOOL)spaced; - -/// Exposed for unit testing the no-progress / iteration-cap guard (FUN-4). -/// Builds a glyph assembly from the given parts stretched to glyphHeight. -- (void)constructGlyphWithParts:(NSArray *)parts - height:(CGFloat)glyphHeight - glyphs:(NSArray *_Nonnull __autoreleasing *_Nonnull)glyphs - offsets:(NSArray *_Nonnull __autoreleasing *_Nonnull)offsets - height:(CGFloat *)height; - -@end - -NS_ASSUME_NONNULL_END diff --git a/iosMath/render/internal/MTTypesetter.m b/iosMath/render/internal/MTTypesetter.m index cfd6f1d8..6aa2b55b 100644 --- a/iosMath/render/internal/MTTypesetter.m +++ b/iosMath/render/internal/MTTypesetter.m @@ -1582,51 +1582,16 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) { NSParameterAssert(glyphs); NSParameterAssert(offsets); - - // Guard against malformed font MATH data (FUN-4): if extender parts have a - // zero or degenerate fullAdvance, adding more copies never increases the - // assembled height and the loop would spin forever on the UI thread. - // - // Two complementary defenses: - // 1. No-progress guard: after both normal exit branches fail, if neither - // minHeight nor maxHeight increased by at least kMinHeightProgress over - // the previous iteration, the font data is degenerate — stop and return - // the best-effort assembly built so far. - // NOTE: checking only minHeight is insufficient. Some valid fonts have - // extenders where endConnector == startConnector == fullAdvance (e.g. - // arrowright in Latin Modern Math), causing each extender-to-extender - // joint to contribute zero to minOffset. minHeight stays flat across - // iterations for those fonts, but maxHeight grows normally and branch B - // eventually fires. The guard must require BOTH to stall. - // 2. Hard iteration cap (belt-and-suspenders): stop after kMaxExtenders - // regardless, covering any degenerate arithmetic the progress check - // might miss. - // - // Well-formed fonts are unaffected: a valid extender produces either growing - // minHeight (branch A path) or growing maxHeight (branch B path), so neither - // guard is ever reached. - static const int kMaxExtenders = 10000; - static const CGFloat kMinHeightProgress = 0.01; // pt; far below any real extender advance - - // Track heights from the previous iteration to detect non-progress. - CGFloat prevMinHeight = -CGFLOAT_MAX; - CGFloat prevMaxHeight = -CGFLOAT_MAX; - - // Hoist last-built arrays so the hard-cap fallback (after the loop) can - // return them. They are nil until the first iteration with a non-nil prev. - NSArray* lastGlyphs = nil; - NSArray* lastOffsets = nil; - CGFloat lastMinHeight = 0; - - for (int numExtenders = 0; numExtenders <= kMaxExtenders; numExtenders++) { + + for (int numExtenders = 0; true; numExtenders++) { NSMutableArray* glyphsRv = [NSMutableArray array]; NSMutableArray* offsetsRv = [NSMutableArray array]; - + MTGlyphPart* prev = nil; CGFloat minDistance = _styleFont.mathTable.minConnectorOverlap; CGFloat minOffset = 0; CGFloat maxDelta = CGFLOAT_MAX; // the maximum amount we can increase the offsets by - + for (MTGlyphPart* part in parts) { int repeats = 1; if (part.isExtender) { @@ -1649,7 +1614,7 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) prev = part; } } - + NSAssert(glyphsRv.count == offsetsRv.count, @"Offsets should match the glyphs"); if (!prev) { continue; // maybe only extenders @@ -1678,34 +1643,6 @@ - (void) constructGlyphWithParts:(NSArray*) parts height:(CGFloat) *height = lastOffset + prev.fullAdvance; return; } - - // Neither exit branch fired. Save this iteration's assembly as the - // best effort built so far, then apply the no-progress guard. - lastGlyphs = glyphsRv; - lastOffsets = offsetsRv; - lastMinHeight = minHeight; - - if (numExtenders > 0 - && minHeight - prevMinHeight < kMinHeightProgress - && maxHeight - prevMaxHeight < kMinHeightProgress) { - // Adding more extenders is growing neither the minimum nor the - // maximum assembled height — degenerate font data. Return the - // best-effort assembly rather than looping forever. - *glyphs = lastGlyphs; - *offsets = lastOffsets; - *height = lastMinHeight; - return; - } - prevMinHeight = minHeight; - prevMaxHeight = maxHeight; - } - - // Hit the hard iteration cap (extremely degenerate font data). - // Return the last assembly we successfully built. - if (lastGlyphs) { - *glyphs = lastGlyphs; - *offsets = lastOffsets; - *height = lastMinHeight; } } diff --git a/iosMathTests/MTGlyphAssemblyTest.m b/iosMathTests/MTGlyphAssemblyTest.m deleted file mode 100644 index 1c5686b7..00000000 --- a/iosMathTests/MTGlyphAssemblyTest.m +++ /dev/null @@ -1,179 +0,0 @@ -// -// MTGlyphAssemblyTest.m -// iosMath -// -// Tests for the glyph-assembly loop-termination guard introduced in FUN-4. -// Without the guard, testConstructGlyphWithZeroAdvanceExtenderTerminates hangs -// indefinitely; with the guard it returns a best-effort assembly promptly. -// - -#import - -#import "MTTypesetter.h" -#import "MTTypesetter+Testing.h" -#import "MTFontMathTable.h" -#import "MTGlyphPart+Testing.h" -#import "MTFont.h" -#import "MTFontManager.h" -#import "MTMathList.h" -#import "MTMathListBuilder.h" -#import "MTMathListDisplay.h" -#import "MTMathListDisplayInternal.h" - -// --------------------------------------------------------------------------- -// Helpers -// --------------------------------------------------------------------------- - -/// Build a synthetic MTGlyphPart with caller-supplied field values. -static MTGlyphPart *makePart(CGGlyph glyph, - CGFloat fullAdvance, - CGFloat startConnector, - CGFloat endConnector, - BOOL extender) -{ - MTGlyphPart *part = [[MTGlyphPart alloc] init]; - part.glyph = glyph; - part.fullAdvance = fullAdvance; - part.startConnectorLength = startConnector; - part.endConnectorLength = endConnector; - part.isExtender = extender; - return part; -} - -// --------------------------------------------------------------------------- - -@interface MTGlyphAssemblyTest : XCTestCase -@property (nonatomic) MTFont *font; -@property (nonatomic) MTTypesetter *typesetter; -@end - -@implementation MTGlyphAssemblyTest - -- (void)setUp { - [super setUp]; - self.font = MTFontManager.fontManager.defaultFont; - // Construct a fully initialised MTTypesetter so _styleFont is set. - // constructGlyphWithParts: reads _styleFont.mathTable.minConnectorOverlap - // to determine the minimum connector distance; without _styleFont it crashes. - self.typesetter = [[MTTypesetter alloc] initWithFont:self.font - style:kMTLineStyleDisplay - cramped:NO - spaced:NO]; -} - -- (void)tearDown { - [super tearDown]; -} - -// --------------------------------------------------------------------------- -// RED test: zero-advance extender must not hang (FUN-4) -// --------------------------------------------------------------------------- - -/// Regression guard for FUN-4. -/// -/// A parts list with two non-extender parts (each fullAdvance = 10 pt) flanking -/// a zero-advance extender simulates malformed font MATH data. The flanking -/// parts constrain maxDelta (via their connector lengths), so neither normal -/// exit branch ever fires: -/// - minHeight stays flat each time we add more zero-advance extenders. -/// - maxHeight stays flat or shrinks (maxDelta ≤ 0 once extender joints are -/// factored in), so branch B never becomes reachable. -/// Without the no-progress guard the loop spins forever. With the guard the -/// method returns a best-effort assembly promptly. -/// -/// NOTE: XCTest has a default per-test timeout of 60 s (CI kills after ~10 min). -/// Without the fix this test is caught by the timeout and shows as a failure/hang; -/// with the fix it returns in microseconds. -- (void)testConstructGlyphWithZeroAdvanceExtenderTerminates -{ - // Two fixed (non-extender) parts with connector lengths that constrain - // maxDelta once the extender's joints are added. connector=5 pt, - // minConnectorOverlap (from the font) is ~0.4 pt, so maxDelta per joint - // ≈ 0. The zero-advance extender contributes nothing to minOffset. - MTGlyphPart *partLeft = makePart(1, 10.0, 0.0, 5.0, NO); // endConnector=5 - MTGlyphPart *extender = makePart(2, 0.0, 0.0, 0.0, YES); // fullAdvance=0 — degenerate - MTGlyphPart *partRight = makePart(3, 10.0, 5.0, 0.0, NO); // startConnector=5 - - NSArray *parts = @[ partLeft, extender, partRight ]; - - NSArray *glyphs = nil; - NSArray *offsets = nil; - CGFloat height = 0; - - // Request a height far beyond what these degenerate parts can ever reach. - [self.typesetter constructGlyphWithParts:parts - height:1000.0 - glyphs:&glyphs - offsets:&offsets - height:&height]; - - // The method must return with a usable (non-empty) assembly. - XCTAssertNotNil(glyphs, @"glyphs must not be nil after degenerate-extender assembly"); - XCTAssertNotNil(offsets, @"offsets must not be nil after degenerate-extender assembly"); - XCTAssertGreaterThan(glyphs.count, 0u, @"best-effort assembly must contain at least one glyph"); - XCTAssertEqual(glyphs.count, offsets.count, @"glyph/offset arrays must be same length"); - // height is whatever the degenerate font could produce — just verify it's finite and >= 0. - XCTAssertGreaterThanOrEqual(height, 0.0, @"returned height must be non-negative"); - XCTAssertTrue(isfinite(height), @"returned height must be finite"); -} - -/// Complementary check: with a positive-advance extender the normal exit branch -/// fires and minHeight >= glyphHeight (branch A) or the spread-delta branch B. -- (void)testConstructGlyphWithPositiveAdvanceExtenderProducesAdequateHeight -{ - // Non-extender: 10 pt base. - MTGlyphPart *base = makePart(1, 10.0, 0.0, 0.0, NO); - // Extender with a real advance (5 pt). Requesting 100 pt means we need - // roughly 18+ copies; the loop should terminate normally via branch A. - MTGlyphPart *extender = makePart(2, 5.0, 0.0, 0.0, YES); - - NSArray *parts = @[ base, extender ]; - - NSArray *glyphs = nil; - NSArray *offsets = nil; - CGFloat height = 0; - CGFloat requestedHeight = 100.0; - - [self.typesetter constructGlyphWithParts:parts - height:requestedHeight - glyphs:&glyphs - offsets:&offsets - height:&height]; - - XCTAssertNotNil(glyphs); - XCTAssertNotNil(offsets); - XCTAssertGreaterThan(glyphs.count, 0u); - XCTAssertEqual(glyphs.count, offsets.count); - // Normal exit: assembled height must reach (or exceed) the requested height. - XCTAssertGreaterThanOrEqual(height, requestedHeight, - @"normal exit: assembled height %g must satisfy requested height %g", - height, requestedHeight); -} - -// --------------------------------------------------------------------------- -// Regression guard: bundled-font tall delimiter still renders (no behaviour change) -// --------------------------------------------------------------------------- - -/// Renders a tall \left( ... \right) that forces glyph assembly (not just -/// variant selection) and asserts the display has positive dimensions. -/// This proves the no-progress guard does not perturb the common path with -/// well-formed font MATH data. -- (void)testTallDelimiterRendersWithBundledFont -{ - MTFont *font = MTFontManager.fontManager.defaultFont; - // A fraction tall enough to force glyph assembly for the parentheses. - NSString *latex = @"\\left( \\frac{\\frac{1}{2}}{\\frac{3}{4}} \\right)"; - MTMathList *list = [MTMathListBuilder buildFromString:latex]; - XCTAssertNotNil(list); - - MTMathListDisplay *display = [MTTypesetter createLineForMathList:list - font:font - style:kMTLineStyleDisplay]; - XCTAssertNotNil(display); - XCTAssertGreaterThan(display.ascent + display.descent, 0, - @"tall delimiter display must have positive height"); - XCTAssertGreaterThan(display.width, 0, - @"tall delimiter display must have positive width"); -} - -@end diff --git a/iosMathTests/MTTypesetterTest.m b/iosMathTests/MTTypesetterTest.m index af6df427..13005593 100644 --- a/iosMathTests/MTTypesetterTest.m +++ b/iosMathTests/MTTypesetterTest.m @@ -2725,4 +2725,71 @@ - (void)testOversetRowRendersAtScriptScriptWhenNestedInSuperscript XCTAssertLessThan(nestedStack.over.ascent, baselineStack.over.ascent); } +#pragma mark - Glyph assembly validation (FUN-4) + +// A glyph assembly whose extender part has a non-positive fullAdvance is +// degenerate: adding more copies of the extender never increases the assembled +// height, which would make MTTypesetter's assembly loop spin forever. The font +// math table must reject such an assembly at load time (returning nil) so the +// typesetter falls back to the largest discrete variant instead of hanging. +// The bundled fonts contain no such assembly, so these tests build a synthetic +// math table to exercise the guard. + +// Returns a real glyph from the bundled font (and its round-tripped name) so the +// synthetic assembly is keyed exactly as -getGlyphAssemblyFromTable: looks it up. +- (CGGlyph)glyphForCharacter:(unichar)ch name:(NSString**)outName +{ + CGGlyph glyph = 0; + CTFontGetGlyphsForCharacters(self.font.ctFont, &ch, &glyph, 1); + *outName = [self.font getGlyphName:glyph]; + return glyph; +} + +- (MTFontMathTable*)mathTableWithAssemblyKey:(NSString*)key + glyphName:(NSString*)glyphName + extenderAdvance:(int)extenderAdvance +{ + NSDictionary* startPart = @{ @"advance": @(100), @"startConnector": @(0), @"endConnector": @(20), @"extender": @NO, @"glyph": glyphName }; + NSDictionary* extender = @{ @"advance": @(extenderAdvance), @"startConnector": @(20), @"endConnector": @(20), @"extender": @YES, @"glyph": glyphName }; + NSDictionary* endPart = @{ @"advance": @(100), @"startConnector": @(20), @"endConnector": @(0), @"extender": @NO, @"glyph": glyphName }; + NSDictionary* mathTable = @{ + @"version": @"1.4", + key: @{ glyphName: @{ @"italic": @(0), @"parts": @[ startPart, extender, endPart ] } } + }; + return [[MTFontMathTable alloc] initWithFont:self.font mathTable:mathTable]; +} + +- (void)testVerticalGlyphAssemblyWithZeroAdvanceExtenderIsRejected +{ + NSString* glyphName = nil; + CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName]; + XCTAssertNotNil(glyphName); + + MTFontMathTable* table = [self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:0]; + NSArray* parts = [table getVerticalGlyphAssemblyForGlyph:glyph]; + XCTAssertNil(parts, @"a vertical assembly with a zero-advance extender must be rejected"); +} + +- (void)testHorizontalGlyphAssemblyWithZeroAdvanceExtenderIsRejected +{ + NSString* glyphName = nil; + CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName]; + XCTAssertNotNil(glyphName); + + MTFontMathTable* table = [self mathTableWithAssemblyKey:@"h_assembly" glyphName:glyphName extenderAdvance:0]; + NSArray* parts = [table getHorizontalGlyphAssemblyForGlyph:glyph]; + XCTAssertNil(parts, @"a horizontal assembly with a zero-advance extender must be rejected"); +} + +- (void)testValidGlyphAssemblyIsAccepted +{ + NSString* glyphName = nil; + CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName]; + XCTAssertNotNil(glyphName); + + MTFontMathTable* table = [self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:50]; + NSArray* parts = [table getVerticalGlyphAssemblyForGlyph:glyph]; + XCTAssertEqual(parts.count, 3u, @"a well-formed assembly must be returned unchanged"); +} + @end From ed4cc0baa2957faaa29f87e0e5b7b86b3162e4ae Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 26 Jun 2026 01:32:41 +0530 Subject: [PATCH 5/6] fix(FUN-4): throw on malformed glyph assembly at load instead of returning nil MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: a zero-advance extender is malformed plist data, the same category as an invalid plist version — which MTFontMathTable already rejects by throwing. Treat it consistently and fail loud rather than silently mis-rendering. - Validate all v_assembly/h_assembly entries in -initWithFont:mathTable: and throw NSException (naming the offending glyph) if an extender has a non-positive advance. Validating at load — not lazily in getGlyphAssemblyFromTable: — keeps the exception out of the per-glyph typesetting loop (UI thread, not wrapped in @try) and fails deterministically before any rendering. - Drop the lazy nil guard added previously. - Tests updated to assert construction throws for a degenerate extender. Co-Authored-By: Claude Opus 4.8 --- iosMath/render/internal/MTFontMathTable.m | 36 ++++++++++++++++------- iosMathTests/MTTypesetterTest.m | 22 +++++++------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/iosMath/render/internal/MTFontMathTable.m b/iosMath/render/internal/MTFontMathTable.m index 9819cb9d..a95b00eb 100644 --- a/iosMath/render/internal/MTFontMathTable.m +++ b/iosMath/render/internal/MTFontMathTable.m @@ -58,6 +58,7 @@ - (instancetype)initWithFont:(nonnull MTFont*) font mathTable:(nonnull NSDiction reason:[NSString stringWithFormat:@"Invalid version of math table plist: %@", _mathTable[@"version"]] userInfo:nil]; } + [self validateGlyphAssemblies]; } return self; } @@ -505,6 +506,31 @@ - (CGFloat)minConnectorOverlap static NSString* const kHorizAssembly = @"h_assembly"; static NSString* const kAssemblyParts = @"parts"; +// Reject malformed glyph-assembly data at load time (FUN-4). An extender part +// with a non-positive advance never increases the assembled height, so the +// typesetter's assembly loop would spin forever trying to reach the requested +// size. We throw here — mirroring the invalid-version check in init and the +// math_table_to_plist.py generator's own check — so a bad plist fails loudly and +// deterministically at load rather than mis-rendering or hanging mid-typeset. +- (void) validateGlyphAssemblies +{ + for (NSString* tableKey in @[kVertAssembly, kHorizAssembly]) { + NSDictionary* assemblyTable = (NSDictionary*) _mathTable[tableKey]; + for (NSString* glyphName in assemblyTable) { + NSArray* parts = (NSArray*) assemblyTable[glyphName][kAssemblyParts]; + for (NSDictionary* partInfo in parts) { + BOOL isExtender = [(NSNumber*) partInfo[@"extender"] boolValue]; + int advance = [(NSNumber*) partInfo[@"advance"] intValue]; + if (isExtender && advance <= 0) { + @throw [NSException exceptionWithName:NSInternalInconsistencyException + reason:[NSString stringWithFormat:@"Glyph assembly for '%@' has an extender part with non-positive advance (%d); this would hang the renderer.", glyphName, advance] + userInfo:nil]; + } + } + } + } +} + - (NSArray *)getGlyphAssemblyFromTable:(NSString*)tableKey forGlyph:(CGGlyph)glyph { NSDictionary* assemblyTable = (NSDictionary*) _mathTable[tableKey]; @@ -531,16 +557,6 @@ - (CGFloat)minConnectorOverlap NSString* partGlyphName = (NSString*) partInfo[@"glyph"]; part.glyph = [self.font getGlyphWithName:partGlyphName]; - // Guard against malformed MATH data (FUN-4): an extender with a - // non-positive fullAdvance never increases the assembled height, so the - // typesetter's assembly loop would spin forever. Reject the whole - // assembly so the caller falls back to the largest discrete variant. - // The math_table_to_plist.py generator rejects such fonts up front; this - // is the runtime guard for plists not produced by that script. - if (part.isExtender && part.fullAdvance <= 0) { - return nil; - } - [rv addObject:part]; } return rv; diff --git a/iosMathTests/MTTypesetterTest.m b/iosMathTests/MTTypesetterTest.m index 13005593..f1430e4a 100644 --- a/iosMathTests/MTTypesetterTest.m +++ b/iosMathTests/MTTypesetterTest.m @@ -2730,10 +2730,10 @@ - (void)testOversetRowRendersAtScriptScriptWhenNestedInSuperscript // A glyph assembly whose extender part has a non-positive fullAdvance is // degenerate: adding more copies of the extender never increases the assembled // height, which would make MTTypesetter's assembly loop spin forever. The font -// math table must reject such an assembly at load time (returning nil) so the -// typesetter falls back to the largest discrete variant instead of hanging. -// The bundled fonts contain no such assembly, so these tests build a synthetic -// math table to exercise the guard. +// math table must reject such a plist at load time by throwing, the same way it +// already throws for an invalid plist version, rather than silently mis-rendering +// the glyph. The bundled fonts contain no such assembly, so these tests build a +// synthetic math table to exercise the guard. // Returns a real glyph from the bundled font (and its round-tripped name) so the // synthetic assembly is keyed exactly as -getGlyphAssemblyFromTable: looks it up. @@ -2762,23 +2762,21 @@ - (MTFontMathTable*)mathTableWithAssemblyKey:(NSString*)key - (void)testVerticalGlyphAssemblyWithZeroAdvanceExtenderIsRejected { NSString* glyphName = nil; - CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName]; + [self glyphForCharacter:'(' name:&glyphName]; XCTAssertNotNil(glyphName); - MTFontMathTable* table = [self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:0]; - NSArray* parts = [table getVerticalGlyphAssemblyForGlyph:glyph]; - XCTAssertNil(parts, @"a vertical assembly with a zero-advance extender must be rejected"); + XCTAssertThrows([self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:0], + @"a vertical assembly with a zero-advance extender must be rejected at load"); } - (void)testHorizontalGlyphAssemblyWithZeroAdvanceExtenderIsRejected { NSString* glyphName = nil; - CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName]; + [self glyphForCharacter:'(' name:&glyphName]; XCTAssertNotNil(glyphName); - MTFontMathTable* table = [self mathTableWithAssemblyKey:@"h_assembly" glyphName:glyphName extenderAdvance:0]; - NSArray* parts = [table getHorizontalGlyphAssemblyForGlyph:glyph]; - XCTAssertNil(parts, @"a horizontal assembly with a zero-advance extender must be rejected"); + XCTAssertThrows([self mathTableWithAssemblyKey:@"h_assembly" glyphName:glyphName extenderAdvance:0], + @"a horizontal assembly with a zero-advance extender must be rejected at load"); } - (void)testValidGlyphAssemblyIsAccepted From f603652e97d89cbe06c81c38250b233302e93e43 Mon Sep 17 00:00:00 2001 From: Kostub D Date: Sun, 28 Jun 2026 15:57:09 +0530 Subject: [PATCH 6/6] test(FUN-4): cover negative-advance extender (full <=0 condition) Co-Authored-By: Claude Opus 4.8 --- iosMathTests/MTTypesetterTest.m | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/iosMathTests/MTTypesetterTest.m b/iosMathTests/MTTypesetterTest.m index 2b3bf454..4c946f5e 100644 --- a/iosMathTests/MTTypesetterTest.m +++ b/iosMathTests/MTTypesetterTest.m @@ -2779,6 +2779,17 @@ - (void)testHorizontalGlyphAssemblyWithZeroAdvanceExtenderIsRejected @"a horizontal assembly with a zero-advance extender must be rejected at load"); } +- (void)testGlyphAssemblyWithNegativeAdvanceExtenderIsRejected +{ + NSString* glyphName = nil; + [self glyphForCharacter:'(' name:&glyphName]; + XCTAssertNotNil(glyphName); + + // Guard the full `advance <= 0` condition, not just the zero boundary. + XCTAssertThrows([self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:-10], + @"an assembly with a negative-advance extender must be rejected at load"); +} + - (void)testValidGlyphAssemblyIsAccepted { NSString* glyphName = nil;