From 4c287f20b3a946e63db30857686386586e998d7c Mon Sep 17 00:00:00 2001 From: rfm Date: Thu, 21 Nov 2024 21:05:34 +0000 Subject: [PATCH] More regular expression leak fix changes ... use older code (plus fixes). --- ChangeLog | 4 +- Source/NSRegularExpression.m | 281 ++++++++++++++++++++++++----------- 2 files changed, 199 insertions(+), 86 deletions(-) diff --git a/ChangeLog b/ChangeLog index 678691bfa..0bac5c189 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,7 @@ 2024-11-21 Richard Frith-Macdonald - * NSRegularExpression: avoid leaks in ICU code by calling replacement - methods with a non-NULL destination. + * NSRegularExpression: avoid leaks in ICU code by calling more + primitive methods. * NSAutoreleasePool: fix leaks with libobjc2 ARC methods when a pool is in a context that we leave due to an exception. * NSObject: add -trackOwnership to log the retain/release/dealloc of diff --git a/Source/NSRegularExpression.m b/Source/NSRegularExpression.m index 80c26c37f..1482fb34a 100644 --- a/Source/NSRegularExpression.m +++ b/Source/NSRegularExpression.m @@ -35,9 +35,19 @@ * won't work because libicu internally renames all entry points with some cpp * magic. */ +#if !defined(HAVE_UREGEX_OPENUTEXT) #if U_ICU_VERSION_MAJOR_NUM > 4 || (U_ICU_VERSION_MAJOR_NUM == 4 && U_ICU_VERSION_MINOR_NUM >= 4) || defined(HAVE_ICU_H) #define HAVE_UREGEX_OPENUTEXT 1 #endif +#endif + +/* Until the uregex_replaceAllUText() and uregex_replaceFirstUText() work + * without leaking memory, we can't use them :-( + * Preoblem exists on Ubuntu in 2024 with icu-74.2 + */ +#if defined(HAVE_UREGEX_OPENUTEXT) +#undef HAVE_UREGEX_OPENUTEXT +#endif #define NSRegularExpressionWorks @@ -158,7 +168,8 @@ - (id) initWithPattern: (NSString*)aPattern NSDictionary *userInfo; NSString *description; - description = [NSString stringWithFormat: @"The value “%@” is invalid.", aPattern]; + description = [NSString + stringWithFormat: @"The value “%@” is invalid.", aPattern]; userInfo = [NSDictionary dictionaryWithObjectsAndKeys: aPattern, @"NSInvalidValue", @@ -245,11 +256,46 @@ - (id) initWithPattern: (NSString*)aPattern } #endif + // Raise an NSInvalidArgumentException to match macOS behaviour. + if (!aPattern) + { + NSException *exp; + + exp = [NSException exceptionWithName: NSInvalidArgumentException + reason: @"nil argument" + userInfo: nil]; + RELEASE(self); + [exp raise]; + } + [aPattern getCharacters: buffer range: NSMakeRange(0, length)]; regex = uregex_open(buffer, length, flags, &pe, &s); if (U_FAILURE(s)) { - // FIXME: Do something sensible with the error parameter. + /* Match macOS behaviour if the pattern is invalid. + * Example: + * Domain=NSCocoaErrorDomain + * Code=2048 "The value “” is invalid." + * UserInfo={NSInvalidValue=} + */ + if (e) + { + NSDictionary *userInfo; + NSString *description; + + description = [NSString + stringWithFormat: @"The value “%@” is invalid.", aPattern]; + + userInfo = [NSDictionary dictionaryWithObjectsAndKeys: + aPattern, @"NSInvalidValue", + description, NSLocalizedDescriptionKey, + nil]; + + *e = [NSError errorWithDomain: NSCocoaErrorDomain + code: NSFormattingError + userInfo: userInfo]; + } + DESTROY(self); return self; } @@ -868,29 +914,35 @@ - (NSUInteger) replaceMatchesInString: (NSMutableString*)string { // FIXME: We're computing a value that is most likely ignored in an // expensive way. - NSInteger results = [self numberOfMatchesInString: string - options: opts - range: range]; - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + NSInteger results = [self numberOfMatchesInString: string + options: opts + range: range]; + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + GSUTextString *ret = [GSUTextString new]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + UText *output = NULL; UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceAllUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - utext_close(&dst); + output = uregex_replaceAllUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return 0; } - [string setString: ms]; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + [string setString: ret]; + RELEASE(ret); + uregex_close(r); + + utext_close(&txt); + utext_close(output); + utext_close(&replacement); return results; } @@ -899,26 +951,31 @@ - (NSString*) stringByReplacingMatchesInString: (NSString*)string range: (NSRange)range withTemplate: (NSString*)template { - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + UText *output = NULL; + GSUTextString *ret = [GSUTextString new]; + URegularExpression *r = setupRegex(regex, string, &txt, opts, range, 0); UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceAllUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&replacement); - utext_close(&txt); - utext_close(&dst); + output = uregex_replaceAllUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return nil; } - return ms; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + uregex_close(r); + + utext_close(&txt); + utext_close(output); + utext_close(&replacement); + return AUTORELEASE(ret); } - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result @@ -926,13 +983,13 @@ - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result offset: (NSInteger)offset template: (NSString*)template { - UErrorCode s = 0; - UText dst = UTEXT_INITIALIZER; - UText txt = UTEXT_INITIALIZER; - UText replacement = UTEXT_INITIALIZER; - NSMutableString *ms = [NSMutableString string]; - NSRange range = [result range]; - URegularExpression *r = setupRegex(regex, + UErrorCode s = 0; + UText txt = UTEXT_INITIALIZER; + UText replacement = UTEXT_INITIALIZER; + UText *output = NULL; + GSUTextString *ret = [GSUTextString new]; + NSRange range = [result range]; + URegularExpression *r = setupRegex(regex, [string substringWithRange: range], &txt, 0, @@ -940,18 +997,22 @@ - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result 0); UTextInitWithNSString(&replacement, template); - UTextInitWithNSMutableString(&dst, ms); - uregex_replaceFirstUText(r, &replacement, &dst, &s); - uregex_close(r); - utext_close(&dst); - utext_close(&txt); - utext_close(&replacement); + output = uregex_replaceFirstUText(r, &replacement, NULL, &s); if (0 != s) { + uregex_close(r); + utext_close(&replacement); + utext_close(&txt); + DESTROY(ret); return nil; } - return ms; + utext_clone(&ret->txt, output, TRUE, TRUE, &s); + utext_close(output); + uregex_close(r); + utext_close(&txt); + utext_close(&replacement); + return AUTORELEASE(ret); } #else - (NSUInteger) replaceMatchesInString: (NSMutableString*)string @@ -964,31 +1025,51 @@ - (NSUInteger) replaceMatchesInString: (NSMutableString*)string NSInteger results = [self numberOfMatchesInString: string options: opts range: range]; - UErrorCode s = 0; - uint32_t length = [string length]; - uint32_t replLength = [template length]; - unichar replacement[replLength]; - int32_t outLength; - unichar *output; - NSString *out; - URegularExpression *r; - TEMP_BUFFER(buffer, length); - - r = setupRegex(regex, string, buffer, length, opts, range, 0); - [template getCharacters: replacement range: NSMakeRange(0, replLength)]; - - outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); - - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceAll(r, replacement, replLength, output, outLength, &s); - out = - [[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]; - [string setString: out]; - RELEASE(out); - + if (results > 0) + { + UErrorCode s = 0; + uint32_t length = [string length]; + uint32_t replLength = [template length]; + unichar replacement[replLength]; + int32_t outLength; + URegularExpression *r; + TEMP_BUFFER(buffer, length); + + r = setupRegex(regex, string, buffer, length, opts, range, 0); + [template getCharacters: replacement range: NSMakeRange(0, replLength)]; + + outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; + + s = 0; // May have been set to a buffer overflow error + + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceAll(r, replacement, replLength, + output, outLength + 1, &s); + if (0 == s) + { + NSString *out; + + out = [[NSString alloc] initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]; + [string setString: out]; + RELEASE(out); + } + else + { + NSZoneFree(0, output); + results = 0; + } + } + else + { + results = 0; + } + uregex_close(r); + } return results; } @@ -1003,20 +1084,36 @@ - (NSString*) stringByReplacingMatchesInString: (NSString*)string uint32_t replLength = [template length]; unichar replacement[replLength]; int32_t outLength; - unichar *output; + NSString *result = nil; TEMP_BUFFER(buffer, length); r = setupRegex(regex, string, buffer, length, opts, range, 0); [template getCharacters: replacement range: NSMakeRange(0, replLength)]; outLength = uregex_replaceAll(r, replacement, replLength, NULL, 0, &s); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; + + s = 0; // may have been set to a buffer overflow error + + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceAll(r, replacement, replLength, output, outLength + 1, &s); + if (0 == s) + { + result = AUTORELEASE([[NSString alloc] + initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]); + } + else + { + NSZoneFree(0, output); + } + } - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceAll(r, replacement, replLength, output, outLength, &s); - return AUTORELEASE([[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]); + uregex_close(r); + return result; } - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result @@ -1030,7 +1127,7 @@ - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result uint32_t replLength = [template length]; unichar replacement[replLength]; int32_t outLength; - unichar *output; + NSString *str = nil; TEMP_BUFFER(buffer, range.length); r = setupRegex(regex, @@ -1043,12 +1140,28 @@ - (NSString*) replacementStringForResult: (NSTextCheckingResult*)result [template getCharacters: replacement range: NSMakeRange(0, replLength)]; outLength = uregex_replaceFirst(r, replacement, replLength, NULL, 0, &s); - s = 0; - output = NSZoneMalloc(0, outLength * sizeof(unichar)); - uregex_replaceFirst(r, replacement, replLength, output, outLength, &s); - return AUTORELEASE([[NSString alloc] initWithCharactersNoCopy: output - length: outLength - freeWhenDone: YES]); + if (0 == s || U_BUFFER_OVERFLOW_ERROR == s) + { + unichar *output; + + s = 0; + output = NSZoneMalloc(0, (outLength + 1) * sizeof(unichar)); + uregex_replaceFirst(r, replacement, replLength, + output, outLength + 1, &s); + if (0 == s) + { + str = AUTORELEASE([[NSString alloc] + initWithCharactersNoCopy: output + length: outLength + freeWhenDone: YES]); + } + else + { + NSZoneFree(0, output); + } + } + uregex_close(r); + return str; } #endif