From e5754d8cac39cb4d9b24b696adb33948b00ffb64 Mon Sep 17 00:00:00 2001 From: Andrew Skowronski Date: Fri, 24 Jul 2026 17:03:27 -0400 Subject: [PATCH] [#14] Address Copilot review follow-ups - CreateArchive: marshal only the first `count` entries (matching what native consumes) and validate `count` against the array lengths, instead of marshalling every element. - Clarify that the StringBuilder-output ASCII limitation is Windows-specific (Ansi marshals as UTF-8 on macOS/Linux). - NonAsciiPath.WithCopy: best-effort temp-dir cleanup so a delete failure can't mask a test failure. --- UnityFileSystem.Tests/UnityFileSystemTests.cs | 5 ++++- UnityFileSystem/DllWrapper.cs | 20 ++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/UnityFileSystem.Tests/UnityFileSystemTests.cs b/UnityFileSystem.Tests/UnityFileSystemTests.cs index 394c26d..b131aab 100644 --- a/UnityFileSystem.Tests/UnityFileSystemTests.cs +++ b/UnityFileSystem.Tests/UnityFileSystemTests.cs @@ -27,7 +27,10 @@ public static void WithCopy(string source, string fileName, Action test) } finally { - Directory.Delete(dir, true); + // Best-effort cleanup: a delete failure (e.g. a handle not yet released on Windows) + // must not mask a test failure thrown from the block above. + try { Directory.Delete(dir, true); } + catch { /* ignore */ } } } } diff --git a/UnityFileSystem/DllWrapper.cs b/UnityFileSystem/DllWrapper.cs index dacc8a7..9e2883b 100644 --- a/UnityFileSystem/DllWrapper.cs +++ b/UnityFileSystem/DllWrapper.cs @@ -196,9 +196,10 @@ public static class DllWrapper public static extern ReturnCode GetArchiveNodeCount(UnityArchiveHandle handle, out int count); // Strings returned from native (here and in the other StringBuilder-based calls) come back as - // UTF-8 bytes but are marshalled as the system ANSI code page, so only ASCII round-trips correctly. - // These are internal archive/serialized-file names, which Unity keeps ASCII, so it's not an issue in - // practice. Input paths, by contrast, are marshalled as UTF-8 (LPUTF8Str) to support non-ASCII paths. + // UTF-8 bytes but are marshalled as the system ANSI code page. On Windows that's a lossy code page, + // so only ASCII round-trips correctly (on macOS/Linux it happens to be UTF-8). These are internal + // archive/serialized-file names, which Unity keeps ASCII, so it's not an issue in practice. Input + // paths, by contrast, are marshalled as UTF-8 (LPUTF8Str) to support non-ASCII paths on all platforms. [DllImport("UnityFileSystemApi", CallingConvention = CallingConvention.Cdecl, EntryPoint = "UFS_GetArchiveNode")] @@ -215,14 +216,19 @@ private static extern ReturnCode CreateArchiveNative(IntPtr[] sourceFiles, IntPt public static ReturnCode CreateArchive(string[] sourceFiles, string[] aliases, bool[] isSerializedFile, int count, string archiveFile, CompressionType compression, out int crc) { - var sourcePtrs = new IntPtr[sourceFiles.Length]; - var aliasPtrs = new IntPtr[aliases.Length]; + if (count < 0 || count > sourceFiles.Length || count > aliases.Length || count > isSerializedFile.Length) + throw new ArgumentOutOfRangeException(nameof(count)); + + // Marshal only the first `count` entries, since that's all the native call consumes. + var sourcePtrs = new IntPtr[count]; + var aliasPtrs = new IntPtr[count]; try { - for (int i = 0; i < sourceFiles.Length; ++i) + for (int i = 0; i < count; ++i) + { sourcePtrs[i] = Marshal.StringToCoTaskMemUTF8(sourceFiles[i]); - for (int i = 0; i < aliases.Length; ++i) aliasPtrs[i] = Marshal.StringToCoTaskMemUTF8(aliases[i]); + } return CreateArchiveNative(sourcePtrs, aliasPtrs, isSerializedFile, count, archiveFile, compression, out crc); }