fix(core): account for more infinite recursion possiblities

.github/workflows/release.yml

@@ -22,7 +22,7 @@ jobs:
         job:
           - { target: aarch64-apple-darwin        , os: macos-latest  }
           - { target: x86_64-apple-darwin         , os: macos-latest  }
-          - { target: x86_64-unknown-linux-gnu    , os: ubuntu-20.04  }
+          - { target: x86_64-unknown-linux-gnu    , os: ubuntu-latest  }
           - { target: x86_64-unknown-linux-musl   , os: ubuntu-latest, use-cross: true }
           - { target: arm-unknown-linux-gnueabihf , os: ubuntu-latest, use-cross: true }
           - { target: aarch64-unknown-linux-gnu   , os: ubuntu-latest, use-cross: true }

lib/lrs.ex

@@ -13,27 +13,35 @@ defmodule LRS do
     force_build: System.get_env("LRS_FORCE_BUILD") == "true"
 
   # Define the function signature that matches the Rust NIF
-  @spec longest_repeating_substring(String.t()) :: {:ok, String.t()}
+  @spec longest_repeating_substring(String.t()) :: String.t()
   def longest_repeating_substring(""), do: ""
   def longest_repeating_substring(_string), do: :erlang.nif_error(:nif_not_loaded)
 
-  def recursively_remove_repeating_substring(string, above_size \\ 30)
-  def recursively_remove_repeating_substring("", _), do: ""
+  def recursively_remove_repeating_substring(string, above_size \\ 30, max_depth \\ 100)
+  def recursively_remove_repeating_substring("", _, _), do: ""
 
-  def recursively_remove_repeating_substring(string, above_size) do
-    longest = longest_repeating_substring(string)
+  def recursively_remove_repeating_substring(string, above_size, max_depth) do
+    case longest_repeating_substring(string) do
+      "" ->
+        string
+
+      longest when is_binary(longest) ->
+        if String.length(longest) > above_size do
+          new_string = String.replace(string, longest, " ", global: false)
 
-    if String.length(longest) > above_size do
-      new_string = String.replace(string, longest, " ", global: false)
+          # Only recurse if the string actually changed, we haven't hit a base case,
+          # and we haven't exceeded max depth
+          if new_string != string and new_string != "" and max_depth > 0 do
+            recursively_remove_repeating_substring(new_string, above_size, max_depth - 1)
+          else
+            string
+          end
+        else
+          string
+        end
 
-      # Check if the string actually changed after replacement
-      if new_string != string do
-        recursively_remove_repeating_substring(new_string, above_size)
-      else
+      _ ->
         string
-      end
-    else
-      string
     end
   end
 end

test/lrs_test.exs

@@ -53,13 +53,20 @@ defmodule LRSTest do
     end
 
     test "handles case where replacement doesn't change string" do
-      # Mock the longest_repeating_substring function to return a value that won't be found
-      # This test assumes the implementation will handle this case correctly
-      string = "This is a test string"
+      # Create a string with a repeating pattern that will be found
+      string = "abcabcabcabc"
 
-      # This is a simple test to ensure no infinite recursion
-      # The actual behavior depends on the implementation of longest_repeating_substring
-      assert is_binary(LRS.recursively_remove_repeating_substring(string, 5))
+      # The function should complete without infinite recursion
+      result = LRS.recursively_remove_repeating_substring(string, 3)
+
+      # The result should be different from the input (since we replaced something)
+      refute result == string
+
+      # The result should be shorter than the input (since we replaced multiple chars with a space)
+      assert String.length(result) < String.length(string)
+
+      # Verify it completes in a reasonable time (no infinite loop)
+      assert Process.alive?(self())
     end
 
     test "handles multiple nested repeating substrings" do
@@ -101,5 +108,22 @@ defmodule LRSTest do
       refute String.contains?(result_20, repeat_35)
       refute String.contains?(result_20, repeat_25)
     end
+
+    test "respects max recursion depth" do
+      # Create a string that would cause deep recursion
+      string = String.duplicate("abc", 100)
+
+      # With a very low max depth, it should stop early
+      result = LRS.recursively_remove_repeating_substring(string, 3, 5)
+
+      # The result should be different from the input (some replacements happened)
+      refute result == string
+
+      # But not all replacements should have happened due to depth limit
+      assert String.contains?(result, "abc")
+
+      # Verify it completes in a reasonable time (no infinite loop)
+      assert Process.alive?(self())
+    end
   end
 end