Merge pull request #1515 from qdraw/feature/202403_GetFileNameRegex_l…

…onger GetFileName refactor
qdraw · Apr 8, 2024 · 3c52ada · 3c52ada
2 parents 77fedf8 + 4175f12
commit 3c52ada
Show file tree

Hide file tree

Showing 3 changed files with 134 additions and 61 deletions.
diff --git a/history.md b/history.md
@@ -45,6 +45,7 @@ Semantic Versioning 2.0.0 is from version 0.1.6+
 
 - [x] (Changed) _Front-end_  Make prev / next more contrast (PR #1511)
 - [x] (Fixed) _Docs_ Demo site is not working (PR #1486)
+- [x] (Fixed) _Back-end_  GetFileNameRegex refactor to avoid timeouts (PR #1515)
 
 ## version 0.6.0 - 2024-03-15 {#v0.6.0}
 

diff --git a/starsky/starsky.foundation.platform/Helpers/PathHelper.cs b/starsky/starsky.foundation.platform/Helpers/PathHelper.cs
@@ -1,25 +1,13 @@
+using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
-using System.Text.RegularExpressions;
 
 namespace starsky.foundation.platform.Helpers
 {
-	public static partial class PathHelper
+	public static class PathHelper
 	{
-		/// <summary>
-		/// Regex to match a filename in a path
-		/// unescaped:
-		/// [^/]+(?=(?:\.[^.]+)?$)
-		/// pre compiled regex Regex.Match
-		/// </summary>
-		/// <returns>Regex object</returns>
-		[GeneratedRegex(
-			"[^/]+(?=(?:\\.[^.]+)?$)",
-			RegexOptions.CultureInvariant,
-			matchTimeoutMilliseconds: 300)]
-		private static partial Regex GetFileNameRegex();
-
 		/// <summary>
 		/// Return value (works for POSIX/Windows paths)
 		/// </summary>
@@ -32,9 +20,33 @@ public static string GetFileName(string? filePath)
 				return string.Empty;
 			}
 
-			return GetFileNameRegex().Match(filePath).Value;
+			if ( filePath.Length >= 4095 )
+			{
+				// why? https://serverfault.com/questions/9546/filename-length-limits-on-linux
+				throw new ArgumentException("[PathHelper] FilePath over Unix limits", nameof(filePath));
+			}
+
+			var fileName = GetFileNameUnix(filePath.AsSpan());
+			return fileName.ToString();
 		}
 
+		[SuppressMessage("Style", "IDE0057:Use range operator")]
+		[SuppressMessage("ReSharper", "ReplaceSliceWithRangeIndexer")]
+		internal static ReadOnlySpan<char> GetFileNameUnix(ReadOnlySpan<char> path)
+		{
+			var length = GetPathRootUnix(path).Length;
+			var num = path.LastIndexOf('/');
+			return path.Slice(num < length ? length : num + 1);
+		}
+
+		private static ReadOnlySpan<char> GetPathRootUnix(ReadOnlySpan<char> path)
+		{
+			return !IsPathRootedUnix(path) ? [] : "/".AsSpan();
+		}
+
+		private static bool IsPathRootedUnix(ReadOnlySpan<char> path) =>
+			path.Length > 0 && path[0] == '/';
+
 		/// <summary>
 		/// Removes the latest backslash. Path.DirectorySeparatorChar
 		/// </summary>
@@ -52,7 +64,7 @@ public static string GetFileName(string? filePath)
 
 			// remove latest backslash
 			if ( basePath.Substring(basePath.Length - 1, 1) ==
-				 Path.DirectorySeparatorChar.ToString() )
+			     Path.DirectorySeparatorChar.ToString() )
 			{
 				basePath = basePath.Substring(0, basePath.Length - 1);
 			}
@@ -94,7 +106,7 @@ public static string AddBackslash(string thumbnailTempFolder)
 			if ( string.IsNullOrWhiteSpace(thumbnailTempFolder) ) return thumbnailTempFolder;
 
 			if ( thumbnailTempFolder.Substring(thumbnailTempFolder.Length - 1,
-					1) != Path.DirectorySeparatorChar.ToString() )
+				    1) != Path.DirectorySeparatorChar.ToString() )
 			{
 				thumbnailTempFolder += Path.DirectorySeparatorChar.ToString();
 			}

diff --git a/starsky/starskytest/starsky.foundation.platform/Helpers/PathHelperTest.cs b/starsky/starskytest/starsky.foundation.platform/Helpers/PathHelperTest.cs
@@ -1,50 +1,49 @@
+using System;
 using System.IO;
-using System.Linq;
-using System.Text.RegularExpressions;
-using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using starsky.foundation.platform.Helpers;
-using starsky.foundation.storage.Helpers;
-using starskytest.FakeCreateAn;
 
 namespace starskytest.starsky.foundation.platform.Helpers;
 
 [TestClass]
 public class PathHelperTests
 {
-	[TestMethod]
-	public void GetFileName_ReturnsValidFileName()
+	[DataTestMethod] // [Theory]
+	[DataRow("path/to/file.txt", "file.txt")]
+	[DataRow("file.txt", "file.txt")]
+	[DataRow("/file.txt", "file.txt")]
+	[DataRow("/test/file.txt", "file.txt")]
+	[DataRow("/test/file/", "")]
+	[DataRow("/test/file", "file")] // no backslash
+	public void GetFileName_ReturnsValidFileName(string input, string expectedFileName)
 	{
-		// Arrange
-		const string filePath = "path/to/file.txt";
-		const string expectedFileName = "file.txt";
+		// Act
+		var actualFileName = PathHelper.GetFileName(input);
 
+		// Assert
+		Assert.AreEqual(expectedFileName, actualFileName);
+	}
+
+	[DataTestMethod] // [Theory]
+	[DataRow("path/to/file.txt", "file.txt")]
+	[DataRow("/test/file/", "")]
+	[DataRow("/test/file", "file")] // no backslash
+	[DataRow("", "")]
+	public void GetFileNameUnix_ReturnsValidFileName(string input, string expectedFileName)
+	{
 		// Act
-		var actualFileName = PathHelper.GetFileName(filePath);
+		var actualFileName = PathHelper.GetFileNameUnix(input).ToString();
 
 		// Assert
 		Assert.AreEqual(expectedFileName, actualFileName);
 	}
 
 	[TestMethod]
-	[ExpectedException(typeof(RegexMatchTimeoutException))]
-	public async Task GetFileName_ReturnsFileName_WithMaliciousInput_UnixOnly()
+	[ExpectedException(typeof(ArgumentException))]
+	public void GetFileName_ReturnsFileName_WithMaliciousInput()
 	{
 		// Act and Assert
-		var test = await
-			StreamToStringHelper.StreamToStringAsync(
-				new MemoryStream(CreateAnImage.Bytes.ToArray()));
-		var test2 = await
-			StreamToStringHelper.StreamToStringAsync(
-				new MemoryStream(CreateAnImageA6600.Bytes.ToArray()));
-
-		var result = string.Empty;
-		for ( var i = 0; i < 200; i++ )
-		{
-			result += test + test2 + test + test;
-		}
-
-		PathHelper.GetFileName(result);
+		PathHelper.GetFileName(TestContentVeryLongString);
 	}
 
 	[TestMethod]
@@ -104,11 +103,11 @@ public void RemoveLatestBackslash_ReturnsBasePath_WhenBasePathIsRoot()
 	public void RemoveLatestSlash_RemovesLatestSlash_WhenSlashExists()
 	{
 		// Arrange
-		string basePath = "/path/to/directory/";
-		string expectedPath = "/path/to/directory";
+		const string basePath = "/path/to/directory/";
+		const string expectedPath = "/path/to/directory";
 
 		// Act
-		string actualPath = PathHelper.RemoveLatestSlash(basePath);
+		var actualPath = PathHelper.RemoveLatestSlash(basePath);
 
 		// Assert
 		Assert.AreEqual(expectedPath, actualPath);
@@ -118,10 +117,10 @@ public void RemoveLatestSlash_RemovesLatestSlash_WhenSlashExists()
 	public void RemoveLatestSlash_DoesNotRemoveSlash_WhenSlashDoesNotExist()
 	{
 		// Arrange
-		string basePath = "/path/to/directory";
+		const string basePath = "/path/to/directory";
 
 		// Act
-		string actualPath = PathHelper.RemoveLatestSlash(basePath);
+		var actualPath = PathHelper.RemoveLatestSlash(basePath);
 
 		// Assert
 		Assert.AreEqual(basePath, actualPath);
@@ -131,7 +130,7 @@ public void RemoveLatestSlash_DoesNotRemoveSlash_WhenSlashDoesNotExist()
 	public void RemoveLatestSlash_ReturnsEmptyString_WhenBasePathIsNull()
 	{
 		// Act
-		string actualPath = PathHelper.RemoveLatestSlash(null!);
+		var actualPath = PathHelper.RemoveLatestSlash(null!);
 
 		// Assert
 		Assert.AreEqual(string.Empty, actualPath);
@@ -141,10 +140,10 @@ public void RemoveLatestSlash_ReturnsEmptyString_WhenBasePathIsNull()
 	public void RemoveLatestSlash_ReturnsEmptyString_WhenBasePathIsEmpty()
 	{
 		// Arrange
-		string basePath = string.Empty;
+		var basePath = string.Empty;
 
 		// Act
-		string actualPath = PathHelper.RemoveLatestSlash(basePath);
+		var actualPath = PathHelper.RemoveLatestSlash(basePath);
 
 		// Assert
 		Assert.AreEqual(string.Empty, actualPath);
@@ -154,10 +153,10 @@ public void RemoveLatestSlash_ReturnsEmptyString_WhenBasePathIsEmpty()
 	public void RemoveLatestSlash_ReturnsEmptyString_WhenBasePathIsRoot()
 	{
 		// Arrange
-		string basePath = "/";
+		const string basePath = "/";
 
 		// Act
-		string actualPath = PathHelper.RemoveLatestSlash(basePath);
+		var actualPath = PathHelper.RemoveLatestSlash(basePath);
 
 		// Assert
 		Assert.AreEqual(string.Empty, actualPath);
@@ -263,7 +262,7 @@ public void PrefixDbSlash_WhenCalledWithAlreadyPrefixedPath_ReturnsPathWithoutCh
 		const string expected = "/test/subfolder/file.txt";
 
 		// Act
-		string result = PathHelper.PrefixDbSlash(subPath);
+		var result = PathHelper.PrefixDbSlash(subPath);
 
 		// Assert
 		Assert.AreEqual(expected, result);
@@ -273,10 +272,10 @@ public void PrefixDbSlash_WhenCalledWithAlreadyPrefixedPath_ReturnsPathWithoutCh
 	public void TestRemovePrefixDbSlash_WithLeadingSlash()
 	{
 		// Arrange
-		string subPath = "/path/to/file";
+		const string subPath = "/path/to/file";
 
 		// Act
-		string result = PathHelper.RemovePrefixDbSlash(subPath);
+		var result = PathHelper.RemovePrefixDbSlash(subPath);
 
 		// Assert
 		Assert.AreEqual("path/to/file", result);
@@ -286,10 +285,10 @@ public void TestRemovePrefixDbSlash_WithLeadingSlash()
 	public void TestRemovePrefixDbSlash_WithoutLeadingSlash()
 	{
 		// Arrange
-		string subPath = "path/to/file";
+		const string subPath = "path/to/file";
 
 		// Act
-		string result = PathHelper.RemovePrefixDbSlash(subPath);
+		var result = PathHelper.RemovePrefixDbSlash(subPath);
 
 		// Assert
 		Assert.AreEqual("path/to/file", result);
@@ -302,7 +301,7 @@ public void TestRemovePrefixDbSlash_OnlyLeadingSlash()
 		const string subPath = "/";
 
 		// Act
-		string result = PathHelper.RemovePrefixDbSlash(subPath);
+		var result = PathHelper.RemovePrefixDbSlash(subPath);
 
 		// Assert
 		Assert.AreEqual(string.Empty, result);
@@ -361,4 +360,65 @@ public void TestSplitInputFilePaths_WithValidInput_ReturnsArrayOfStrings()
 		Assert.AreEqual("/path/to/file1", result[0]);
 		Assert.AreEqual("/path/to/file2", result[1]);
 	}
+
+	private const string TestContentVeryLongString =
+		"this-is-a-really-long-slug-that-goes-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-and-on-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-and-on-and-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-" +
+		"and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-" +
+		"and-and-on-and-on-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-and-on-" +
+		"and-on-and-on-and-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-" +
+		"and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and" +
+		"-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on" +
+		"-and-on-and-on-and-on-and-on-and-and-on-and-on-and-on-and-on-and-on-and-on-and-" +
+		"on-and-on-and-on-and-and-on-and-and-on-and-on-and-on-and-and-on-and-and-on-and-" +
+		"on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-on-and-and-on-and" +
+		"-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and" +
+		"-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and" +
+		"-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and" +
+		"-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and-and";
 }