diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index a7e4b50c..eedbd4aa 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -15,7 +15,9 @@ package main import ( + "os" "path/filepath" + "strings" "github.com/columnar-tech/dbc/config" ) @@ -114,3 +116,34 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { " Available Versions:\n"+ " ╰── 1.0.0\n", suite.runCmd(m)) } + +func (suite *SubcommandTestSuite) TestSearchCmdWithMissingVersionInManifest() { + // Install a driver + m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. + GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + suite.runCmd(m) + + // Corrupt the manifest by removing the version key + manifestPath := filepath.Join(suite.tempdir, "test-driver-1.toml") + manifestData, err := os.ReadFile(manifestPath) + suite.Require().NoError(err, "should be able to read manifest file") + + // Remove the version line from the manifest + lines := []string{} + for _, line := range strings.Split(string(manifestData), "\n") { + if !strings.HasPrefix(line, "version =") { + lines = append(lines, line) + } + } + corruptedManifest := strings.Join(lines, "\n") + + err = os.WriteFile(manifestPath, []byte(corruptedManifest), 0644) + suite.Require().NoError(err, "should be able to write corrupted manifest") + + suite.Require().NotPanics(func() { + m = SearchCmd{}.GetModelCustom( + baseModel{getDriverRegistry: getTestDriverRegistry, + downloadPkg: downloadTestPkg}) + suite.runCmd(m) + }, "Search should not panic when manifest is missing version key") +} diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index e58b175b..3c87a5c5 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -40,6 +40,7 @@ func (suite *SubcommandTestSuite) TestUninstallManifestOnly() { } contents := `name = "Some Found Driver" +version = "1.0.0" # Doesn't matter what's in here @@ -60,6 +61,7 @@ func (suite *SubcommandTestSuite) TestUninstallDriverAndManifest() { pkgdir := path.Join(suite.tempdir, "somepath") os.Mkdir(pkgdir, 0o755) contents := `name = "Found Driver" +version = "1.0.0" # Doesn't matter what's in here diff --git a/config/config.go b/config/config.go index e9dded52..cc776241 100644 --- a/config/config.go +++ b/config/config.go @@ -35,6 +35,8 @@ const adbcEnvVar = "ADBC_DRIVER_PATH" var platformTuple string +var ErrInvalidManifest = errors.New("invalid manifest") + func init() { os := runtime.GOOS switch os { @@ -299,6 +301,14 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes di.ManifestVersion, currentManifestVersion) } + // Callers can assume these fields are set so return an error if they aren't + if di.Name == "" { + return Manifest{}, fmt.Errorf("%w: name is required", ErrInvalidManifest) + } + if di.Version == nil { + return Manifest{}, fmt.Errorf("%w: version is required", ErrInvalidManifest) + } + result := Manifest{ DriverInfo: DriverInfo{ ID: driverName, @@ -323,12 +333,12 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes if strVal, ok := v.(string); ok { result.Driver.Shared.platformMap[k] = strVal } else { - return Manifest{}, fmt.Errorf("invalid type for platform %s, expected string", k) + return Manifest{}, fmt.Errorf("%w: invalid type for platform %s, expected string", ErrInvalidManifest, k) } } default: if requireShared { - return Manifest{}, errors.New("invalid type for 'Driver.shared' in manifest, expected string or table") + return Manifest{}, fmt.Errorf("%w: invalid type for 'Driver.shared' in manifest, expected string or table", ErrInvalidManifest) } } diff --git a/config/driver_test.go b/config/driver_test.go index 0ea48c54..6c94ee9a 100644 --- a/config/driver_test.go +++ b/config/driver_test.go @@ -125,3 +125,37 @@ version = '0.1.0' _, err := loadDriverFromManifest(prefix, driverName) require.ErrorContains(t, err, "manifest version 100 is unsupported, only 1 and lower are supported by this version of dbc") } + +func TestLoadDriverFromInvalidManifest(t *testing.T) { + tests := []struct { + name string + manifest string + errContains string + }{ + { + name: "missing name", + manifest: `version = '1.0.0'`, + errContains: "name is required", + }, + { + name: "missing version", + manifest: `name = 'Test Driver'`, + errContains: "version is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prefix := t.TempDir() + driverName := "test_driver" + manifestPath := filepath.Join(prefix, driverName+".toml") + + require.NoError(t, os.WriteFile(manifestPath, []byte(tt.manifest), 0644)) + + _, err := loadDriverFromManifest(prefix, driverName) + require.ErrorIs(t, err, ErrInvalidManifest) + require.ErrorContains(t, err, tt.errContains) + require.ErrorContains(t, err, manifestPath) + }) + } +}