Add ContentFileRead test and fix file reading on windows (#4242)

Robust.Shared/ContentPack/ResourceManager.cs

@@ -6,6 +6,7 @@ using System.Text.RegularExpressions;
 using System.Threading;
 using Robust.Shared.Configuration;
 using Robust.Shared.IoC;
+using Robust.Shared.Localization;
 using Robust.Shared.Log;
 using Robust.Shared.Utility;
 
@@ -190,6 +191,14 @@ namespace Robust.Shared.ContentPack
                 throw new FileNotFoundException($"Path '{path}' contains invalid characters/filenames.");
             }
 #endif
+
+            if (path.Value.CanonPath.EndsWith(ResPath.Separator))
+            {
+                // This is a folder, not a file.
+                fileStream = null;
+                return false;
+            }
+
             foreach (var (prefix, root) in _contentRoots)
             {
                 if (!path.Value.TryRelativeTo(prefix, out var relative))

Robust.Shared/Serialization/TypeSerializers/Implementations/ResPathSerializer.cs

@@ -36,18 +36,16 @@ public sealed class ResPathSerializer : ITypeSerializer<ResPath, ValueDataNode>,
         try
         {
             var resourceManager = dependencies.Resolve<IResourceManager>();
-            if (resourceManager.ContentFileExists(path.CanonPath))
+            if (node.Value.EndsWith(ResPath.Separator))
             {
-                return new ValidatedValueNode(node);
+                if (resourceManager.ContentGetDirectoryEntries(path).Any())
+                    return new ValidatedValueNode(node);
+
+                return new ErrorNode(node, $"Folder not found. ({path})");
             }
 
-            if (node.Value.EndsWith(ResPath.SeparatorStr)
-                // Once Resource path is purged this will be
-                //  resourceManager.ContentGetDirectoryEntries(path).Any()
-                && resourceManager.ContentGetDirectoryEntries(new ResPath(path.CanonPath)).Any())
-            {
+            if (resourceManager.ContentFileExists(path))
                 return new ValidatedValueNode(node);
-            }
 
             return new ErrorNode(node, $"File not found. ({path})");
         }

Robust.Shared/Utility/FileHelper.cs

@@ -38,33 +38,57 @@ internal static class FileHelper
 
     private static unsafe bool TryGetFileWindows(string path, [NotNullWhen(true)] out FileStream? stream)
     {
-        HANDLE file;
-        fixed (char* pPath = path)
+        if (path.EndsWith("\\"))
         {
-            file = Windows.CreateFileW(
-                (ushort*)pPath,
-                Windows.GENERIC_READ,
-                FILE.FILE_SHARE_READ,
-                null,
-                OPEN.OPEN_EXISTING,
-                FILE.FILE_ATTRIBUTE_NORMAL,
-                HANDLE.NULL);
+            stream = null;
+            return false;
         }
 
-        if (file == HANDLE.INVALID_VALUE)
+        try
         {
-            var lastError = Marshal.GetLastWin32Error();
-            if (lastError is ERROR.ERROR_FILE_NOT_FOUND or ERROR.ERROR_PATH_NOT_FOUND)
+            HANDLE file;
+            fixed (char* pPath = path)
+            {
+                file = Windows.CreateFileW(
+                    (ushort*)pPath,
+                    Windows.GENERIC_READ,
+                    FILE.FILE_SHARE_READ,
+                    null,
+                    OPEN.OPEN_EXISTING,
+                    FILE.FILE_ATTRIBUTE_NORMAL,
+                    HANDLE.NULL);
+            }
+
+            if (file == HANDLE.INVALID_VALUE)
+            {
+                var lastError = Marshal.GetLastWin32Error();
+                if (lastError is ERROR.ERROR_FILE_NOT_FOUND or ERROR.ERROR_PATH_NOT_FOUND)
+                {
+                    stream = null;
+                    return false;
+                }
+
+                Marshal.ThrowExceptionForHR(Windows.HRESULT_FROM_WIN32(lastError));
+            }
+
+            var sf = new SafeFileHandle(file, ownsHandle: true);
+            stream = new FileStream(sf, FileAccess.Read);
+            return true;
+
+        }
+        catch (UnauthorizedAccessException)
+        {
+            // UnauthorizedAccessException aka this is a folder not a file because of course that is what that means.
+            // Who the fuck though this was the right way of handling that? This should clearly just be a
+            // ERROR_FILE_NOT_FOUND or some other result like that,
+            // https://github.com/dotnet/runtime/issues/70275
+            if (Directory.Exists(path))
             {
                 stream = null;
                 return false;
             }
 
-            Marshal.ThrowExceptionForHR(Windows.HRESULT_FROM_WIN32(lastError));
+            throw;
         }
-
-        var sf = new SafeFileHandle(file, ownsHandle: true);
-        stream = new FileStream(sf, FileAccess.Read);
-        return true;
     }
 }

Robust.UnitTesting/Shared/Resources/ContentFileReadTest.cs

@@ -0,0 +1,63 @@
+using System.Linq;
+using System.Threading.Tasks;
+using NUnit.Framework;
+using Robust.Shared.ContentPack;
+using Robust.Shared.Utility;
+
+namespace Robust.UnitTesting.Shared.Resources;
+
+public sealed class ContentFileReadTest : RobustIntegrationTest
+{
+    [Test]
+    [TestOf(typeof(ResourceManager))]
+    public async Task TestFileReading()
+    {
+        var client = StartClient();
+        await client.WaitIdleAsync();
+
+        var resMan = client.ResolveDependency<IResourceManager>();
+
+        // This tests relies on /Textures/error.rsi existing.
+
+        var rsiFolder = new ResPath("/Textures/error.rsi");
+        var rsiFolderExplicit = new ResPath("/Textures/error.rsi/");
+        var jsonFile = rsiFolder / "meta.json";
+        var missingFile =  rsiFolder / "404FileNotFound";
+        var missingFolder =  rsiFolder / "404FolderNotFound/";
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(resMan.ContentFileExists(jsonFile));
+            Assert.That(resMan.TryContentFileRead(jsonFile, out _));
+            Assert.That(resMan.ContentGetDirectoryEntries(rsiFolder).Any());
+            Assert.That(resMan.ContentGetDirectoryEntries(rsiFolderExplicit).Any());
+        });
+
+        Assert.Multiple(() =>
+        {
+            Assert.DoesNotThrow(() => resMan.ContentFileExists(rsiFolder));
+            Assert.DoesNotThrow(() => resMan.ContentFileExists(rsiFolderExplicit));
+            Assert.DoesNotThrow(() => resMan.ContentFileExists(missingFile));
+            Assert.DoesNotThrow(() => resMan.ContentFileExists(missingFolder));
+
+            Assert.DoesNotThrow(() => resMan.TryContentFileRead(rsiFolder, out _));
+            Assert.DoesNotThrow(() => resMan.TryContentFileRead(rsiFolderExplicit, out _));
+            Assert.DoesNotThrow(() => resMan.TryContentFileRead(missingFile, out _));
+            Assert.DoesNotThrow(() => resMan.TryContentFileRead(missingFolder, out _));
+        });
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(resMan.ContentFileExists(rsiFolder), Is.False);
+            Assert.That(resMan.ContentFileExists(rsiFolderExplicit), Is.False);
+            Assert.That(resMan.ContentFileExists(missingFile), Is.False);
+            Assert.That(resMan.ContentFileExists(missingFolder), Is.False);
+
+            Assert.That(resMan.TryContentFileRead(rsiFolder, out _), Is.False);
+            Assert.That(resMan.TryContentFileRead(rsiFolderExplicit, out _), Is.False);
+            Assert.That(resMan.TryContentFileRead(missingFile, out _), Is.False);
+            Assert.That(resMan.TryContentFileRead(missingFolder , out _), Is.False);
+        });
+    }
+}
+