diff --git a/Robust.Shared/ContentPack/ResourceManager.cs b/Robust.Shared/ContentPack/ResourceManager.cs index 40cad7949..205e45fa5 100644 --- a/Robust.Shared/ContentPack/ResourceManager.cs +++ b/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)) diff --git a/Robust.Shared/Serialization/TypeSerializers/Implementations/ResPathSerializer.cs b/Robust.Shared/Serialization/TypeSerializers/Implementations/ResPathSerializer.cs index a0fa9c5f6..fb5b36b3e 100644 --- a/Robust.Shared/Serialization/TypeSerializers/Implementations/ResPathSerializer.cs +++ b/Robust.Shared/Serialization/TypeSerializers/Implementations/ResPathSerializer.cs @@ -36,18 +36,16 @@ public sealed class ResPathSerializer : ITypeSerializer, try { var resourceManager = dependencies.Resolve(); - 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})"); } diff --git a/Robust.Shared/Utility/FileHelper.cs b/Robust.Shared/Utility/FileHelper.cs index e4dcfe8c8..6dfcea493 100644 --- a/Robust.Shared/Utility/FileHelper.cs +++ b/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; } } diff --git a/Robust.UnitTesting/Shared/Resources/ContentFileReadTest.cs b/Robust.UnitTesting/Shared/Resources/ContentFileReadTest.cs new file mode 100644 index 000000000..7685c4c3d --- /dev/null +++ b/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(); + + // 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); + }); + } +} +