Fix String::trim on all-separator input. Fix RegEx freeing Oniguruma patterns.

String::trim: when the input consisted only of separators, both find_first_not_of
and find_last_not_of returned npos and the pos2 fallback computed length() - 1, so
trim("  ") returned " " and every all-separator value came back one character
shorter instead of empty, making an all-whitespace value look non-empty to callers.
All eight overloads were affected: std::string, std::string_view, String and
String::View, in their char and separator-set forms. Return an empty result when
there is no non-separator, and compute the trimmed range as
substr(pos1, pos2 - pos1 + 1).

RegEx: mCompiledPattern stores whichever engine compiled the pattern, a pcre2_code*
or an OnigRegex, but the destructor released it with pcre2_code_free() whenever
RegExCache did not own it. With useCache = false that freed an Oniguruma pattern as
a PCRE2 block, which corrupts the heap: constructing and destroying such a RegEx
crashes with SIGSEGV, a path snippetparser.cpp reaches by passing Utf |
AllowFallback with useCache = false. Release the pattern with the deallocator of the
engine recorded in Options::UseOniguruma, the predicate matches(), getCaptureCount()
and RegExCache::clear() already use, and remove the mOnigEngine flag, which was
never set or read anywhere.

Both fixes come with regression tests. String.trim covers the all-separator boundary
and the trimmed range of every overload. RegExEngines.uncachedPatternIsFreedByItsOwnEngine
destroys uncached patterns from both engines; it crashed the test binary before the fix.

include/eepp/system/regex.hpp

@@ -92,10 +92,11 @@ class EE_API RegEx : public PatternMatcher {
   protected:
 	std::string_view mPattern;
 	mutable size_t mMatchNum;
+	/** Compiled pattern, owned by the engine named in the Options::UseOniguruma bit of mOptions:
+	 *  pcre2_code* or OnigRegex respectively. The cache owns it when mCached is set. */
 	void* mCompiledPattern;
 	int mCaptureCount{ 0 };
 	Uint32 mOptions{ Options::Utf | Options::AllowFallback };
-	bool mOnigEngine : 1 { false };
 	bool mValid : 1 { false };
 	bool mCached : 1 { false };
 	bool mFilterOutCaptures : 1 { false };

src/eepp/core/string.cpp

@@ -1215,9 +1215,11 @@ std::string String::rTrim( const std::string& str, char character ) {
 
 std::string String::trim( const std::string& str, char character ) {
 	std::string::size_type pos1 = str.find_first_not_of( character );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == std::string::npos )
+		return {};
 	std::string::size_type pos2 = str.find_last_not_of( character );
-	return str.substr( pos1 == std::string::npos ? 0 : pos1,
-					   pos2 == std::string::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 std::string_view String::lTrim( const std::string_view& str, char character ) {
@@ -1232,9 +1234,11 @@ std::string_view String::rTrim( const std::string_view& str, char character ) {
 
 std::string_view String::trim( const std::string_view& str, char character ) {
 	std::string::size_type pos1 = str.find_first_not_of( character );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == std::string::npos )
+		return {};
 	std::string::size_type pos2 = str.find_last_not_of( character );
-	return str.substr( pos1 == std::string::npos ? 0 : pos1,
-					   pos2 == std::string::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 String::View String::lTrim( const String::View& str, char character ) {
@@ -1249,9 +1253,11 @@ String::View String::rTrim( const String::View& str, char character ) {
 
 String::View String::trim( const String::View& str, char character ) {
 	String::View::size_type pos1 = str.find_first_not_of( character );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == String::View::npos )
+		return {};
 	String::View::size_type pos2 = str.find_last_not_of( character );
-	return str.substr( pos1 == String::View::npos ? 0 : pos1,
-					   pos2 == String::View::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 void String::trimInPlace( std::string& str, char character ) {
@@ -1272,9 +1278,11 @@ String String::rTrim( const String& str, char character ) {
 
 String String::trim( const String& str, char character ) {
 	StringType::size_type pos1 = str.find_first_not_of( character );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == String::InvalidPos )
+		return {};
 	StringType::size_type pos2 = str.find_last_not_of( character );
-	return str.substr( pos1 == String::InvalidPos ? 0 : pos1,
-					   pos2 == String::InvalidPos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 void String::trimInPlace( String& str, char character ) {
@@ -1293,9 +1301,11 @@ std::string String::rTrim( const std::string& str, std::string_view characters )
 
 std::string String::trim( const std::string& str, std::string_view characters ) {
 	std::string::size_type pos1 = str.find_first_not_of( characters );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == std::string::npos )
+		return {};
 	std::string::size_type pos2 = str.find_last_not_of( characters );
-	return str.substr( pos1 == std::string::npos ? 0 : pos1,
-					   pos2 == std::string::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 std::string_view String::lTrim( const std::string_view& str, std::string_view characters ) {
@@ -1310,9 +1320,11 @@ std::string_view String::rTrim( const std::string_view& str, std::string_view ch
 
 std::string_view String::trim( const std::string_view& str, std::string_view characters ) {
 	std::string::size_type pos1 = str.find_first_not_of( characters );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == std::string::npos )
+		return {};
 	std::string::size_type pos2 = str.find_last_not_of( characters );
-	return str.substr( pos1 == std::string::npos ? 0 : pos1,
-					   pos2 == std::string::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 String::View String::lTrim( const String::View& str, String::View characters ) {
@@ -1327,9 +1339,11 @@ String::View String::rTrim( const String::View& str, String::View characters ) {
 
 String::View String::trim( const String::View& str, String::View characters ) {
 	String::View::size_type pos1 = str.find_first_not_of( characters );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == String::View::npos )
+		return {};
 	String::View::size_type pos2 = str.find_last_not_of( characters );
-	return str.substr( pos1 == String::View::npos ? 0 : pos1,
-					   pos2 == String::View::npos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 void String::trimInPlace( std::string& str, std::string_view characters ) {
@@ -1348,9 +1362,11 @@ String String::rTrim( const String& str, std::string_view characters ) {
 
 String String::trim( const String& str, std::string_view characters ) {
 	StringType::size_type pos1 = str.find_first_not_of( characters );
+	// A string made only of separators has nothing left once trimmed.
+	if ( pos1 == String::InvalidPos )
+		return {};
 	StringType::size_type pos2 = str.find_last_not_of( characters );
-	return str.substr( pos1 == String::InvalidPos ? 0 : pos1,
-					   pos2 == String::InvalidPos ? str.length() - 1 : pos2 - pos1 + 1 );
+	return str.substr( pos1, pos2 - pos1 + 1 );
 }
 
 void String::trimInPlace( String& str, std::string_view characters ) {

src/eepp/system/regex.cpp

@@ -135,9 +135,16 @@ RegEx::RegEx( std::string_view pattern, Uint32 options, bool useCache ) :
 }
 
 RegEx::~RegEx() {
-	if ( !mCached && mCompiledPattern != nullptr ) {
+	if ( mCached || mCompiledPattern == nullptr )
+		return;
+
+	// The pattern is owned by whichever engine compiled it, so it must be released with that
+	// engine's deallocator: freeing an Oniguruma pattern with pcre2_code_free() (or the reverse)
+	// corrupts the heap. The cache, which owns the patterns it hands out, does the same split.
+	if ( mOptions & Options::UseOniguruma )
+		onig_free( static_cast<OnigRegex>( mCompiledPattern ) );
+	else
 		pcre2_code_free( reinterpret_cast<pcre2_code*>( mCompiledPattern ) );
-	}
 }
 
 bool RegEx::matches( const char* stringSearch, int stringStartOffset,

src/tests/unit_tests/regex_tests.cpp

@@ -147,3 +147,23 @@ UTEST( RegExEngines, basicTest ) {
 	EXPECT_EQ( 38, matchesOniguruma[0].end );
 	RegExCache::destroySingleton();
 }
+
+UTEST( RegExEngines, uncachedPatternIsFreedByItsOwnEngine ) {
+	// A pattern compiled by Oniguruma but not owned by the cache has to be released with onig_free().
+	// Releasing it with pcre2_code_free() corrupted the heap and crashed this test binary, so the
+	// engine that compiled a pattern decides its deallocator (the same split RegExCache::clear()
+	// makes for the patterns it owns).
+	{
+		RegEx oniguruma( "a+", RegEx::Options::Utf | RegEx::Options::UseOniguruma, false );
+		EXPECT_EQ( oniguruma.isValid(), true );
+		EXPECT_EQ( oniguruma.matches( std::string( "aaa" ) ), true );
+	}
+
+	{
+		RegEx pcre2( "a+", RegEx::Options::Utf | RegEx::Options::AllowFallback, false );
+		EXPECT_EQ( pcre2.isValid(), true );
+		EXPECT_EQ( pcre2.matches( std::string( "aaa" ) ), true );
+	}
+
+	RegExCache::destroySingleton();
+}

src/tests/unit_tests/stringsoperations_tests.cpp

@@ -85,6 +85,55 @@ UTEST( String, fromStringView ) {
 	EXPECT_EQ( 1234, intValue );
 }
 
+UTEST( String, trim ) {
+	// Separators are removed from both ends and interior ones are kept.
+	EXPECT_TRUE( String::trim( std::string( "abc" ) ) == std::string( "abc" ) );
+	EXPECT_TRUE( String::trim( std::string( "  a  " ) ) == std::string( "a" ) );
+	// The char overload trims only that character, so tab and newline survive it; the
+	// string_view overload takes a whole separator set.
+	EXPECT_TRUE( String::trim( std::string( "\t a b \n" ) ) == std::string( "\t a b \n" ) );
+	EXPECT_TRUE( String::trim( std::string( "  a  " ), ' ' ) == std::string( "a" ) );
+	EXPECT_TRUE( String::trim( std::string( "xxbxx" ), 'x' ) == std::string( "b" ) );
+	EXPECT_TRUE( String::trim( std::string( "\t\r\n a \t\r\n" ), std::string_view( " \t\r\n" ) ) ==
+				 std::string( "a" ) );
+
+	// A string made only of separators has nothing left once trimmed. It used to come back as a
+	// shorter string of separators instead of as an empty one.
+	EXPECT_TRUE( String::trim( std::string() ).empty() );
+	EXPECT_TRUE( String::trim( std::string( " " ) ).empty() );
+	EXPECT_TRUE( String::trim( std::string( "  " ) ).empty() );
+	EXPECT_TRUE( String::trim( std::string( "        " ) ).empty() );
+	EXPECT_TRUE( String::trim( std::string( "xxxx" ), 'x' ).empty() );
+	EXPECT_TRUE( String::trim( std::string( "\t\r\n " ), std::string_view( " \t\r\n" ) ).empty() );
+
+	std::string inPlace = "   ";
+	String::trimInPlace( inPlace, ' ' );
+	EXPECT_TRUE( inPlace.empty() );
+	inPlace = "  a  ";
+	String::trimInPlace( inPlace, ' ' );
+	EXPECT_TRUE( inPlace == std::string( "a" ) );
+
+	// The view overloads must report the trimmed range, not a truncated one.
+	EXPECT_TRUE( String::trim( std::string_view() ).empty() );
+	EXPECT_TRUE( String::trim( std::string_view( "     " ) ).empty() );
+	EXPECT_TRUE( String::trim( std::string_view( "  a  " ) ) == std::string_view( "a" ) );
+	EXPECT_TRUE( String::trim( std::string_view( "xx" ), std::string_view( "x" ) ).empty() );
+	EXPECT_TRUE( String::trim( std::string_view( "xxa xx" ), std::string_view( "x " ) ) ==
+				 std::string_view( "a" ) );
+
+	// The UTF-32 overloads are separate implementations and had the same defect.
+	EXPECT_TRUE( String::trim( String() ).empty() );
+	EXPECT_TRUE( String::trim( String( "   " ) ).empty() );
+	EXPECT_TRUE( String::trim( String( "  a  " ) ) == String( "a" ) );
+	EXPECT_TRUE( String::trim( String( "xxx" ), std::string_view( "x" ) ).empty() );
+	EXPECT_TRUE( String::trim( String( "  a  " ), std::string_view( " " ) ) == String( "a" ) );
+	EXPECT_TRUE( String::trim( String::View( U"   " ) ).empty() );
+	EXPECT_TRUE( String::trim( String::View( U"  a  " ) ) == String::View( U"a" ) );
+	EXPECT_TRUE( String::trim( String::View( U"  " ), String::View( U" " ) ).empty() );
+	EXPECT_TRUE( String::trim( String::View( U"  a  " ), String::View( U" " ) ) ==
+				 String::View( U"a" ) );
+}
+
 UTEST( String, reusableFormattingAndUtf8Assignment ) {
 	std::string formatted;
 	formatted.reserve( 128 );