From 10ce19ae0939128b8c9f514a344e0a6963ae180e Mon Sep 17 00:00:00 2001 From: Stuart Padley Date: Sat, 21 Mar 2026 11:17:40 -0700 Subject: [PATCH 1/6] InstallExternalLibrary/UninstallExternalLibrary: ZIP extraction, manifest-based cleanup, conflict detection, ALTER support - Fix build script to use VS 2022 instead of VS 2019 - Add global.json to pin .NET SDK 8.0 - Fix: use library name as-is (no unconditional .dll append) - InstallExternalLibrary: ZIP extraction with file conflict detection, manifest tracking - UninstallExternalLibrary: manifest-based targeted file deletion, bottom-up empty dir cleanup - ALTER EXTERNAL LIBRARY: clean up previous install before re-installing - CHANGES.md documenting all changes --- .../src/managed/CSharpExtension.cs | 166 ++++++++++++++++-- 1 file changed, 153 insertions(+), 13 deletions(-) diff --git a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs index ff3ac700..17f97d8d 100644 --- a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs +++ b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs @@ -686,6 +686,17 @@ public static short InstallExternalLibrary( string installDir = Interop.UTF8PtrToStr(libraryInstallDirectory, (ulong)libraryInstallDirectoryLength); string libName = Interop.UTF8PtrToStr(libraryName, (ulong)libraryNameLength); + // If a manifest from a previous install of this same library + // exists (e.g. ALTER EXTERNAL LIBRARY), clean up the old files + // before installing the new content. This prevents orphaned + // files and avoids false conflict errors when the new ZIP + // contains some of the same filenames as the old one. + string existingManifest = Path.Combine(installDir, libName + ".manifest"); + if (File.Exists(existingManifest)) + { + CleanupManifest(existingManifest, installDir); + } + // Check if the file is actually a ZIP by reading magic bytes (PK prefix). // All ZIP format variants start with 0x50 0x4B ('PK'). // If not a ZIP (e.g. a raw DLL), there is nothing to extract — the file @@ -735,21 +746,62 @@ public static short InstallExternalLibrary( Directory.CreateDirectory(installDir); } + // Check for file-level conflicts before extracting. + // Directory (folder) overlaps are allowed — multiple libraries + // may extract into the same parent folders. + var extractedFiles = new List(); + if (innerZipPath != null) { - // Extract the inner zip to the install directory - ZipFile.ExtractToDirectory(innerZipPath, installDir, true); + using (var innerArchive = ZipFile.OpenRead(innerZipPath)) + { + foreach (var entry in innerArchive.Entries) + { + if (string.IsNullOrEmpty(entry.Name)) + continue; + + string relPath = entry.FullName.Replace('/', Path.DirectorySeparatorChar); + string destFile = Path.Combine(installDir, relPath); + + if (File.Exists(destFile)) + { + throw new InvalidOperationException( + $"Cannot install library '{libName}': file '{relPath}' already exists. " + + "Another library has already installed a file with this name."); + } + + extractedFiles.Add(relPath); + } + } + + // All conflict checks passed — extract + ZipFile.ExtractToDirectory(innerZipPath, installDir, false); } else { - // Copy all extracted files directly to the install directory + // Collect files from the extracted temp directory + CollectRelativeFiles(tempFolder, "", extractedFiles); + + // Check for file conflicts before copying + foreach (string relPath in extractedFiles) + { + string destFile = Path.Combine(installDir, relPath); + if (File.Exists(destFile)) + { + throw new InvalidOperationException( + $"Cannot install library '{libName}': file '{relPath}' already exists. " + + "Another library has already installed a file with this name."); + } + } + + // All conflict checks passed — copy files foreach (string file in Directory.GetFiles(tempFolder)) { string destFile = Path.Combine(installDir, Path.GetFileName(file)); - File.Copy(file, destFile, true); + File.Copy(file, destFile, false); } - // Copy subdirectories + // Copy subdirectories (merge into existing dirs) foreach (string dir in Directory.GetDirectories(tempFolder)) { string destDir = Path.Combine(installDir, Path.GetFileName(dir)); @@ -761,15 +813,25 @@ public static short InstallExternalLibrary( // DllUtils.CreateDllList (which searches for "{libName}.*") can find it. // When the ZIP contains files with different names (e.g. "RegexSample.dll" // for a library named "regex"), copy the first DLL as "{libName}.dll". - if (Directory.GetFiles(installDir, libName + ".*").Length == 0) + if (Directory.GetFiles(installDir, libName + ".*").Length == 0 && + !File.Exists(Path.Combine(installDir, libName))) { string[] dlls = Directory.GetFiles(installDir, "*.dll"); if (dlls.Length > 0) { - string destFile = Path.Combine(installDir, libName + ".dll"); + string destFile = Path.Combine(installDir, libName); File.Copy(dlls[0], destFile, true); + extractedFiles.Add(libName); } } + + // Write a manifest listing all extracted file paths so that + // UninstallExternalLibrary can remove exactly these files. + if (extractedFiles.Count > 0) + { + string manifestPath = Path.Combine(installDir, libName + ".manifest"); + File.WriteAllLines(manifestPath, extractedFiles); + } } else { @@ -780,7 +842,7 @@ public static short InstallExternalLibrary( Directory.CreateDirectory(installDir); } - string destFile = Path.Combine(installDir, libName + ".dll"); + string destFile = Path.Combine(installDir, libName); File.Copy(libFilePath, destFile, true); } } @@ -839,18 +901,24 @@ public static short UninstallExternalLibrary( try { string installDir = Interop.UTF8PtrToStr(libraryInstallDirectory, (ulong)libraryInstallDirectoryLength); + string libName = Interop.UTF8PtrToStr(libraryName, (ulong)libraryNameLength); if (Directory.Exists(installDir)) { - // Delete all contents within the install directory - foreach (string file in Directory.GetFiles(installDir)) + // Check for a manifest written during install that lists + // all files extracted from the library's ZIP content. + string manifestPath = Path.Combine(installDir, libName + ".manifest"); + if (File.Exists(manifestPath)) { - File.Delete(file); + CleanupManifest(manifestPath, installDir); } - foreach (string dir in Directory.GetDirectories(installDir)) + // Delete the library file itself (for non-ZIP libraries + // that were copied directly during install) + string libraryFile = Path.Combine(installDir, libName); + if (File.Exists(libraryFile)) { - Directory.Delete(dir, true); + File.Delete(libraryFile); } } } @@ -900,5 +968,77 @@ private static void CopyDirectory(string sourceDir, string destDir) CopyDirectory(dir, destSubDir); } } + + /// + /// Recursively collects all file paths relative to the root directory. + /// + private static void CollectRelativeFiles(string directory, string prefix, List results) + { + foreach (string file in Directory.GetFiles(directory)) + { + string relPath = string.IsNullOrEmpty(prefix) + ? Path.GetFileName(file) + : Path.Combine(prefix, Path.GetFileName(file)); + results.Add(relPath); + } + + foreach (string dir in Directory.GetDirectories(directory)) + { + string dirName = Path.GetFileName(dir); + string newPrefix = string.IsNullOrEmpty(prefix) + ? dirName + : Path.Combine(prefix, dirName); + CollectRelativeFiles(dir, newPrefix, results); + } + } + + /// + /// Reads a manifest file, deletes all listed files, removes any + /// directories that become empty (bottom-up), then deletes the + /// manifest itself. Used by both UninstallExternalLibrary and + /// InstallExternalLibrary (to clean up a previous version during + /// ALTER EXTERNAL LIBRARY). + /// + private static void CleanupManifest(string manifestPath, string installDir) + { + string[] extractedFiles = File.ReadAllLines(manifestPath); + var parentDirs = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (string relPath in extractedFiles) + { + if (string.IsNullOrWhiteSpace(relPath)) + continue; + + string fullPath = Path.Combine(installDir, relPath); + if (File.Exists(fullPath)) + { + File.Delete(fullPath); + } + + // Track parent directories for empty-dir cleanup + string dir = Path.GetDirectoryName(fullPath); + while (!string.IsNullOrEmpty(dir) && + !dir.Equals(installDir, StringComparison.OrdinalIgnoreCase)) + { + parentDirs.Add(dir); + dir = Path.GetDirectoryName(dir); + } + } + + // Remove empty directories bottom-up (deepest first) + var sortedDirs = new List(parentDirs); + sortedDirs.Sort((a, b) => b.Length.CompareTo(a.Length)); + foreach (string dir in sortedDirs) + { + if (Directory.Exists(dir) && + Directory.GetFiles(dir).Length == 0 && + Directory.GetDirectories(dir).Length == 0) + { + Directory.Delete(dir, false); + } + } + + File.Delete(manifestPath); + } } } From 1a782e32f2d2d14c7de55919d478b7479f8dc91a Mon Sep 17 00:00:00 2001 From: Stuart Padley Date: Fri, 17 Apr 2026 05:18:14 -0700 Subject: [PATCH 2/6] Add tests for manifest-based install/uninstall, conflict detection, ALTER New tests: - ManifestWrittenTest, ManifestListsNestedFilesTest - InstallLibNameAliasNoExtensionTest - DirectoryOverlapAllowedTest, FileConflictFailsTest - UninstallPreservesOtherLibrariesTest - UninstallRemovesEmptyNestedDirsTest - AlterExternalLibraryTest Update 3 existing tests to reflect new libName-without-extension naming: - InstallInvalidZipTest, InstallRawDllNotZipTest, InstallZipWithManyFilesTest --- .../test/src/native/CSharpLibraryTests.cpp | 365 +++++++++++++++++- 1 file changed, 357 insertions(+), 8 deletions(-) diff --git a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp index cf1a9d59..93f1f974 100644 --- a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp +++ b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp @@ -12,6 +12,8 @@ // //********************************************************************* #include "CSharpExtensionApiTests.h" +#include +#include using namespace std; namespace fs = experimental::filesystem; @@ -211,8 +213,8 @@ namespace ExtensionApiTest "bad-package", packagePath, installDir); EXPECT_EQ(result, SQL_SUCCESS); - EXPECT_TRUE(fs::exists(fs::path(installDir) / "bad-package.dll")) - << "Raw file not found in install directory as bad-package.dll"; + EXPECT_TRUE(fs::exists(fs::path(installDir) / "bad-package")) + << "Raw file not found in install directory as bad-package"; CleanupInstallDir(installDir); } @@ -384,9 +386,9 @@ namespace ExtensionApiTest "rawdllpackage", packagePath, installDir); EXPECT_EQ(result, SQL_SUCCESS); - // The raw DLL should be copied using the library name with .dll extension - EXPECT_TRUE(fs::exists(fs::path(installDir) / "rawdllpackage.dll")) - << "Raw DLL not found in install directory as rawdllpackage.dll"; + // The raw DLL should be copied using the library name (no extension) + EXPECT_TRUE(fs::exists(fs::path(installDir) / "rawdllpackage")) + << "Raw DLL not found in install directory as rawdllpackage"; CleanupInstallDir(installDir); } @@ -446,10 +448,10 @@ namespace ExtensionApiTest dllCount++; } } - EXPECT_EQ(dllCount, 51) << "Expected 50 extracted DLL files + 1 alias for library name, found " << dllCount; + EXPECT_EQ(dllCount, 50) << "Expected 50 extracted DLL files, found " << dllCount; - // Verify the alias DLL exists so DllUtils can discover the library by name - EXPECT_TRUE(fs::exists(fs::path(installDir) / "manyfilespackage.dll")); + // Verify the extensionless alias exists so DllUtils can discover the library by name + EXPECT_TRUE(fs::exists(fs::path(installDir) / "manyfilespackage")); CleanupInstallDir(installDir); } @@ -531,4 +533,351 @@ namespace ExtensionApiTest CleanupInstallDir(installDir); } + + //---------------------------------------------------------------------------------------------- + // Helper: read manifest file into a vector of relative paths + // + static vector ReadManifest(const string &manifestPath) + { + vector lines; + ifstream f(manifestPath); + string line; + while (getline(f, line)) + { + if (!line.empty() && line.back() == '\r') + { + line.pop_back(); + } + if (!line.empty()) + { + lines.push_back(line); + } + } + return lines; + } + + //---------------------------------------------------------------------------------------------- + // Name: ManifestWrittenTest + // + // Description: + // Verifies that after installing a ZIP package, a manifest file named + // "{libName}.manifest" is written in the install directory and lists + // every file extracted from the package. + // + TEST_F(CSharpExtensionApiTests, ManifestWrittenTest) + { + string packagesPath = GetPackagesPath(); + string packagePath = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + ASSERT_TRUE(fs::exists(packagePath)); + + string installDir = CreateInstallDir(); + + SQLRETURN result = CallInstall(sm_installExternalLibraryFuncPtr, + "testpackageB", packagePath, installDir); + EXPECT_EQ(result, SQL_SUCCESS); + + string manifestPath = (fs::path(installDir) / "testpackageB.manifest").string(); + ASSERT_TRUE(fs::exists(manifestPath)) << "Manifest file not created"; + + vector entries = ReadManifest(manifestPath); + EXPECT_GE(entries.size(), 2u) << "Manifest should list at least 2 extracted files"; + + // Manifest should contain the actual extracted file names + bool hasDll = false; + bool hasDeps = false; + for (const auto &e : entries) + { + if (e.find("testpackageB.dll") != string::npos) hasDll = true; + if (e.find("testpackageB.deps.json") != string::npos) hasDeps = true; + } + EXPECT_TRUE(hasDll) << "Manifest missing testpackageB.dll"; + EXPECT_TRUE(hasDeps) << "Manifest missing testpackageB.deps.json"; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: ManifestListsNestedFilesTest + // + // Description: + // Verifies that the manifest records nested file paths using the + // relative path form so that uninstall can locate the files. + // + TEST_F(CSharpExtensionApiTests, ManifestListsNestedFilesTest) + { + string packagesPath = GetPackagesPath(); + string packagePath = (fs::path(packagesPath) / "testpackageD-NESTED.zip").string(); + ASSERT_TRUE(fs::exists(packagePath)); + + string installDir = CreateInstallDir(); + + SQLRETURN result = CallInstall(sm_installExternalLibraryFuncPtr, + "nestedlib", packagePath, installDir); + EXPECT_EQ(result, SQL_SUCCESS); + + string manifestPath = (fs::path(installDir) / "nestedlib.manifest").string(); + ASSERT_TRUE(fs::exists(manifestPath)); + + vector entries = ReadManifest(manifestPath); + + bool hasNestedDll = false; + bool hasRuntimeDll = false; + for (const auto &e : entries) + { + // Accept either separator for cross-platform resilience + if (e.find("MyLib.dll") != string::npos && + (e.find("net8.0") != string::npos)) + { + hasNestedDll = true; + } + if (e.find("native.dll") != string::npos && + e.find("win-x64") != string::npos) + { + hasRuntimeDll = true; + } + } + EXPECT_TRUE(hasNestedDll) << "Manifest missing lib/net8.0/MyLib.dll entry"; + EXPECT_TRUE(hasRuntimeDll) << "Manifest missing runtimes/win-x64/native.dll entry"; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: InstallLibNameAliasNoExtensionTest + // + // Description: + // When the ZIP does not contain a file matching "{libName}.*", the + // install routine creates an alias named "{libName}" (without any + // extension). Verifies the alias is present and the manifest lists it. + // + TEST_F(CSharpExtensionApiTests, InstallLibNameAliasNoExtensionTest) + { + string packagesPath = GetPackagesPath(); + string packagePath = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + ASSERT_TRUE(fs::exists(packagePath)); + + string installDir = CreateInstallDir(); + + // Library name "myAlias" does not match the package's testpackageB.* + SQLRETURN result = CallInstall(sm_installExternalLibraryFuncPtr, + "myAlias", packagePath, installDir); + EXPECT_EQ(result, SQL_SUCCESS); + + // Alias file created as libName exactly (no ".dll" extension) + EXPECT_TRUE(fs::exists(fs::path(installDir) / "myAlias")) + << "Expected alias file 'myAlias' (no extension) not found"; + EXPECT_FALSE(fs::exists(fs::path(installDir) / "myAlias.dll")) + << "Alias should NOT have .dll extension"; + + // Manifest should include the alias + vector entries = ReadManifest( + (fs::path(installDir) / "myAlias.manifest").string()); + bool hasAlias = false; + for (const auto &e : entries) + { + if (e == "myAlias") { hasAlias = true; break; } + } + EXPECT_TRUE(hasAlias) << "Manifest missing alias entry 'myAlias'"; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: DirectoryOverlapAllowedTest + // + // Description: + // Installing two libraries into the same install directory succeeds + // as long as they do not share any filenames. Directory (folder) + // overlap is explicitly permitted. + // + TEST_F(CSharpExtensionApiTests, DirectoryOverlapAllowedTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + ASSERT_TRUE(fs::exists(pkgA)); + ASSERT_TRUE(fs::exists(pkgB)); + + string installDir = CreateInstallDir(); + + // Install lib1 from package A (contents: testpackageA.dll, testpackageA.txt) + SQLRETURN r1 = CallInstall(sm_installExternalLibraryFuncPtr, + "lib1", pkgA, installDir); + EXPECT_EQ(r1, SQL_SUCCESS); + + // Install lib2 from package B (contents: testpackageB.dll, testpackageB.deps.json) + // No filename conflict => succeeds + SQLRETURN r2 = CallInstall(sm_installExternalLibraryFuncPtr, + "lib2", pkgB, installDir); + EXPECT_EQ(r2, SQL_SUCCESS); + + // Both libraries' files coexist + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "lib1.manifest")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "lib2.manifest")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: FileConflictFailsTest + // + // Description: + // Installing a second library that would overwrite a file already + // installed by another library must fail with SQL_ERROR. The first + // library's files must remain intact. + // + TEST_F(CSharpExtensionApiTests, FileConflictFailsTest) + { + string packagesPath = GetPackagesPath(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + ASSERT_TRUE(fs::exists(pkgB)); + + string installDir = CreateInstallDir(); + + // Install "lib1" from package B + SQLRETURN r1 = CallInstall(sm_installExternalLibraryFuncPtr, + "lib1", pkgB, installDir); + EXPECT_EQ(r1, SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + + // Install a DIFFERENT library "lib2" from the same package. + // Both would write testpackageB.dll => conflict, must fail. + SQLRETURN r2 = CallInstall(sm_installExternalLibraryFuncPtr, + "lib2", pkgB, installDir); + EXPECT_EQ(r2, SQL_ERROR) << "Expected conflict error on duplicate filename"; + + // lib1's files must survive + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "lib1.manifest")); + // lib2 must not have written a manifest + EXPECT_FALSE(fs::exists(fs::path(installDir) / "lib2.manifest")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: UninstallPreservesOtherLibrariesTest + // + // Description: + // Uninstalling one library must only delete that library's files + // (as listed in its manifest). Other libraries' files in the same + // directory must remain untouched. + // + TEST_F(CSharpExtensionApiTests, UninstallPreservesOtherLibrariesTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + + string installDir = CreateInstallDir(); + + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "lib1", pkgA, installDir), SQL_SUCCESS); + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "lib2", pkgB, installDir), SQL_SUCCESS); + + // Uninstall lib1 only + SQLRETURN r = CallUninstall(sm_uninstallExternalLibraryFuncPtr, + "lib1", installDir); + EXPECT_EQ(r, SQL_SUCCESS); + + // lib1's files + manifest gone + EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.txt")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "lib1.manifest")); + + // lib2's files + manifest intact + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.deps.json")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "lib2.manifest")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: UninstallRemovesEmptyNestedDirsTest + // + // Description: + // After uninstalling a library whose files lived in nested + // subdirectories, the now-empty nested directories are removed + // bottom-up. + // + TEST_F(CSharpExtensionApiTests, UninstallRemovesEmptyNestedDirsTest) + { + string packagesPath = GetPackagesPath(); + string pkgD = (fs::path(packagesPath) / "testpackageD-NESTED.zip").string(); + ASSERT_TRUE(fs::exists(pkgD)); + + string installDir = CreateInstallDir(); + + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "nestedlib", pkgD, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "lib" / "net8.0")); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "runtimes" / "win-x64")); + + // Uninstall + SQLRETURN r = CallUninstall(sm_uninstallExternalLibraryFuncPtr, + "nestedlib", installDir); + EXPECT_EQ(r, SQL_SUCCESS); + + // Nested directories should have been removed when they became empty + EXPECT_FALSE(fs::exists(fs::path(installDir) / "lib" / "net8.0")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "lib")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "runtimes" / "win-x64")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "runtimes")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: AlterExternalLibraryTest + // + // Description: + // Installing a library a second time with the same libName (simulating + // ALTER EXTERNAL LIBRARY) removes the old content tracked by the + // previous manifest before extracting the new package, even when the + // new package would otherwise conflict with leftover files. + // + TEST_F(CSharpExtensionApiTests, AlterExternalLibraryTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + + string installDir = CreateInstallDir(); + + // v1: install as "myLib" from package A => testpackageA.dll, testpackageA.txt + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "myLib", pkgA, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + + // v2: install again as "myLib" from package B (ALTER) + SQLRETURN r = CallInstall(sm_installExternalLibraryFuncPtr, + "myLib", pkgB, installDir); + EXPECT_EQ(r, SQL_SUCCESS) << "ALTER-style reinstall should succeed"; + + // v1's unique files gone + EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.txt")); + + // v2's files present + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.deps.json")); + + // Manifest reflects v2 content only + vector entries = ReadManifest( + (fs::path(installDir) / "myLib.manifest").string()); + bool hasA = false, hasB = false; + for (const auto &e : entries) + { + if (e.find("testpackageA") != string::npos) hasA = true; + if (e.find("testpackageB") != string::npos) hasB = true; + } + EXPECT_FALSE(hasA) << "Manifest still references v1 files"; + EXPECT_TRUE(hasB) << "Manifest missing v2 files"; + + CleanupInstallDir(installDir); + } } From 80606500e776c4dc1f1084de9ffc48874db0ed89 Mon Sep 17 00:00:00 2001 From: Stuart Padley Date: Fri, 17 Apr 2026 06:14:29 -0700 Subject: [PATCH 3/6] Add 6 production-quality tests for Install/Uninstall API - ErrorMessagePopulatedOnFailureTest: verify libError is surfaced to SQL Server for non-existent file, zip-slip, and file-conflict failure modes - UninstallNonZipLibraryTest: raw-DLL install/uninstall (no-manifest path) - InnerZipFileConflictFailsTest: conflict detection in inner-zip code path - TempFolderCleanedUpAfterConflictTest: no GUID temp-dir leaks after failures - AlterFromNonZipToZipTest: ALTER from raw DLL to ZIP (missing-manifest case) - AliasFileRemovedOnUninstallTest: libName-alias file recorded in manifest and removed on uninstall --- .../test/src/native/CSharpLibraryTests.cpp | 302 ++++++++++++++++++ 1 file changed, 302 insertions(+) diff --git a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp index 93f1f974..546f9cd9 100644 --- a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp +++ b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp @@ -134,6 +134,68 @@ namespace ExtensionApiTest return hasFiles; } + // Helper: call InstallExternalLibrary and capture the error message (if any). + // Returns SQL result; populates errorMessage with the UTF-8 error text. + // + static SQLRETURN CallInstallCaptureError( + FN_installExternalLibrary *installFunc, + const string &libName, + const string &libFilePath, + const string &installDir, + string &errorMessage) + { + SQLCHAR *libError = nullptr; + SQLINTEGER libErrorLength = 0; + + SQLRETURN result = (*installFunc)( + SQLGUID(), + reinterpret_cast(const_cast(libName.c_str())), + static_cast(libName.length()), + reinterpret_cast(const_cast(libFilePath.c_str())), + static_cast(libFilePath.length()), + reinterpret_cast(const_cast(installDir.c_str())), + static_cast(installDir.length()), + &libError, + &libErrorLength); + + errorMessage.clear(); + if (libError != nullptr && libErrorLength > 0) + { + errorMessage.assign(reinterpret_cast(libError), + static_cast(libErrorLength)); + } + + return result; + } + + // Helper: count GUID-shaped subdirectories in a directory (temp folders + // created by the install code during ZIP extraction). + // + static int CountGuidTempDirs(const string &dir) + { + int count = 0; + if (!fs::exists(dir)) + { + return 0; + } + for (const auto &entry : fs::directory_iterator(dir)) + { + if (!fs::is_directory(entry.path())) + { + continue; + } + string name = entry.path().filename().string(); + // GUID format: 8-4-4-4-12 = 36 chars with hyphens at 8,13,18,23 + if (name.length() == 36 && + name[8] == '-' && name[13] == '-' && + name[18] == '-' && name[23] == '-') + { + ++count; + } + } + return count; + } + //---------------------------------------------------------------------------------------------- // Name: InstallZipContainingZipTest // @@ -880,4 +942,244 @@ namespace ExtensionApiTest CleanupInstallDir(installDir); } + + //---------------------------------------------------------------------------------------------- + // Name: ErrorMessagePopulatedOnFailureTest + // + // Description: + // SQL Server surfaces the libraryError string to end users. Every failure + // path must populate libError with a non-empty, UTF-8-decodable message. + // Validates this for three distinct failure modes: non-existent source file, + // zip-slip attack, and file-level conflict. + // + TEST_F(CSharpExtensionApiTests, ErrorMessagePopulatedOnFailureTest) + { + string packagesPath = GetPackagesPath(); + string installDir = CreateInstallDir(); + string msg; + + // Failure mode 1: non-existent source file + SQLRETURN r = CallInstallCaptureError(sm_installExternalLibraryFuncPtr, + "missing", "C:\\does\\not\\exist.zip", installDir, msg); + EXPECT_EQ(r, SQL_ERROR); + EXPECT_FALSE(msg.empty()) << "No error message populated for missing file"; + + // Failure mode 2: zip-slip attack + string zipSlip = (fs::path(packagesPath) / "testpackageH-ZIPSLIP.zip").string(); + r = CallInstallCaptureError(sm_installExternalLibraryFuncPtr, + "slip", zipSlip, installDir, msg); + EXPECT_EQ(r, SQL_ERROR); + EXPECT_FALSE(msg.empty()) << "No error message populated for zip-slip"; + + // Failure mode 3: file-level conflict + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "libA", pkgB, installDir), SQL_SUCCESS); + r = CallInstallCaptureError(sm_installExternalLibraryFuncPtr, + "libB", pkgB, installDir, msg); + EXPECT_EQ(r, SQL_ERROR); + EXPECT_FALSE(msg.empty()) << "No error message populated for conflict"; + // Message should mention the conflicting library name for diagnosability + EXPECT_NE(msg.find("libB"), string::npos) + << "Conflict message should include library name. Got: " << msg; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: UninstallNonZipLibraryTest + // + // Description: + // When a raw DLL (non-ZIP) is installed, no manifest is written. Uninstall + // must still remove the library file via the libName fallback path. + // Exercises the File.Delete(libraryFile) branch in UninstallExternalLibrary. + // + TEST_F(CSharpExtensionApiTests, UninstallNonZipLibraryTest) + { + string packagesPath = GetPackagesPath(); + string rawDll = (fs::path(packagesPath) / "testpackageE-RAWDLL.dll").string(); + ASSERT_TRUE(fs::exists(rawDll)); + + string installDir = CreateInstallDir(); + + // Install the raw DLL as "rawlib" — no manifest should be written + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "rawlib", rawDll, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "rawlib")); + ASSERT_FALSE(fs::exists(fs::path(installDir) / "rawlib.manifest")) + << "Raw-DLL install should not write a manifest"; + + // Uninstall must still delete the library file via the libName fallback + SQLRETURN r = CallUninstall(sm_uninstallExternalLibraryFuncPtr, + "rawlib", installDir); + EXPECT_EQ(r, SQL_SUCCESS); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "rawlib")) + << "Raw library file not removed by uninstall"; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: InnerZipFileConflictFailsTest + // + // Description: + // FileConflictFailsTest exercises the direct-files code path (package B, + // no inner zip). This test exercises the inner-zip code path (package A) + // which has its own separate conflict-detection loop. Regressing only + // one path must be caught. + // + TEST_F(CSharpExtensionApiTests, InnerZipFileConflictFailsTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + ASSERT_TRUE(fs::exists(pkgA)); + + string installDir = CreateInstallDir(); + + // Install "lib1" from package A (inner-zip path) => testpackageA.dll + .txt + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "lib1", pkgA, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "lib1.manifest")); + + // Install "lib2" from the same package — inner-zip conflict must fail + SQLRETURN r = CallInstall(sm_installExternalLibraryFuncPtr, + "lib2", pkgA, installDir); + EXPECT_EQ(r, SQL_ERROR) << "Inner-zip path must detect filename conflict"; + + // lib1's state must be untouched + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageA.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "lib1.manifest")); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "lib2.manifest")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: TempFolderCleanedUpAfterConflictTest + // + // Description: + // After a failed install (conflict or otherwise), the GUID-named temp + // folder used for outer-zip extraction must be cleaned up by the finally + // block. Regression would slowly leak disk space on every failed install. + // + TEST_F(CSharpExtensionApiTests, TempFolderCleanedUpAfterConflictTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + + string installDir = CreateInstallDir(); + + // Trigger a conflict: install then reinstall same package under different names + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "lib1", pkgB, installDir), SQL_SUCCESS); + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "lib2", pkgB, installDir), SQL_ERROR); + + // Also trigger inner-zip path conflict + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "libA", pkgA, installDir), SQL_SUCCESS); + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "libB", pkgA, installDir), SQL_ERROR); + + // Also trigger zip-slip failure + string zipSlip = (fs::path(packagesPath) / "testpackageH-ZIPSLIP.zip").string(); + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "slip", zipSlip, installDir), SQL_ERROR); + + EXPECT_EQ(CountGuidTempDirs(installDir), 0) + << "GUID-named temp folder leaked after failed install(s)"; + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: AlterFromNonZipToZipTest + // + // Description: + // ALTER EXTERNAL LIBRARY scenario where v1 was a raw DLL (no manifest) + // and v2 is a ZIP package. Install code must handle the missing-manifest + // case gracefully and complete the new install. The old lib file remains + // until explicit uninstall — that matches sicongliu's baseline semantics. + // + TEST_F(CSharpExtensionApiTests, AlterFromNonZipToZipTest) + { + string packagesPath = GetPackagesPath(); + string rawDll = (fs::path(packagesPath) / "testpackageE-RAWDLL.dll").string(); + string pkgB = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); + + string installDir = CreateInstallDir(); + + // v1: raw DLL install (no manifest created) + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "myLib", rawDll, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "myLib")); + ASSERT_FALSE(fs::exists(fs::path(installDir) / "myLib.manifest")); + + // v2: ZIP install of the same libName — must NOT crash on missing manifest + SQLRETURN r = CallInstall(sm_installExternalLibraryFuncPtr, + "myLib", pkgB, installDir); + EXPECT_EQ(r, SQL_SUCCESS) + << "ALTER from non-ZIP to ZIP should succeed even without prior manifest"; + + // v2's files must be present + v2 manifest must exist + EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); + EXPECT_TRUE(fs::exists(fs::path(installDir) / "myLib.manifest")); + + CleanupInstallDir(installDir); + } + + //---------------------------------------------------------------------------------------------- + // Name: AliasFileRemovedOnUninstallTest + // + // Description: + // When a ZIP contains DLLs whose names don't match the library name, + // the install code creates an alias file named exactly {libName} (no + // extension) so DllUtils.CreateDllList can discover it. This alias + // must be recorded in the manifest and removed on uninstall — + // otherwise orphaned alias files accumulate over install/uninstall + // cycles. + // + TEST_F(CSharpExtensionApiTests, AliasFileRemovedOnUninstallTest) + { + string packagesPath = GetPackagesPath(); + string pkgA = (fs::path(packagesPath) / "testpackageA-ZIP.zip").string(); + + string installDir = CreateInstallDir(); + + // Install package A (contains testpackageA.dll) under a different libName. + // Since no file matches "aliaslib.*", the install code must create an + // "aliaslib" alias file copied from the first DLL. + ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, + "aliaslib", pkgA, installDir), SQL_SUCCESS); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "aliaslib")) + << "Alias file not created"; + + // The alias must also be listed in the manifest + vector entries = ReadManifest( + (fs::path(installDir) / "aliaslib.manifest").string()); + bool hasAlias = false; + for (const auto &e : entries) + { + if (e == "aliaslib") + { + hasAlias = true; + break; + } + } + EXPECT_TRUE(hasAlias) << "Alias file not recorded in manifest"; + + // Uninstall must remove both the content and the alias + ASSERT_EQ(CallUninstall(sm_uninstallExternalLibraryFuncPtr, + "aliaslib", installDir), SQL_SUCCESS); + EXPECT_FALSE(fs::exists(fs::path(installDir) / "aliaslib")) + << "Alias file leaked after uninstall"; + EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.dll")) + << "Content file leaked after uninstall"; + EXPECT_FALSE(fs::exists(fs::path(installDir) / "aliaslib.manifest")) + << "Manifest file leaked after uninstall"; + + CleanupInstallDir(installDir); + } } From f55001dec2b938a925ad348ce7749becfcbcd9f3 Mon Sep 17 00:00:00 2001 From: Stuart Padley Date: Fri, 17 Apr 2026 06:42:26 -0700 Subject: [PATCH 4/6] Address review: alias naming, zip-slip DID, ALTER atomicity, libName validation, Linux case-sensitivity, cleanup - Alias file now written as {libName}.dll so DllUtils.CreateDllList ({libName}.*) finds it (fixes feature regression on Linux) - Raw-DLL install writes {libName}.dll; uninstall fallback updated to match - Defense-in-depth zip-slip check on inner-ZIP entries before adding to manifest and again in CleanupManifest before deleting - ALTER is now transactional: stage + validate new ZIP before removing old version, with manifest-aware conflict suppression - Reject library names containing path separators, '..', null, or absolute paths - Use OS-appropriate path comparer (Ordinal on Linux, OrdinalIgnoreCase on Windows) - Sort dirs for bottom-up cleanup by separator count, not string length - Extract conflict detection into single helper (removes duplicate check) - Trim error messages and comments; update 3 tests to match new alias naming --- .../src/managed/CSharpExtension.cs | 364 +++++++++++------- .../test/src/native/CSharpLibraryTests.cpp | 72 ++-- 2 files changed, 251 insertions(+), 185 deletions(-) diff --git a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs index 17f97d8d..9e2fb4eb 100644 --- a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs +++ b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs @@ -686,180 +686,131 @@ public static short InstallExternalLibrary( string installDir = Interop.UTF8PtrToStr(libraryInstallDirectory, (ulong)libraryInstallDirectoryLength); string libName = Interop.UTF8PtrToStr(libraryName, (ulong)libraryNameLength); - // If a manifest from a previous install of this same library - // exists (e.g. ALTER EXTERNAL LIBRARY), clean up the old files - // before installing the new content. This prevents orphaned - // files and avoids false conflict errors when the new ZIP - // contains some of the same filenames as the old one. - string existingManifest = Path.Combine(installDir, libName + ".manifest"); - if (File.Exists(existingManifest)) - { - CleanupManifest(existingManifest, installDir); - } + ValidateLibraryName(libName); - // Check if the file is actually a ZIP by reading magic bytes (PK prefix). - // All ZIP format variants start with 0x50 0x4B ('PK'). - // If not a ZIP (e.g. a raw DLL), there is nothing to extract — the file - // is already stored by SQL Server where DllUtils will find it, so we - // return success without any action. - bool isZipFile = false; - using (var fs = new FileStream(libFilePath, FileMode.Open, FileAccess.Read)) + if (!Directory.Exists(installDir)) { - byte[] magic = new byte[2]; - isZipFile = fs.Read(magic, 0, 2) == 2 && - magic[0] == 0x50 && magic[1] == 0x4B; + Directory.CreateDirectory(installDir); } - if (isZipFile) - { - tempFolder = Path.Combine(installDir, Guid.NewGuid().ToString()); + string manifestPath = Path.Combine(installDir, libName + ".manifest"); + HashSet oldManifestEntries = ReadManifestEntries(manifestPath); - // Extract the outer zip to a temp directory. - if (Directory.Exists(tempFolder)) + if (!IsZipFile(libFilePath)) + { + // Raw DLL. Clean up any previous manifest-based install of the same + // library, then copy the file as "{libName}.dll" so DllUtils can find it. + if (File.Exists(manifestPath)) { - Directory.Delete(tempFolder, true); + CleanupManifest(manifestPath, installDir); } - ZipFile.ExtractToDirectory(libFilePath, tempFolder); + File.Copy(libFilePath, Path.Combine(installDir, libName + ".dll"), true); + return SQL_SUCCESS; + } - // Verify the archive contained at least one entry - if (Directory.GetFiles(tempFolder).Length == 0 && - Directory.GetDirectories(tempFolder).Length == 0) - { - throw new InvalidOperationException( - "The library archive contains no entries. Nothing to install."); - } + // ZIP path. Stage the new content to a temp folder, validate it, then + // clean up the old version and move the new files into place. This keeps + // ALTER EXTERNAL LIBRARY transactional: a corrupt ZIP leaves the existing + // install intact. + tempFolder = Path.Combine(installDir, Guid.NewGuid().ToString()); + ZipFile.ExtractToDirectory(libFilePath, tempFolder); - // Look for an inner zip file - string innerZipPath = null; - foreach (string file in Directory.GetFiles(tempFolder)) - { - if (Path.GetExtension(file).Equals(".zip", StringComparison.OrdinalIgnoreCase)) - { - innerZipPath = file; - break; - } - } + if (Directory.GetFiles(tempFolder).Length == 0 && + Directory.GetDirectories(tempFolder).Length == 0) + { + throw new InvalidOperationException( + "The library archive contains no entries."); + } - if (!Directory.Exists(installDir)) + string innerZipPath = null; + foreach (string file in Directory.GetFiles(tempFolder)) + { + if (Path.GetExtension(file).Equals(".zip", StringComparison.OrdinalIgnoreCase)) { - Directory.CreateDirectory(installDir); + innerZipPath = file; + break; } + } - // Check for file-level conflicts before extracting. - // Directory (folder) overlaps are allowed — multiple libraries - // may extract into the same parent folders. - var extractedFiles = new List(); - - if (innerZipPath != null) + // Collect relative paths we're about to install, validating each path + // stays under installDir (defense-in-depth zip-slip check on top of + // ZipFile.ExtractToDirectory's own validation). + var extractedFiles = new List(); + if (innerZipPath != null) + { + using (var innerArchive = ZipFile.OpenRead(innerZipPath)) { - using (var innerArchive = ZipFile.OpenRead(innerZipPath)) + foreach (var entry in innerArchive.Entries) { - foreach (var entry in innerArchive.Entries) - { - if (string.IsNullOrEmpty(entry.Name)) - continue; - - string relPath = entry.FullName.Replace('/', Path.DirectorySeparatorChar); - string destFile = Path.Combine(installDir, relPath); - - if (File.Exists(destFile)) - { - throw new InvalidOperationException( - $"Cannot install library '{libName}': file '{relPath}' already exists. " + - "Another library has already installed a file with this name."); - } - - extractedFiles.Add(relPath); - } - } + if (string.IsNullOrEmpty(entry.Name)) + continue; - // All conflict checks passed — extract - ZipFile.ExtractToDirectory(innerZipPath, installDir, false); - } - else - { - // Collect files from the extracted temp directory - CollectRelativeFiles(tempFolder, "", extractedFiles); - - // Check for file conflicts before copying - foreach (string relPath in extractedFiles) - { - string destFile = Path.Combine(installDir, relPath); - if (File.Exists(destFile)) - { - throw new InvalidOperationException( - $"Cannot install library '{libName}': file '{relPath}' already exists. " + - "Another library has already installed a file with this name."); - } + extractedFiles.Add(ValidateRelativePath(installDir, entry.FullName)); } + } + } + else + { + CollectRelativeFiles(tempFolder, "", extractedFiles); + } - // All conflict checks passed — copy files - foreach (string file in Directory.GetFiles(tempFolder)) - { - string destFile = Path.Combine(installDir, Path.GetFileName(file)); - File.Copy(file, destFile, false); - } + CheckForConflicts(installDir, libName, extractedFiles, oldManifestEntries); - // Copy subdirectories (merge into existing dirs) - foreach (string dir in Directory.GetDirectories(tempFolder)) - { - string destDir = Path.Combine(installDir, Path.GetFileName(dir)); - CopyDirectory(dir, destDir); - } - } + // All checks passed. Remove the previous version's files (if any), then + // extract/copy the new content into installDir. + if (File.Exists(manifestPath)) + { + CleanupManifest(manifestPath, installDir); + } - // Ensure at least one file matches the library name pattern so that - // DllUtils.CreateDllList (which searches for "{libName}.*") can find it. - // When the ZIP contains files with different names (e.g. "RegexSample.dll" - // for a library named "regex"), copy the first DLL as "{libName}.dll". - if (Directory.GetFiles(installDir, libName + ".*").Length == 0 && - !File.Exists(Path.Combine(installDir, libName))) + if (innerZipPath != null) + { + ZipFile.ExtractToDirectory(innerZipPath, installDir, false); + } + else + { + foreach (string file in Directory.GetFiles(tempFolder)) { - string[] dlls = Directory.GetFiles(installDir, "*.dll"); - if (dlls.Length > 0) - { - string destFile = Path.Combine(installDir, libName); - File.Copy(dlls[0], destFile, true); - extractedFiles.Add(libName); - } + File.Copy(file, Path.Combine(installDir, Path.GetFileName(file)), false); } - - // Write a manifest listing all extracted file paths so that - // UninstallExternalLibrary can remove exactly these files. - if (extractedFiles.Count > 0) + foreach (string dir in Directory.GetDirectories(tempFolder)) { - string manifestPath = Path.Combine(installDir, libName + ".manifest"); - File.WriteAllLines(manifestPath, extractedFiles); + CopyDirectory(dir, Path.Combine(installDir, Path.GetFileName(dir))); } } - else + + // If no file in installDir matches "{libName}.*", copy the first .dll + // found as "{libName}.dll" so DllUtils.CreateDllList can discover it. + if (Directory.GetFiles(installDir, libName + ".*").Length == 0) { - // Not a ZIP (e.g. a raw DLL) — copy file to the install directory - // using the library name so DllUtils can discover it. - if (!Directory.Exists(installDir)) + string[] dlls = Directory.GetFiles(installDir, "*.dll"); + if (dlls.Length > 0) { - Directory.CreateDirectory(installDir); + string alias = Path.Combine(installDir, libName + ".dll"); + File.Copy(dlls[0], alias, false); + extractedFiles.Add(libName + ".dll"); } + } - string destFile = Path.Combine(installDir, libName); - File.Copy(libFilePath, destFile, true); + if (extractedFiles.Count > 0) + { + File.WriteAllLines(manifestPath, extractedFiles); } } catch (Exception e) { - var stackTracePart = string.IsNullOrEmpty(e.StackTrace) ? string.Empty : e.StackTrace + Environment.NewLine; + string stackTracePart = string.IsNullOrEmpty(e.StackTrace) ? string.Empty : e.StackTrace + Environment.NewLine; Logging.Error(stackTracePart + "Error: " + e.Message); SetLibraryError(e.Message, libraryError, libraryErrorLength); result = SQL_ERROR; } finally { - // Clean up the temp folder if (tempFolder != null && Directory.Exists(tempFolder)) { try { Directory.Delete(tempFolder, true); } - catch { /* Best effort cleanup */ } + catch { /* best-effort */ } } } @@ -913,9 +864,9 @@ public static short UninstallExternalLibrary( CleanupManifest(manifestPath, installDir); } - // Delete the library file itself (for non-ZIP libraries - // that were copied directly during install) - string libraryFile = Path.Combine(installDir, libName); + // Raw-DLL fallback: libraries installed without a manifest + // were copied directly as "{libName}.dll". + string libraryFile = Path.Combine(installDir, libName + ".dll"); if (File.Exists(libraryFile)) { File.Delete(libraryFile); @@ -992,42 +943,148 @@ private static void CollectRelativeFiles(string directory, string prefix, List= 0 || + libName.Contains("..") || + Path.IsPathRooted(libName)) + { + throw new ArgumentException( + $"Library name '{libName}' contains invalid characters."); + } + } + + // A ZIP file of any variant begins with the 'PK' magic bytes (0x50 0x4B). + private static bool IsZipFile(string path) + { + using (var fs = new FileStream(path, FileMode.Open, FileAccess.Read)) + { + byte[] magic = new byte[2]; + return fs.Read(magic, 0, 2) == 2 && magic[0] == 0x50 && magic[1] == 0x4B; + } + } + + // Reads an existing manifest into a set of relative paths. Empty set if absent. + private static HashSet ReadManifestEntries(string manifestPath) + { + var set = new HashSet(s_pathComparer); + if (File.Exists(manifestPath)) + { + foreach (string line in File.ReadAllLines(manifestPath)) + { + if (!string.IsNullOrWhiteSpace(line)) + { + set.Add(line); + } + } + } + return set; + } + + // Converts a ZIP entry path to a platform-native relative path and verifies + // it does not escape installDir (defense-in-depth zip-slip check). + private static string ValidateRelativePath(string installDir, string zipEntryFullName) + { + string relPath = zipEntryFullName.Replace('/', Path.DirectorySeparatorChar); + string fullInstall = Path.GetFullPath(installDir); + string fullCombined = Path.GetFullPath(Path.Combine(fullInstall, relPath)); + string sep = Path.DirectorySeparatorChar.ToString(); + string prefix = fullInstall.EndsWith(sep) ? fullInstall : fullInstall + sep; + + if (!fullCombined.StartsWith(prefix, s_pathComparison)) + { + throw new InvalidOperationException( + $"Library archive contains entry with invalid path: '{zipEntryFullName}'."); + } + return relPath; + } + + // Throws if any staged relative path collides with an existing file that is not + // owned by the previous install of this same library. + private static void CheckForConflicts( + string installDir, + string libName, + List relPaths, + HashSet ownedByPrevious) + { + foreach (string relPath in relPaths) + { + if (ownedByPrevious.Contains(relPath)) + { + continue; + } + if (File.Exists(Path.Combine(installDir, relPath))) + { + throw new InvalidOperationException( + $"Cannot install library '{libName}': file '{relPath}' already exists in the install directory."); + } + } + } + /// - /// Reads a manifest file, deletes all listed files, removes any - /// directories that become empty (bottom-up), then deletes the - /// manifest itself. Used by both UninstallExternalLibrary and - /// InstallExternalLibrary (to clean up a previous version during - /// ALTER EXTERNAL LIBRARY). + /// Reads a manifest, deletes each listed file, removes any directories + /// that become empty (bottom-up), then deletes the manifest itself. /// private static void CleanupManifest(string manifestPath, string installDir) { - string[] extractedFiles = File.ReadAllLines(manifestPath); - var parentDirs = new HashSet(StringComparer.OrdinalIgnoreCase); + string fullInstall = Path.GetFullPath(installDir); + string sep = Path.DirectorySeparatorChar.ToString(); + string prefix = fullInstall.EndsWith(sep) ? fullInstall : fullInstall + sep; - foreach (string relPath in extractedFiles) + string[] entries = File.ReadAllLines(manifestPath); + var parentDirs = new HashSet(s_pathComparer); + + foreach (string relPath in entries) { if (string.IsNullOrWhiteSpace(relPath)) + { continue; + } + + string fullPath; + try + { + fullPath = Path.GetFullPath(Path.Combine(fullInstall, relPath)); + } + catch + { + continue; + } + + // Defense in depth: skip any entry that resolves outside installDir. + if (!fullPath.StartsWith(prefix, s_pathComparison)) + { + continue; + } - string fullPath = Path.Combine(installDir, relPath); if (File.Exists(fullPath)) { File.Delete(fullPath); } - // Track parent directories for empty-dir cleanup string dir = Path.GetDirectoryName(fullPath); while (!string.IsNullOrEmpty(dir) && - !dir.Equals(installDir, StringComparison.OrdinalIgnoreCase)) + !dir.Equals(fullInstall, s_pathComparison)) { parentDirs.Add(dir); dir = Path.GetDirectoryName(dir); } } - // Remove empty directories bottom-up (deepest first) + // Remove empty directories deepest first. var sortedDirs = new List(parentDirs); - sortedDirs.Sort((a, b) => b.Length.CompareTo(a.Length)); + sortedDirs.Sort((a, b) => SeparatorCount(b).CompareTo(SeparatorCount(a))); foreach (string dir in sortedDirs) { if (Directory.Exists(dir) && @@ -1040,5 +1097,18 @@ private static void CleanupManifest(string manifestPath, string installDir) File.Delete(manifestPath); } + + private static int SeparatorCount(string path) + { + int count = 0; + for (int i = 0; i < path.Length; i++) + { + if (path[i] == Path.DirectorySeparatorChar) + { + count++; + } + } + return count; + } } } diff --git a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp index 546f9cd9..b4c6bbfc 100644 --- a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp +++ b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp @@ -275,8 +275,8 @@ namespace ExtensionApiTest "bad-package", packagePath, installDir); EXPECT_EQ(result, SQL_SUCCESS); - EXPECT_TRUE(fs::exists(fs::path(installDir) / "bad-package")) - << "Raw file not found in install directory as bad-package"; + EXPECT_TRUE(fs::exists(fs::path(installDir) / "bad-package.dll")) + << "Raw file not found in install directory as bad-package.dll"; CleanupInstallDir(installDir); } @@ -448,9 +448,9 @@ namespace ExtensionApiTest "rawdllpackage", packagePath, installDir); EXPECT_EQ(result, SQL_SUCCESS); - // The raw DLL should be copied using the library name (no extension) - EXPECT_TRUE(fs::exists(fs::path(installDir) / "rawdllpackage")) - << "Raw DLL not found in install directory as rawdllpackage"; + // The raw DLL should be copied using the library name with .dll extension. + EXPECT_TRUE(fs::exists(fs::path(installDir) / "rawdllpackage.dll")) + << "Raw DLL not found in install directory as rawdllpackage.dll"; CleanupInstallDir(installDir); } @@ -512,8 +512,8 @@ namespace ExtensionApiTest } EXPECT_EQ(dllCount, 50) << "Expected 50 extracted DLL files, found " << dllCount; - // Verify the extensionless alias exists so DllUtils can discover the library by name - EXPECT_TRUE(fs::exists(fs::path(installDir) / "manyfilespackage")); + // Verify the alias exists so DllUtils can discover the library by name. + EXPECT_TRUE(fs::exists(fs::path(installDir) / "manyfilespackage.dll")); CleanupInstallDir(installDir); } @@ -705,14 +705,14 @@ namespace ExtensionApiTest } //---------------------------------------------------------------------------------------------- - // Name: InstallLibNameAliasNoExtensionTest + // Name: InstallLibNameAliasTest // // Description: // When the ZIP does not contain a file matching "{libName}.*", the - // install routine creates an alias named "{libName}" (without any - // extension). Verifies the alias is present and the manifest lists it. + // install routine creates an alias named "{libName}.dll" so that + // DllUtils.CreateDllList (which searches "{libName}.*") can find it. // - TEST_F(CSharpExtensionApiTests, InstallLibNameAliasNoExtensionTest) + TEST_F(CSharpExtensionApiTests, InstallLibNameAliasTest) { string packagesPath = GetPackagesPath(); string packagePath = (fs::path(packagesPath) / "testpackageB-DLL.zip").string(); @@ -725,21 +725,18 @@ namespace ExtensionApiTest "myAlias", packagePath, installDir); EXPECT_EQ(result, SQL_SUCCESS); - // Alias file created as libName exactly (no ".dll" extension) - EXPECT_TRUE(fs::exists(fs::path(installDir) / "myAlias")) - << "Expected alias file 'myAlias' (no extension) not found"; - EXPECT_FALSE(fs::exists(fs::path(installDir) / "myAlias.dll")) - << "Alias should NOT have .dll extension"; + EXPECT_TRUE(fs::exists(fs::path(installDir) / "myAlias.dll")) + << "Expected alias file 'myAlias.dll' not found"; - // Manifest should include the alias + // Manifest should include the alias. vector entries = ReadManifest( (fs::path(installDir) / "myAlias.manifest").string()); bool hasAlias = false; for (const auto &e : entries) { - if (e == "myAlias") { hasAlias = true; break; } + if (e == "myAlias.dll") { hasAlias = true; break; } } - EXPECT_TRUE(hasAlias) << "Manifest missing alias entry 'myAlias'"; + EXPECT_TRUE(hasAlias) << "Manifest missing alias entry 'myAlias.dll'"; CleanupInstallDir(installDir); } @@ -1002,18 +999,18 @@ namespace ExtensionApiTest string installDir = CreateInstallDir(); - // Install the raw DLL as "rawlib" — no manifest should be written + // Install the raw DLL as "rawlib" — no manifest should be written. ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, "rawlib", rawDll, installDir), SQL_SUCCESS); - ASSERT_TRUE(fs::exists(fs::path(installDir) / "rawlib")); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "rawlib.dll")); ASSERT_FALSE(fs::exists(fs::path(installDir) / "rawlib.manifest")) << "Raw-DLL install should not write a manifest"; - // Uninstall must still delete the library file via the libName fallback + // Uninstall must still delete the library file via the libName fallback. SQLRETURN r = CallUninstall(sm_uninstallExternalLibraryFuncPtr, "rawlib", installDir); EXPECT_EQ(r, SQL_SUCCESS); - EXPECT_FALSE(fs::exists(fs::path(installDir) / "rawlib")) + EXPECT_FALSE(fs::exists(fs::path(installDir) / "rawlib.dll")) << "Raw library file not removed by uninstall"; CleanupInstallDir(installDir); @@ -1111,19 +1108,19 @@ namespace ExtensionApiTest string installDir = CreateInstallDir(); - // v1: raw DLL install (no manifest created) + // v1: raw DLL install (no manifest created). ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, "myLib", rawDll, installDir), SQL_SUCCESS); - ASSERT_TRUE(fs::exists(fs::path(installDir) / "myLib")); + ASSERT_TRUE(fs::exists(fs::path(installDir) / "myLib.dll")); ASSERT_FALSE(fs::exists(fs::path(installDir) / "myLib.manifest")); - // v2: ZIP install of the same libName — must NOT crash on missing manifest + // v2: ZIP install of the same libName — must NOT crash on missing manifest. SQLRETURN r = CallInstall(sm_installExternalLibraryFuncPtr, "myLib", pkgB, installDir); EXPECT_EQ(r, SQL_SUCCESS) << "ALTER from non-ZIP to ZIP should succeed even without prior manifest"; - // v2's files must be present + v2 manifest must exist + // v2's files must be present + v2 manifest must exist. EXPECT_TRUE(fs::exists(fs::path(installDir) / "testpackageB.dll")); EXPECT_TRUE(fs::exists(fs::path(installDir) / "myLib.manifest")); @@ -1135,11 +1132,10 @@ namespace ExtensionApiTest // // Description: // When a ZIP contains DLLs whose names don't match the library name, - // the install code creates an alias file named exactly {libName} (no - // extension) so DllUtils.CreateDllList can discover it. This alias - // must be recorded in the manifest and removed on uninstall — - // otherwise orphaned alias files accumulate over install/uninstall - // cycles. + // the install code creates an alias file named {libName}.dll so + // DllUtils.CreateDllList can discover it. This alias must be recorded + // in the manifest and removed on uninstall — otherwise orphaned alias + // files accumulate over install/uninstall cycles. // TEST_F(CSharpExtensionApiTests, AliasFileRemovedOnUninstallTest) { @@ -1150,19 +1146,19 @@ namespace ExtensionApiTest // Install package A (contains testpackageA.dll) under a different libName. // Since no file matches "aliaslib.*", the install code must create an - // "aliaslib" alias file copied from the first DLL. + // "aliaslib.dll" alias file copied from the first DLL. ASSERT_EQ(CallInstall(sm_installExternalLibraryFuncPtr, "aliaslib", pkgA, installDir), SQL_SUCCESS); - ASSERT_TRUE(fs::exists(fs::path(installDir) / "aliaslib")) + ASSERT_TRUE(fs::exists(fs::path(installDir) / "aliaslib.dll")) << "Alias file not created"; - // The alias must also be listed in the manifest + // The alias must also be listed in the manifest. vector entries = ReadManifest( (fs::path(installDir) / "aliaslib.manifest").string()); bool hasAlias = false; for (const auto &e : entries) { - if (e == "aliaslib") + if (e == "aliaslib.dll") { hasAlias = true; break; @@ -1170,10 +1166,10 @@ namespace ExtensionApiTest } EXPECT_TRUE(hasAlias) << "Alias file not recorded in manifest"; - // Uninstall must remove both the content and the alias + // Uninstall must remove both the content and the alias. ASSERT_EQ(CallUninstall(sm_uninstallExternalLibraryFuncPtr, "aliaslib", installDir), SQL_SUCCESS); - EXPECT_FALSE(fs::exists(fs::path(installDir) / "aliaslib")) + EXPECT_FALSE(fs::exists(fs::path(installDir) / "aliaslib.dll")) << "Alias file leaked after uninstall"; EXPECT_FALSE(fs::exists(fs::path(installDir) / "testpackageA.dll")) << "Content file leaked after uninstall"; From 0b3916b9a5f394e2582dac1ec95f52b71bbce696 Mon Sep 17 00:00:00 2001 From: Stuart Padley Date: Fri, 17 Apr 2026 06:48:45 -0700 Subject: [PATCH 5/6] Remove 'fallback' language from comments --- .../dotnet-core-CSharp/src/managed/CSharpExtension.cs | 4 ++-- .../dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs index 9e2fb4eb..70e6cba4 100644 --- a/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs +++ b/language-extensions/dotnet-core-CSharp/src/managed/CSharpExtension.cs @@ -864,8 +864,8 @@ public static short UninstallExternalLibrary( CleanupManifest(manifestPath, installDir); } - // Raw-DLL fallback: libraries installed without a manifest - // were copied directly as "{libName}.dll". + // Non-ZIP installs write a single "{libName}.dll" file and + // no manifest; remove that file directly. string libraryFile = Path.Combine(installDir, libName + ".dll"); if (File.Exists(libraryFile)) { diff --git a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp index b4c6bbfc..599b65bc 100644 --- a/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp +++ b/language-extensions/dotnet-core-CSharp/test/src/native/CSharpLibraryTests.cpp @@ -988,7 +988,7 @@ namespace ExtensionApiTest // // Description: // When a raw DLL (non-ZIP) is installed, no manifest is written. Uninstall - // must still remove the library file via the libName fallback path. + // must still remove the library file by its libName.dll path. // Exercises the File.Delete(libraryFile) branch in UninstallExternalLibrary. // TEST_F(CSharpExtensionApiTests, UninstallNonZipLibraryTest) @@ -1006,7 +1006,7 @@ namespace ExtensionApiTest ASSERT_FALSE(fs::exists(fs::path(installDir) / "rawlib.manifest")) << "Raw-DLL install should not write a manifest"; - // Uninstall must still delete the library file via the libName fallback. + // Uninstall must still delete the libName.dll file. SQLRETURN r = CallUninstall(sm_uninstallExternalLibraryFuncPtr, "rawlib", installDir); EXPECT_EQ(r, SQL_SUCCESS); From af9f8d721a5bd095e5d2c73b05543a63f498f52b Mon Sep 17 00:00:00 2001 From: stuartpa Date: Fri, 17 Apr 2026 09:07:20 -0700 Subject: [PATCH 6/6] Add Microsoft.Data.SqlClient 5.2.2 reference to csproj This PR branch is based on sicongliu's branch which predates PR #83 (DECIMAL Type Support on main). PR #83 added Microsoft.Data.SqlClient 5.2.2 as a required runtime dependency for SqlDecimal handling. Without it, sp_execute_external_script fails with HRESULT 0x80004004 when any script touches DECIMAL types. --- .../src/managed/Microsoft.SqlServer.CSharpExtension.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/language-extensions/dotnet-core-CSharp/src/managed/Microsoft.SqlServer.CSharpExtension.csproj b/language-extensions/dotnet-core-CSharp/src/managed/Microsoft.SqlServer.CSharpExtension.csproj index 3b8c8e44..82cb3f69 100644 --- a/language-extensions/dotnet-core-CSharp/src/managed/Microsoft.SqlServer.CSharpExtension.csproj +++ b/language-extensions/dotnet-core-CSharp/src/managed/Microsoft.SqlServer.CSharpExtension.csproj @@ -11,5 +11,6 @@ + \ No newline at end of file