Fix off by one error in GetPrefixLocation recursion (#584)

* Fix off by one error in GetPrefixLocation recursion

* Adds new quoted strings test case

Author: gfs

AppInspector.RulesEngine/TextContainer.cs

@@ -276,6 +276,14 @@ public TextContainer(string content, string language, Languages languages, ILogg
         }
     }
 
+    /// <summary>
+    /// Find the location of the provided prefix string if any in the text container between the provided start of line index and the current index
+    /// </summary>
+    /// <param name="startOfLineIndex">Minimum Character index in FullContent to locate Prefix</param>
+    /// <param name="currentIndex">Maximal Chracter index in FullContent to locate Prefix</param>
+    /// <param name="prefix">Prefix string to attempt to locate</param>
+    /// <param name="multiline">If multi-line comments should be detected</param>
+    /// <returns>The index of the specified prefix string in the FullContent if found, otherwise -1</returns>
     private int GetPrefixLocation(int startOfLineIndex, int currentIndex, string prefix, bool multiline)
     {
         // Find the first potential index of the prefix
@@ -300,7 +308,12 @@ private int GetPrefixLocation(int startOfLineIndex, int currentIndex, string pre
             // It might be like var address = "http://contoso.com";
             if (numDoubleQuotes % 2 == 1 || numSingleQuotes % 2 == 1)
             {
-                return GetPrefixLocation(startOfLineIndex, prefixLoc, prefix, multiline);
+                // The second argument is the maximal index to return since this calls LastIndexOf, subtract 1 to exclude this instance
+                if ((prefixLoc -1) >= startOfLineIndex)
+                {
+                    return GetPrefixLocation(startOfLineIndex, prefixLoc - 1, prefix, multiline);
+                }
+                return -1;
             }
         }
 

AppInspector.Tests/RuleProcessor/QuotedStringsTests.cs

@@ -10,6 +10,9 @@
 
 namespace AppInspector.Tests.RuleProcessor;
 
+/// <summary>
+/// Tests for properly detecting commented/live code status in the presence of comment markers inside of quoted strings
+/// </summary>
 [TestClass]
 public class QuotedStringsTests
 {
@@ -29,6 +32,12 @@ public class QuotedStringsTests
 /* 
 contoso.com 
 */ var url = ""https://contoso.com""";
+    private const string testRubyInterpolatedStrings = @"findMe = ""findMe""
+puts ""Hello, #{findMe}!"" # findMe
+def inspect # :nodoc:
+    ""#<#{findMe} #{findMe}>"" #findMe
+end
+"; // Should find 5 instances, excluding the two true comments
 
     private static string detectContosoRule = @"
     [
@@ -54,7 +63,32 @@ public class QuotedStringsTests
     }
 ]
 ";
-    
+
+    private static string detectFindMeRule = @"
+    [
+    {
+        ""id"": ""RE000001"",
+        ""name"": ""Testing.Rules.Quotes"",
+        ""tags"": [
+            ""Testing.Rules.Quotes""
+        ],
+        ""severity"": ""Critical"",
+        ""description"": ""Find findMe"",
+        ""patterns"": [
+            {
+                ""pattern"": ""findMe"",
+                ""type"": ""regex"",
+                ""confidence"": ""High"",
+                ""scopes"": [
+                    ""code""
+                ]
+            }
+        ],
+        ""_comment"": """"
+    }
+]
+";
+
     private readonly ILoggerFactory _loggerFactory =
         new LogOptions { ConsoleVerbosityLevel = LogEventLevel.Verbose }.GetLoggerFactory();
 
@@ -78,4 +112,24 @@ public void QuotedStrings(string content, int numIssues)
         Assert.AreEqual(numIssues,
             ruleProcessor.AnalyzeFile(content, new FileEntry("testfile.cs", new MemoryStream()), info).Count());
     }
+
+    /// <summary>
+    /// Ruby interpolated strings provide an interesting test case because they use the comment character as part of interpolation
+    ///     the comment marker is one character long, and it may often come right after the quotation mark
+    /// </summary>
+    /// <param name="content"></param>
+    /// <param name="numIssues"></param>
+    
+    [DataRow(testRubyInterpolatedStrings, 5)]
+    [DataTestMethod]
+    public void QuotedStringsRuby(string content, int numIssues)
+    {
+        RuleSet rules = new(_loggerFactory);
+        rules.AddString(detectFindMeRule, "findMeRule");
+        Microsoft.ApplicationInspector.RulesEngine.RuleProcessor ruleProcessor =
+            new Microsoft.ApplicationInspector.RulesEngine.RuleProcessor(rules, new RuleProcessorOptions());
+        _languages.FromFileNameOut("testfile.rb", out LanguageInfo info);
+        Assert.AreEqual(numIssues,
+            ruleProcessor.AnalyzeFile(content, new FileEntry("testfile.rb", new MemoryStream()), info).Count());
+    }
 }