From 9a03e293a8d6c05b82792f676f9a514e97771c5e Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Wed, 17 Sep 2025 16:12:23 -0700 Subject: [PATCH 01/10] Add failing test to cover behavior --- cmd/dbc/sync_test.go | 2 + .../test-driver-invalid-manifest.tar.gz | Bin 0 -> 1134 bytes cmd/dbc/testdata/test_index.yaml | 8 ++- cmd/dbc/uninstall_test.go | 48 ++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 cmd/dbc/testdata/test-driver-invalid-manifest.tar.gz diff --git a/cmd/dbc/sync_test.go b/cmd/dbc/sync_test.go index 9392d89c..d337bb99 100644 --- a/cmd/dbc/sync_test.go +++ b/cmd/dbc/sync_test.go @@ -45,6 +45,8 @@ func downloadTestPkg(pkg dbc.PkgInfo) (*os.File, error) { return os.Open(filepath.Join("testdata", "test-driver-manifest-only.tar.gz")) case "test-driver-no-sig": return os.Open(filepath.Join("testdata", "test-driver-no-sig.tar.gz")) + case "test-driver-invalid-manifest": + return os.Open(filepath.Join("testdata", "test-driver-invalid-manifest.tar.gz")) default: return nil, fmt.Errorf("unknown driver: %s", pkg.Driver.Path) } diff --git a/cmd/dbc/testdata/test-driver-invalid-manifest.tar.gz b/cmd/dbc/testdata/test-driver-invalid-manifest.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..78dbfbf49b843e38e687822ba813fa94fee6b1bf GIT binary patch literal 1134 zcmV-!1d;n6iwFSpILl}N1MOE`Xd_h=o~rBC@j+o>i@xk7f<9zBng2;w1Dhskk~Zm2 zW^9t`5^iR0W+utZB=ggxRSQK01s{|mim=MQi0C4H)F&SnVRwZE1w}+(d=NwzaUcA{ z2PaKz-RQP*QrLxUdgxrk?(%;$h7GOwt$#kW{_m!YN>Z-d>oA6f{mxr`Cb##l--BTzvSkpk z(WoOrLTxu;%9f&=YL`FaAMVAl`?qZB&~ZHnDGGGa1G8e!&qMy7}z@oxAw>nO9cVo__GM7+d0A9u7QP|a%APryT7jn&wR#y@GJZ7laULru7CHA`N-FweCTP{ztI1TS=de? zs31GEe>$*3{ZkCb^wmGhvJ?i|dmqZ*@jtJBh6IMI8z@2vT$p0HaF`-!nxtup5qO;T zl{u3hkLF^j^n8M7L)W#5>#MOy*i@Qx@w^n9vXXppK9elAq+CmBRy27DEhHUAo}DU8 zB@61pOnlO#q&cQW)kTsCnp0`HY__MlXu?Y_rv+JPc@?KA5{Q2Onc|J*f6GV+&W~-ksC@z(abn9E7~McRG~IH# zU7Q=c)?MkJpkGUIUkkSJ|JOf(h*HQI8A%ngQkKkC6U>aHk~qg*%k@>XZ$(E6(9|pbU;tN; zBinkz)h%;_q3I6rzYw?`uF~bUI$UT1{ZD%D`67Ub5YSw=;hYGCRNu-gcM3NoTXoMc zp*`v%Sqmv;Iix#|FP|ZLj0oU$T}Gy}5n(nh1#sVm)ALUeBtZsn{IO_!Ja+2p5vn`V z&4+L_Xd~qm9x$M6IVETtJR9g3*7Y*!#P Date: Thu, 18 Sep 2025 16:38:28 -0700 Subject: [PATCH 02/10] Update tests --- cmd/dbc/search_test.go | 14 ++++++++++++-- cmd/dbc/testdata/test_index.yaml | 9 +++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index 842f7d44..70b87980 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -15,7 +15,8 @@ func (suite *SubcommandTestSuite) TestSearchCmd() { suite.validateOutput("• test-driver-1 - This is a test driver\r\n"+ "• test-driver-2 - This is another test driver\r\n"+ "• test-driver-manifest-only - This is manifest-only driver\r\n"+ - "• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n\r ", "", suite.runCmd(m)) + "• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n"+ + "• test-driver-invalid-manifest - This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n\r ", "", suite.runCmd(m)) } func (suite *SubcommandTestSuite) TestSearchCmdWithInstalled() { @@ -28,7 +29,8 @@ func (suite *SubcommandTestSuite) TestSearchCmdWithInstalled() { downloadPkg: downloadTestPkg}) suite.validateOutput("• test-driver-1 - This is a test driver [installed: env=>1.1.0]\r\n"+ "• test-driver-2 - This is another test driver\r\n• test-driver-manifest-only - This is manifest-only driver\r\n"+ - "• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n\r ", "", suite.runCmd(m)) + "• test-driver-no-sig - Driver manifest missing Files.signature entry\r\n"+ + "• test-driver-invalid-manifest - This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n\r ", "", suite.runCmd(m)) } func (suite *SubcommandTestSuite) TestSearchCmdVerbose() { @@ -46,6 +48,9 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerbose() { "Available Versions:\r\n ╰── 1.0.0\r\n"+ "• test-driver-no-sig\r\n Title: Test Driver No Signature\r\n "+ "Description: Driver manifest missing Files.signature entry\r\n License: Apache-2.0\r\n "+ + "Available Versions:\r\n ╰── 1.0.0\r\n"+ + "• test-driver-invalid-manifest\r\n Title: Test Driver Invalid Manifest\r\n "+ + "Description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n License: Apache-2.0\r\n "+ "Available Versions:\r\n ╰── 1.0.0\r\n\r ", "", suite.runCmd(m)) } @@ -75,5 +80,10 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { " Description: Driver manifest missing Files.signature entry\r\n"+ " License: Apache-2.0\r\n"+ " Available Versions:\r\n"+ + "• test-driver-invalid-manifest\r\n"+ + " Title: Test Driver Invalid Manifest\r\n"+ + " Description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n"+ + " License: Apache-2.0\r\n"+ + " Available Versions:\r\n"+ " ╰── 1.0.0\r\n\r ", "", suite.runCmd(m)) } diff --git a/cmd/dbc/testdata/test_index.yaml b/cmd/dbc/testdata/test_index.yaml index 7f332267..7f27abf9 100644 --- a/cmd/dbc/testdata/test_index.yaml +++ b/cmd/dbc/testdata/test_index.yaml @@ -64,6 +64,15 @@ drivers: description: Driver manifest missing Files.signature entry license: Apache-2.0 path: test-driver-no-sig + pkginfo: + - version: v1.0.0 + packages: + - platform: linux_amd64 + - platform: linux_arm64 + - platform: macos_amd64 + - platform: macos_arm64 + - platform: windows_amd64 + - platform: windows_arm64 - name: Test Driver Invalid Manifest description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37. license: Apache-2.0 From ba2a44969e0c509ed358f51d6c5422b98b499c6e Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 18 Sep 2025 20:12:15 -0700 Subject: [PATCH 03/10] Fix bad rebase I did --- cmd/dbc/search_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index 70b87980..4e204e7c 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -80,6 +80,7 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { " Description: Driver manifest missing Files.signature entry\r\n"+ " License: Apache-2.0\r\n"+ " Available Versions:\r\n"+ + " ╰── 1.0.0\r\n"+ "• test-driver-invalid-manifest\r\n"+ " Title: Test Driver Invalid Manifest\r\n"+ " Description: This is test driver with an invalid manfiest. See https://github.com/columnar-tech/dbc/issues/37.\r\n"+ From 585aa8e680ed1570facf38c9ab2b8720aacba1aa Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 18 Sep 2025 20:12:24 -0700 Subject: [PATCH 04/10] Update driver list test for change in cdn --- cmd/dbc/driver_list_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/dbc/driver_list_test.go b/cmd/dbc/driver_list_test.go index 0e0db41c..6f62f5d3 100644 --- a/cmd/dbc/driver_list_test.go +++ b/cmd/dbc/driver_list_test.go @@ -23,11 +23,11 @@ func TestUnmarshalDriverList(t *testing.T) { expected []dbc.PkgInfo err error }{ - {"basic", "[drivers]\nflightsql = {version = '1.7.0'}", []dbc.PkgInfo{ - {Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.7.0")}, + {"basic", "[drivers]\nflightsql = {version = '1.8.0'}", []dbc.PkgInfo{ + {Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")}, }, nil}, - {"less", "[drivers]\nflightsql = {version = '<=1.7.0'}", []dbc.PkgInfo{ - {Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.7.0")}, + {"less", "[drivers]\nflightsql = {version = '<=1.8.0'}", []dbc.PkgInfo{ + {Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")}, }, nil}, {"greater", "[drivers]\nflightsql = {version = '>=1.7.0'}", []dbc.PkgInfo{ {Driver: dbc.Driver{Path: "flightsql"}, Version: semver.MustParse("1.8.0")}, From 43bd381c69b96a2c48d16f441e83edaf6108b256 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 18 Sep 2025 20:13:32 -0700 Subject: [PATCH 05/10] Catch edge case (fix bug) --- cmd/dbc/uninstall_test.go | 8 +-- config/config.go | 62 ++++++++++++++++++++- config/config_test.go | 112 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 5 deletions(-) diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index 68c7a9e8..7d70ac65 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -193,11 +193,11 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { // Error: failed to uninstall driver: error removing driver test-driver-invalid-manifest: RemoveAll .: invalid argument suite.validateOutput("Driver `test-driver-invalid-manifest` uninstalled successfully!\r\n\r\n\r ", "", output) - // Ensure we don't nuke the tempdir + // Ensure we don't nuke the tempdir which is the original (major) issue suite.DirExists(suite.tempdir) - // Ensure we removed the files from the package though' - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) - suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "test-driver-invalid-manifest.so")) + // Ensure we removed the files from the package though + suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) + suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "test-driver-invalid-manifest.so")) suite.NoDirExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest")) } diff --git a/config/config.go b/config/config.go index a5b6a2b8..eefc276d 100644 --- a/config/config.go +++ b/config/config.go @@ -331,7 +331,36 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { // dbc installs drivers in a folder, other tools may not so we handle each // differently. if info.Source == "dbc" { - if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil { + shared_dir := filepath.Dir(sharedPath) + + // Handle an edge case first. We've already verified that Driver.shared is + // an absolute path inside cfg.Location but we also expect the shared + // library to be in a subfolder, e.g., + // + // ${cfg.Location} + // └── libadbc_driver_linux_amd64_v1.0.0 + // └── libadbc_driver_linux_amd64_v1.0.0.so + // + // If the MANIFEST we're installed has Driver.shared already set, this is + // invalid becuase it should have Files set instead. When this happens, + // the Driver.shared value in the installed manifest ends up as a absolute + // path to a folder, not a shared library. + shared_path_is_subdir, err := IsImmediateSubDir(cfg.Location, sharedPath) + if err != nil { + return fmt.Errorf("error removing driver %s: %w", info.ID, err) + } + if shared_path_is_subdir { + if err := os.RemoveAll(sharedPath); err != nil { + if errors.Is(err, fs.ErrNotExist) { + continue + } + return fmt.Errorf("error removing driver %s: %w", info.ID, err) + } + continue + } + + // The rest of this is "normal" operation... + if err := os.RemoveAll(shared_dir); err != nil { // Ignore only when not found. This supports manifest-only drivers. // TODO: Come up with a better mechanism to handle manifest-only drivers // and remove this continue when we do @@ -371,3 +400,34 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { return nil } + +// Determine whether target is an immediate subdirectory of base. +func IsImmediateSubDir(base string, target string) (bool, error) { + base_abs, err := filepath.Abs(base) + if err != nil { + return false, err + } + target_abs, err := filepath.Abs(target) + if err != nil { + return false, err + } + + result, err := filepath.Rel(base_abs, target_abs) + if err != nil { + return false, err + } + + parts := strings.Split(result, string(filepath.Separator)) + + // return false if we're not a direct child + if len(parts) != 1 { + return false, nil + } + + // return false if we're above + if strings.HasPrefix(result, ".") { + return false, nil + } + + return true, nil +} diff --git a/config/config_test.go b/config/config_test.go index 22f2d919..ffeb98de 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -3,6 +3,7 @@ package config import ( + "fmt" "os" "path/filepath" "testing" @@ -45,3 +46,114 @@ func TestConfigEnvVarHierarchy(t *testing.T) { cfg = loadConfig(ConfigEnv) assert.Equal(t, "", cfg.Location) } + +func TestIsImmediateSubDir(t *testing.T) { + tests := []struct { + parent string + child string + expected bool + expectErr bool + }{ + { + parent: "/foo", + child: "/foo/bar", + expected: true, + }, + { + parent: "/foo/bar", + child: "/foo/bar", + expected: false, + }, + { + parent: "/foo", + child: "/foo/bar/baz", + expected: false, + }, + { + parent: "/foo/bar", + child: "/foo/baz", + expected: false, + }, + { + parent: "/foo", + child: "/bar", + expected: false, + }, + { + parent: "/foo/bar", + child: "/foo/bar/../baz", + expected: false, + }, + { + parent: "/foo", + child: "/foo/bar/", + expected: true, + }, + { + parent: "/foo/", + child: "/foo/bar", + expected: true, + }, + { + parent: "/foo/", + child: "/foo/bar/", + expected: true, + }, + { + parent: "/", + child: "/foo", + expected: true, + }, + { + parent: "", + child: "/foo", + expected: false, + }, + { + parent: "/foo", + child: "", + expected: false, + }, + { + parent: "", + child: "", + expected: false, + }, + { + parent: "foo", + child: "foo/bar", + expected: true, + }, + { + parent: "/foo", + child: "/foo/./bar", + expected: true, + }, + { + parent: "/foo", + child: "/foo/bar/../baz", + expected: true, + }, + { + parent: "/foo", + child: "/foo/bar/some_file.txt", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s vs %s", tt.parent, tt.child), func(t *testing.T) { + result, err := IsImmediateSubDir(tt.parent, tt.child) + + if tt.expectErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.expected, result, + "IsImmediateSubDir(%q, %q) = %v, expected %v", + tt.parent, tt.child, result, tt.expected) + }) + } +} From 1b43728d15f68df0cd60789a7fc91dd79ab69245 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Fri, 19 Sep 2025 10:56:51 -0700 Subject: [PATCH 06/10] Remove outdated comment --- cmd/dbc/uninstall_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index 7d70ac65..99a728eb 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -189,8 +189,6 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { m = UninstallCmd{Driver: "test-driver-invalid-manifest", Level: config.ConfigEnv}.GetModel() output := suite.runCmd(m) - // TODO: Currently failing with: - // Error: failed to uninstall driver: error removing driver test-driver-invalid-manifest: RemoveAll .: invalid argument suite.validateOutput("Driver `test-driver-invalid-manifest` uninstalled successfully!\r\n\r\n\r ", "", output) // Ensure we don't nuke the tempdir which is the original (major) issue From 49cf0fd36d434f5d555d75cb15d58fedd9c866aa Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Fri, 19 Sep 2025 10:58:10 -0700 Subject: [PATCH 07/10] Don't export isImmediateSubDir --- config/config.go | 4 ++-- config/config_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index eefc276d..762c71dc 100644 --- a/config/config.go +++ b/config/config.go @@ -345,7 +345,7 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { // invalid becuase it should have Files set instead. When this happens, // the Driver.shared value in the installed manifest ends up as a absolute // path to a folder, not a shared library. - shared_path_is_subdir, err := IsImmediateSubDir(cfg.Location, sharedPath) + shared_path_is_subdir, err := isImmediateSubDir(cfg.Location, sharedPath) if err != nil { return fmt.Errorf("error removing driver %s: %w", info.ID, err) } @@ -402,7 +402,7 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { } // Determine whether target is an immediate subdirectory of base. -func IsImmediateSubDir(base string, target string) (bool, error) { +func isImmediateSubDir(base string, target string) (bool, error) { base_abs, err := filepath.Abs(base) if err != nil { return false, err diff --git a/config/config_test.go b/config/config_test.go index ffeb98de..7c8033dd 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -143,7 +143,7 @@ func TestIsImmediateSubDir(t *testing.T) { for _, tt := range tests { t.Run(fmt.Sprintf("%s vs %s", tt.parent, tt.child), func(t *testing.T) { - result, err := IsImmediateSubDir(tt.parent, tt.child) + result, err := isImmediateSubDir(tt.parent, tt.child) if tt.expectErr { assert.Error(t, err) From 4ea8c5cb3714f0fea38ba12e38df7cf4342c4045 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Fri, 19 Sep 2025 11:06:16 -0700 Subject: [PATCH 08/10] Reword comment --- config/config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/config.go b/config/config.go index 762c71dc..72c17105 100644 --- a/config/config.go +++ b/config/config.go @@ -341,10 +341,10 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { // └── libadbc_driver_linux_amd64_v1.0.0 // └── libadbc_driver_linux_amd64_v1.0.0.so // - // If the MANIFEST we're installed has Driver.shared already set, this is - // invalid becuase it should have Files set instead. When this happens, - // the Driver.shared value in the installed manifest ends up as a absolute - // path to a folder, not a shared library. + // But if we happen to have installed a MANIFEST where the author + // mistakenly used Driver.shared instead of Files to point to the shared + // library path, the Driver.shared value in the installed manifest ends up + // as an absolute path to a folder, not a shared library. shared_path_is_subdir, err := isImmediateSubDir(cfg.Location, sharedPath) if err != nil { return fmt.Errorf("error removing driver %s: %w", info.ID, err) From e449b63a65b66eba9eb05ad0e2d449535468be12 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Fri, 19 Sep 2025 11:06:56 -0700 Subject: [PATCH 09/10] Move variable down --- config/config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 72c17105..d3f653b2 100644 --- a/config/config.go +++ b/config/config.go @@ -331,8 +331,6 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { // dbc installs drivers in a folder, other tools may not so we handle each // differently. if info.Source == "dbc" { - shared_dir := filepath.Dir(sharedPath) - // Handle an edge case first. We've already verified that Driver.shared is // an absolute path inside cfg.Location but we also expect the shared // library to be in a subfolder, e.g., @@ -359,6 +357,7 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { continue } + shared_dir := filepath.Dir(sharedPath) // The rest of this is "normal" operation... if err := os.RemoveAll(shared_dir); err != nil { // Ignore only when not found. This supports manifest-only drivers. From cb2e9e7eaae0bb57df584f37d71325ecedb15de0 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Fri, 19 Sep 2025 12:16:37 -0700 Subject: [PATCH 10/10] simplify approach --- cmd/dbc/uninstall_test.go | 7 ++- config/config.go | 64 +++------------------- config/config_test.go | 112 -------------------------------------- 3 files changed, 11 insertions(+), 172 deletions(-) diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index 99a728eb..becb8974 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -194,8 +194,9 @@ func (suite *SubcommandTestSuite) TestUninstallInvalidManifest() { // Ensure we don't nuke the tempdir which is the original (major) issue suite.DirExists(suite.tempdir) - // Ensure we removed the files from the package though + // We do remove the manifest suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest.toml")) - suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "test-driver-invalid-manifest.so")) - suite.NoDirExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest")) + // But we don't remove the driver shared folder in this edge case, so we assert + // they're still around + suite.FileExists(filepath.Join(suite.tempdir, "test-driver-invalid-manifest", "libadbc_driver_invalid_manifest.so")) } diff --git a/config/config.go b/config/config.go index d3f653b2..6f54351e 100644 --- a/config/config.go +++ b/config/config.go @@ -331,35 +331,16 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { // dbc installs drivers in a folder, other tools may not so we handle each // differently. if info.Source == "dbc" { - // Handle an edge case first. We've already verified that Driver.shared is - // an absolute path inside cfg.Location but we also expect the shared - // library to be in a subfolder, e.g., - // - // ${cfg.Location} - // └── libadbc_driver_linux_amd64_v1.0.0 - // └── libadbc_driver_linux_amd64_v1.0.0.so - // - // But if we happen to have installed a MANIFEST where the author - // mistakenly used Driver.shared instead of Files to point to the shared - // library path, the Driver.shared value in the installed manifest ends up - // as an absolute path to a folder, not a shared library. - shared_path_is_subdir, err := isImmediateSubDir(cfg.Location, sharedPath) - if err != nil { - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } - if shared_path_is_subdir { - if err := os.RemoveAll(sharedPath); err != nil { - if errors.Is(err, fs.ErrNotExist) { - continue - } - return fmt.Errorf("error removing driver %s: %w", info.ID, err) - } + sharedDir := filepath.Dir(sharedPath) + // Edge case when manifest is ill-formed: if sharedPath is set to the + // folder containing the shared library instead of the shared library + // itself, sharedDir is cfg.Location and we definitely don't want to + // remove that + if sharedDir == cfg.Location { continue } - shared_dir := filepath.Dir(sharedPath) - // The rest of this is "normal" operation... - if err := os.RemoveAll(shared_dir); err != nil { + if err := os.RemoveAll(sharedDir); err != nil { // Ignore only when not found. This supports manifest-only drivers. // TODO: Come up with a better mechanism to handle manifest-only drivers // and remove this continue when we do @@ -399,34 +380,3 @@ func UninstallDriverShared(cfg Config, info DriverInfo) error { return nil } - -// Determine whether target is an immediate subdirectory of base. -func isImmediateSubDir(base string, target string) (bool, error) { - base_abs, err := filepath.Abs(base) - if err != nil { - return false, err - } - target_abs, err := filepath.Abs(target) - if err != nil { - return false, err - } - - result, err := filepath.Rel(base_abs, target_abs) - if err != nil { - return false, err - } - - parts := strings.Split(result, string(filepath.Separator)) - - // return false if we're not a direct child - if len(parts) != 1 { - return false, nil - } - - // return false if we're above - if strings.HasPrefix(result, ".") { - return false, nil - } - - return true, nil -} diff --git a/config/config_test.go b/config/config_test.go index 7c8033dd..22f2d919 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -3,7 +3,6 @@ package config import ( - "fmt" "os" "path/filepath" "testing" @@ -46,114 +45,3 @@ func TestConfigEnvVarHierarchy(t *testing.T) { cfg = loadConfig(ConfigEnv) assert.Equal(t, "", cfg.Location) } - -func TestIsImmediateSubDir(t *testing.T) { - tests := []struct { - parent string - child string - expected bool - expectErr bool - }{ - { - parent: "/foo", - child: "/foo/bar", - expected: true, - }, - { - parent: "/foo/bar", - child: "/foo/bar", - expected: false, - }, - { - parent: "/foo", - child: "/foo/bar/baz", - expected: false, - }, - { - parent: "/foo/bar", - child: "/foo/baz", - expected: false, - }, - { - parent: "/foo", - child: "/bar", - expected: false, - }, - { - parent: "/foo/bar", - child: "/foo/bar/../baz", - expected: false, - }, - { - parent: "/foo", - child: "/foo/bar/", - expected: true, - }, - { - parent: "/foo/", - child: "/foo/bar", - expected: true, - }, - { - parent: "/foo/", - child: "/foo/bar/", - expected: true, - }, - { - parent: "/", - child: "/foo", - expected: true, - }, - { - parent: "", - child: "/foo", - expected: false, - }, - { - parent: "/foo", - child: "", - expected: false, - }, - { - parent: "", - child: "", - expected: false, - }, - { - parent: "foo", - child: "foo/bar", - expected: true, - }, - { - parent: "/foo", - child: "/foo/./bar", - expected: true, - }, - { - parent: "/foo", - child: "/foo/bar/../baz", - expected: true, - }, - { - parent: "/foo", - child: "/foo/bar/some_file.txt", - expected: false, - }, - } - - for _, tt := range tests { - t.Run(fmt.Sprintf("%s vs %s", tt.parent, tt.child), func(t *testing.T) { - result, err := isImmediateSubDir(tt.parent, tt.child) - - if tt.expectErr { - assert.Error(t, err) - return - } - - assert.NoError(t, err) - assert.Equal(t, tt.expected, result, - "IsImmediateSubDir(%q, %q) = %v, expected %v", - tt.parent, tt.child, result, tt.expected) - }) - } -}