From 97ce205d0462d09e576e4d55b2fed6b77978363d Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 12 Jun 2026 06:40:51 +0530 Subject: [PATCH 1/2] fix(REN-2): make \color/\colorbox validate hex colors and error on invalid input Previously readColor accepted only hex characters, causing named colors like \color{red} to produce a misleading "Missing }" error, and bare hex strings like \color{ff0000} (no leading #) to silently succeed then render uncolored because colorFromHexString: requires a leading #. - Rewrite readColor to read the full token up to }, then validate it against the grammar: '#' followed by exactly 3 or 6 hex digits (#RGB / #RRGGBB). Invalid colors (named colors, missing #, wrong length, non-hex digits) now set MTParseErrorInvalidCommand and return nil instead of silently succeeding. - Fix the \color and \colorbox command handlers to propagate nil from readColor instead of continuing to build with an invalid colorString. - Add test coverage for valid parses (#ff0000, #f00, colorbox) and all invalid cases (named color, missing #, non-hex digit, wrong length) asserting nil list and MTParseErrorInvalidCommand. Co-Authored-By: Claude Sonnet 4.6 --- iosMath/lib/MTMathListBuilder.m | 64 ++++++++++++++++--- iosMathTests/MTMathListBuilderTest.m | 96 ++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 10 deletions(-) diff --git a/iosMath/lib/MTMathListBuilder.m b/iosMath/lib/MTMathListBuilder.m index f524067..ca75360 100644 --- a/iosMath/lib/MTMathListBuilder.m +++ b/iosMath/lib/MTMathListBuilder.m @@ -545,28 +545,62 @@ - (NSString*) readColor [self setError:MTParseErrorCharacterNotFound message:@"Missing {"]; return nil; } - + // Ignore spaces and nonascii. [self skipSpaces]; - // a string of all upper and lower case characters. + // Read the entire token up to the closing brace or whitespace. + // We deliberately do NOT restrict the charset here so that invalid + // inputs (e.g. named colors like "red") are captured whole and can + // produce a clear validation error instead of a confusing "Missing }". NSMutableString* mutable = [NSMutableString string]; while([self hasCharacters]) { unichar ch = [self getNextCharacter]; - if (ch == '#' || (ch >= 'A' && ch <= 'F') || (ch >= 'a' && ch <= 'f') || (ch >= '0' && ch <= '9')) { - [mutable appendString:[NSString stringWithCharacters:&ch length:1]]; - } else { - // we went too far + if (ch == '}') { + // Put the closing brace back; expectCharacter below will consume it. [self unlookCharacter]; break; } + if (ch < 0x21 || ch > 0x7E) { + // Treat whitespace / non-ASCII as end of token (skip over it). + break; + } + [mutable appendString:[NSString stringWithCharacters:&ch length:1]]; } if (![self expectCharacter:'}']) { - // We didn't find an closing brace, so invalid format. + // We didn't find a closing brace, so invalid format. [self setError:MTParseErrorCharacterNotFound message:@"Missing }"]; return nil; } + + // Validate: color must be '#' followed by exactly 3 or 6 hex digits. + // This keeps the grammar consistent with colorFromHexString: which requires + // a leading '#'. Named colors and bare hex strings are not supported. + BOOL valid = NO; + NSUInteger len = mutable.length; + if (len == 4 || len == 7) { + unichar first = [mutable characterAtIndex:0]; + if (first == '#') { + valid = YES; + for (NSUInteger i = 1; i < len && valid; i++) { + unichar c = [mutable characterAtIndex:i]; + BOOL isHex = ((c >= '0' && c <= '9') || + (c >= 'a' && c <= 'f') || + (c >= 'A' && c <= 'F')); + if (!isHex) { + valid = NO; + } + } + } + } + + if (!valid) { + NSString* msg = [NSString stringWithFormat:@"Invalid color: %@", mutable]; + [self setError:MTParseErrorInvalidCommand message:msg]; + return nil; + } + return mutable; } @@ -828,14 +862,24 @@ - (MTMathAtom*) atomForCommand:(NSString*) command return table; } else if ([command isEqualToString:@"color"]) { // A color command has 2 arguments + NSString* colorStr = [self readColor]; + if (!colorStr) { + // readColor already set the error. + return nil; + } MTMathColor* mathColor = [[MTMathColor alloc] init]; - mathColor.colorString = [self readColor]; + mathColor.colorString = colorStr; mathColor.innerList = [self buildInternal:true]; return mathColor; } else if ([command isEqualToString:@"colorbox"]) { - // A color command has 2 arguments + // A colorbox command has 2 arguments + NSString* colorStr = [self readColor]; + if (!colorStr) { + // readColor already set the error. + return nil; + } MTMathColorbox* mathColorbox = [[MTMathColorbox alloc] init]; - mathColorbox.colorString = [self readColor]; + mathColorbox.colorString = colorStr; mathColorbox.innerList = [self buildInternal:true]; return mathColorbox; } else { diff --git a/iosMathTests/MTMathListBuilderTest.m b/iosMathTests/MTMathListBuilderTest.m index 3de1673..be8f69c 100644 --- a/iosMathTests/MTMathListBuilderTest.m +++ b/iosMathTests/MTMathListBuilderTest.m @@ -2866,4 +2866,100 @@ - (void)testProgrammaticBothRowsSerializeNested XCTAssertEqualObjects([MTMathListBuilder mathListToString:list], @"\\underset{b}{\\overset{a}{X}}"); } +#pragma mark - \color and \colorbox tests + +- (void)testColorValidHexSix +{ + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{#ff0000}x" error:&error]; + XCTAssertNil(error, @"Unexpected error: %@", error); + XCTAssertNotNil(list); + XCTAssertEqual(list.atoms.count, (NSUInteger)1); + MTMathColor* colorAtom = (MTMathColor*)list.atoms[0]; + XCTAssertEqual(colorAtom.type, kMTMathAtomColor); + XCTAssertEqualObjects(colorAtom.colorString, @"#ff0000"); + XCTAssertNotNil(colorAtom.innerList); + XCTAssertEqual(colorAtom.innerList.atoms.count, (NSUInteger)1); + // stringValue round-trip (mathListToString uses appendLaTeXToString: which MTMathColor + // inherits from the base class; stringValue is the color-specific round-trip method). + XCTAssertEqualObjects(colorAtom.stringValue, @"\\color{#ff0000}{x}"); +} + +- (void)testColorValidHexThree +{ + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{#f00}x" error:&error]; + XCTAssertNil(error, @"Unexpected error: %@", error); + XCTAssertNotNil(list); + XCTAssertEqual(list.atoms.count, (NSUInteger)1); + MTMathColor* colorAtom = (MTMathColor*)list.atoms[0]; + XCTAssertEqual(colorAtom.type, kMTMathAtomColor); + XCTAssertEqualObjects(colorAtom.colorString, @"#f00"); +} + +- (void)testColorboxValidHexSix +{ + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\colorbox{#00ff00}x" error:&error]; + XCTAssertNil(error, @"Unexpected error: %@", error); + XCTAssertNotNil(list); + XCTAssertEqual(list.atoms.count, (NSUInteger)1); + MTMathColorbox* colorboxAtom = (MTMathColorbox*)list.atoms[0]; + XCTAssertEqual(colorboxAtom.type, kMTMathAtomColorbox); + XCTAssertEqualObjects(colorboxAtom.colorString, @"#00ff00"); +} + +- (void)testColorInvalidNamedColorIsParseError +{ + // Named colors like "red" must be a parse error (not a silent no-op). + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{red}x" error:&error]; + XCTAssertNil(list, @"Expected nil list for invalid color"); + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTParseError); + XCTAssertEqual(error.code, MTParseErrorInvalidCommand); +} + +- (void)testColorInvalidMissingHashIsParseError +{ + // "ff0000" without leading # must be a parse error (silent failure bug). + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{ff0000}x" error:&error]; + XCTAssertNil(list, @"Expected nil list for color missing #"); + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTParseError); + XCTAssertEqual(error.code, MTParseErrorInvalidCommand); +} + +- (void)testColorInvalidNonHexDigitIsParseError +{ + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{#gg0000}x" error:&error]; + XCTAssertNil(list, @"Expected nil list for non-hex color"); + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTParseError); + XCTAssertEqual(error.code, MTParseErrorInvalidCommand); +} + +- (void)testColorInvalidWrongLengthIsParseError +{ + // 4-digit hex is neither #RGB nor #RRGGBB. + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\color{#ff00}x" error:&error]; + XCTAssertNil(list, @"Expected nil list for wrong-length color"); + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTParseError); + XCTAssertEqual(error.code, MTParseErrorInvalidCommand); +} + +- (void)testColorboxInvalidNamedColorIsParseError +{ + NSError* error = nil; + MTMathList* list = [MTMathListBuilder buildFromString:@"\\colorbox{red}x" error:&error]; + XCTAssertNil(list, @"Expected nil list for invalid colorbox color"); + XCTAssertNotNil(error); + XCTAssertEqual(error.domain, MTParseError); + XCTAssertEqual(error.code, MTParseErrorInvalidCommand); +} + @end From d3fb07e840b155230444456bb7db837e261db89f Mon Sep 17 00:00:00 2001 From: Kostub D Date: Fri, 12 Jun 2026 06:45:37 +0530 Subject: [PATCH 2/2] docs(REN-2): document REN-7 dependency for #RGB rendering in readColor Note in-code that 3-digit #RGB is parse-accepted here but renders the wrong color until the colorFromHexString: 3-digit decoder fix (REN-7) lands. Keep accepting #RGB (parse-correct / render-deferred), since the previous parser accepted and mis-rendered it identically. Co-Authored-By: Claude Opus 4.8 --- iosMath/lib/MTMathListBuilder.m | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/iosMath/lib/MTMathListBuilder.m b/iosMath/lib/MTMathListBuilder.m index ca75360..fd337d5 100644 --- a/iosMath/lib/MTMathListBuilder.m +++ b/iosMath/lib/MTMathListBuilder.m @@ -577,6 +577,15 @@ - (NSString*) readColor // Validate: color must be '#' followed by exactly 3 or 6 hex digits. // This keeps the grammar consistent with colorFromHexString: which requires // a leading '#'. Named colors and bare hex strings are not supported. + // + // NOTE: 3-digit #RGB is accepted here at parse time, but correct *rendering* + // of #RGB depends on colorFromHexString: handling the 3-digit shorthand. + // The current decoder is 6-digit-only (scanHexInt on "f00" yields 0xF00 and + // is masked as if it were #000F00), so #RGB currently renders the wrong + // color until that decoder fix (REN-7) lands. We deliberately keep + // accepting #RGB rather than rejecting it: the previous parser also + // accepted and mis-rendered #RGB identically, so this is parse-correct / + // render-deferred, not a regression. BOOL valid = NO; NSUInteger len = mutable.length; if (len == 4 || len == 7) {