Skip to content

SemanticString.Split(char, RemoveEmptyEntries) crashes the process with a stack overflow on a long run of separators, and ignores combined/TrimEntries options #298

Description

@matt-edmondson

What's wrong

SpanSplitEnumerator.MoveNext() (Semantics.Strings/SemanticString.cs, ~line 1020) skips empty entries by calling itself recursively:

if (_options == StringSplitOptions.RemoveEmptyEntries && Current.IsEmpty)
{
    return MoveNext(); // Recursively skip empty entries
}

Each empty entry adds a stack frame, so a long run of separators overflows the stack.

Reproduction (net10.0 console app against the built ktsu.Semantics.Strings.dll):

var s = MyString.Create(new string(',', 100_000) + "x");
foreach (var part in s.Split(',', StringSplitOptions.RemoveEmptyEntries)) { }
  • Actual: Stack overflow. The process aborts with exit code 134. A StackOverflowException can't be caught, so the caller has no way to recover.
  • Expected: one entry, "x", which is what string.Split returns.
  • 10,000 commas still works; 100,000 and 1,000,000 crash.

The same check has two more defects:

  • Combined options are ignored. The code compares with == instead of testing the flag, so RemoveEmptyEntries | TrimEntries removes nothing: "a,,b" gives [a, "", b].
  • TrimEntries is never applied. "a, ,b" with RemoveEmptyEntries | TrimEntries gives [a, " ", b], where string.Split gives [a, b].

Why it matters

This enumerator is documented as the zero-allocation replacement for Split() in performance-critical code. That is exactly where large or untrusted input, such as CSV or log lines, arrives. One malformed line with a long run of delimiters is enough to terminate the host process.

Suggested fix

Replace the recursion with a loop, and test the options as flags:

while (!_remaining.IsEmpty)
{
    // slice the next segment into Current (as today)
    if ((_options & StringSplitOptions.TrimEntries) != 0) Current = Current.Trim();
    if ((_options & StringSplitOptions.RemoveEmptyEntries) != 0 && Current.IsEmpty) continue;
    return true;
}
return false;

Acceptance criteria

  • Splitting 1,000,000 separators followed by "x" with RemoveEmptyEntries yields exactly ["x"] and does not crash.
  • RemoveEmptyEntries | TrimEntries on "a, ,b" yields ["a", "b"].
  • TrimEntries alone trims each entry.
  • Regression tests are added for all three.

Out of scope: the enumerator also drops a trailing empty entry ("a,b," gives 2 parts, where string.Split gives 3). The existing tests in SemanticStringTests.cs (~1374–1405) document related empty-entry behaviour as current, so any change there is a separate decision.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingreadyFully specified; implement as written

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions