fix(FUN-4): prevent infinite loop in glyph assembly on degenerate extenders

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 }

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<MTGlyphPart *> *)getGlyphAssemblyFromTable:(NSString*)tableKey forGlyph:(CGGlyph)glyph
 {
     NSDictionary* assemblyTable = (NSDictionary*) _mathTable[tableKey];

iosMathTests/MTTypesetterTest.m

@@ -2725,6 +2725,82 @@ - (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 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.
+- (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;
+    [self glyphForCharacter:'(' name:&glyphName];
+    XCTAssertNotNil(glyphName);
+
+    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;
+    [self glyphForCharacter:'(' name:&glyphName];
+    XCTAssertNotNil(glyphName);
+
+    XCTAssertThrows([self mathTableWithAssemblyKey:@"h_assembly" glyphName:glyphName extenderAdvance:0],
+                    @"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;
+    CGGlyph glyph = [self glyphForCharacter:'(' name:&glyphName];
+    XCTAssertNotNil(glyphName);
+
+    MTFontMathTable* table = [self mathTableWithAssemblyKey:@"v_assembly" glyphName:glyphName extenderAdvance:50];
+    NSArray<MTGlyphPart*>* parts = [table getVerticalGlyphAssemblyForGlyph:glyph];
+    XCTAssertEqual(parts.count, 3u, @"a well-formed assembly must be returned unchanged");
+}
+
 // REN-3: \color atoms must receive inter-element spacing before the colored sub-display.
 // Before the fix, the colored group abutted the preceding binary operator with no gap.
 // After the fix, the medium binary-operator→ordinary gap (4 mu) separates them.