Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions cmd/dbc/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
package main

import (
"os"
"path/filepath"
"strings"

"github.com/columnar-tech/dbc/config"
)
Expand Down Expand Up @@ -114,3 +116,35 @@ 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")

// This should panic when trying to access the nil Version pointer
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")
}
2 changes: 2 additions & 0 deletions cmd/dbc/uninstall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func (suite *SubcommandTestSuite) TestUninstallManifestOnly() {
}

contents := `name = "Some Found Driver"
version = "1.0.0"

# Doesn't matter what's in here

Expand All @@ -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

Expand Down
14 changes: 12 additions & 2 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: %s", ErrInvalidManifest, "name is required")
Comment thread
amoeba marked this conversation as resolved.
Outdated
}
if di.Version == nil {
return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "version is required")
}

result := Manifest{
DriverInfo: DriverInfo{
ID: driverName,
Expand All @@ -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: %s", ErrInvalidManifest, "invalid type for 'Driver.shared' in manifest, expected string or table")
}
}

Expand Down
33 changes: 33 additions & 0 deletions config/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,36 @@ 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)
})
}
}
Loading