C# IsSubsequenceOf throws IndexOutOfRangeException

[raulsntos]

Godot version:
Godot_v3.2-rc3_mono_x11_64

OS/device including version:
Linux (Pop OS 19.10) but should affect every platform

Issue description:

E 0:00:00.507   Boolean Godot.StringExtensions.IsSubsequenceOf(System.String , System.String , Boolean ): System.IndexOutOfRangeException: Index was outside the bounds of the array.
  <C++ Error>   Unhandled exception
  <C++ Source>  /root/godot/modules/mono/glue/GodotSharp/GodotSharp/Core/StringExtensions.cs:494 @ Boolean Godot.StringExtensions.IsSubsequenceOf(System.String , System.String , Boolean )()
  <Stack Trace> StringExtensions.cs:494 @ Boolean Godot.StringExtensions.IsSubsequenceOf(System.String , System.String , Boolean )()
                TestSubsequence.cs:10 @ void TestSubsequence._Ready()()

Steps to reproduce:
Call method IsSubsequenceOf or IsSubsequenceOfI

Minimal reproduction project:
SubsequenceBug.zip
This project includes SceneBUG where the code is using the engine methods and fails and SceneFIX where I implemented the possible solution described below.

In StringExtensions.cs lines 477 and 494 it tries to access a character in the strings until finding 0 which never happens, instead eventually source or target will be bigger than the length of the strings which results in IndexOutOfRangeException:

godot/modules/mono/glue/GodotSharp/GodotSharp/Core/StringExtensions.cs

Lines 464 to 502 in 12482bf

	public static bool IsSubsequenceOf(this string instance, string text, bool caseSensitive = true)
	{
	int len = instance.Length;
	if (len == 0)
	return true; // Technically an empty string is subsequence of any string
	if (len > text.Length)
	return false;
	int source = 0;
	int target = 0;
	while (instance[source] != 0 && text[target] != 0)
	{
	bool match;
	if (!caseSensitive)
	{
	char sourcec = char.ToLower(instance[source]);
	char targetc = char.ToLower(text[target]);
	match = sourcec == targetc;
	}
	else
	{
	match = instance[source] == text[target];
	}
	if (match)
	{
	source++;
	if (instance[source] == 0)
	return true;
	}
	target++;
	}
	return false;
	}

I could create a PR fixing this but according to CONTRIBUTING.md I'm supposed to open an issue first to discuss the best fix for this bug, this is my first time contributing to Godot.

About fixing this, I thought that we could check the length of the string instead like so:

while (source < len && target < text.Length)

if (match)
{
	source++;
	if (source >= len)
		return true;
}

[bruce965]

EDIT: nevermind, I missed the actual purpose of this code. It would not be possible to use the built-in methods without reading text twice.

---

Would the following code solve the problem?

public static bool IsSubsequenceOf(this string instance, string text, bool caseSensitive = true)
{
    var compareAs = caseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase;
    return text.IndexOf(instance, compareAs) >= 0;
}

(adapted from Remarks at https://docs.microsoft.com/en-us/dotnet/api/system.string.contains)

[madmiraal]

@raulsntos I can confirm that your proposed solution resolves the issue.

I could create a PR fixing this but according to CONTRIBUTING.md I'm supposed to open an issue first to discuss the best fix for this bug, this is my first time contributing to Godot.

The need to discuss the best fix first is for adding new engine features, which are now done separately in the Godot Proposals repository. If you already have a fix for a bug, as you have here, feel free to submit a PR. It will be discussed in the PR if necessary. See the PR workflow documentation for how to submit a PR if you're unsure.