diff --git a/cmd/installer/root.go b/cmd/installer/root.go index 44b55273..cd5e1cbe 100644 --- a/cmd/installer/root.go +++ b/cmd/installer/root.go @@ -37,6 +37,15 @@ const ConnectionTryURL = "https://www.keil.com/pack/keil.vidx" // DefaultPublicIndex is the public index to use in "default mode" const DefaultPublicIndex = KeilDefaultPackRoot + PublicIndexName + +func resolvePublicIndexPath(indexPath string) string { + indexPath = strings.TrimSuffix(indexPath, "/") + if strings.HasSuffix(indexPath, PublicIndexName) { + return indexPath + } + return indexPath + "/" + PublicIndexName +} + const DefaultPublicCacheIndex = KeilDefaultPackRoot + PublicCacheIndex // would be reset to the public index URL when reading the public index @@ -1001,10 +1010,13 @@ func UpdatePublicIndexIfOnline() error { func UpdatePublicIndex(indexPath string, sparse, downloadPdsc, downloadRemainingPdscFiles, skipDeprecatedPdscFiles, updatePrivatePdsc, showInfo, insecureSkipVerify bool, concurrency int, timeout int) error { // For backwards compatibility, allow indexPath to be a file, but ideally it should be empty if indexPath == "" { - indexPath = strings.TrimSuffix(Installation.PublicIndexXML.URL, "/") + "/" + PublicIndexName + indexPath = resolvePublicIndexPath(Installation.PublicIndexXML.URL) } - var err error + indexPath, err := utils.FileURLToPath(indexPath) + if err != nil { + return err + } if strings.HasPrefix(indexPath, "http://") || strings.HasPrefix(indexPath, "https://") { if !strings.HasPrefix(indexPath, "https://127.0.0.1") { @@ -1737,7 +1749,7 @@ func ReadIndexFiles() error { return err } if Installation.PublicIndexXML.URL != "" { - ActualPublicIndex = Installation.PublicIndexXML.URL + PublicIndexName + ActualPublicIndex = resolvePublicIndexPath(Installation.PublicIndexXML.URL) } err = Installation.LocalPidx.Read() diff --git a/cmd/installer/root_test.go b/cmd/installer/root_test.go index cf751134..556fd0eb 100644 --- a/cmd/installer/root_test.go +++ b/cmd/installer/root_test.go @@ -729,6 +729,24 @@ func TestUpdatePublicIndexIfOnline(t *testing.T) { // Should handle gracefully with warning (WarningInsteadOfErrors is true) assert.Nil(err) }) + + t.Run("test invalid index source warns instead of panicking", func(t *testing.T) { + localTestingDir := "test-update-invalid-index-source" + assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot)) + installer.UnlockPackRoot() + defer removePackRoot(localTestingDir) + + installer.ActualPublicIndex = "invalid\x00index.pidx" + var output bytes.Buffer + originalOutput := log.StandardLogger().Out + log.SetOutput(&output) + defer log.SetOutput(originalOutput) + + assert.NotPanics(func() { + assert.Nil(installer.UpdatePublicIndexIfOnline()) + }) + assert.Contains(output.String(), "Cannot update public index") + }) } func TestUpdatePublicIndex(t *testing.T) { @@ -975,6 +993,28 @@ func TestUpdatePublicIndex(t *testing.T) { assert.Equal(copied, indexContent) }) + t.Run("test configured source already ends with "+installer.PublicIndexName, func(t *testing.T) { + localTestingDir := "test-index-source-with-filename" + assert.Nil(installer.SetPackRoot(localTestingDir, CreatePackRoot)) + installer.UnlockPackRoot() + defer removePackRoot(localTestingDir) + + sourceDir := t.TempDir() + sourceIndex := filepath.Join(sourceDir, installer.PublicIndexName) + sourceURL := "file://" + filepath.ToSlash(sourceIndex) + indexContent, err := os.ReadFile(samplePublicIndex) + assert.Nil(err) + indexContent = []byte(strings.Replace(string(indexContent), "http://the.vendor/", sourceURL, 1)) + assert.Nil(os.WriteFile(sourceIndex, indexContent, 0600)) // #nosec G703 -- sourceIndex is confined to t.TempDir. + installer.Installation.PublicIndexXML.URL = sourceURL + + err = installer.UpdatePublicIndex("", Sparse, DownloadPdsc, !DownloadRemainingPdscFiles, skipDeprecatedPdscFiles, !UpdatePrivatePdsc, ShowInfo, !InsecureSkipVerify, Concurrency, Timeout) + assert.Nil(err) + assert.True(utils.FileExists(installer.Installation.PublicIndex)) + assert.Nil(installer.ReadIndexFiles()) + assert.Equal(sourceURL, installer.ActualPublicIndex) + }) + t.Run("test check concurrency function call", func(t *testing.T) { assert.Equal(0, installer.CheckConcurrency(0)) assert.Equal(2, installer.CheckConcurrency(2)) diff --git a/cmd/utils/utils.go b/cmd/utils/utils.go index af592577..17134f07 100644 --- a/cmd/utils/utils.go +++ b/cmd/utils/utils.go @@ -339,7 +339,7 @@ func FileExists(filePath string) bool { // DirExists checks if dirPath is an actual directory in the local file system func DirExists(dirPath string) bool { info, err := os.Stat(dirPath) - if os.IsNotExist(err) { + if err != nil { return false } return info.IsDir() diff --git a/cmd/utils/utils_test.go b/cmd/utils/utils_test.go index 8699bf50..c3c73b01 100644 --- a/cmd/utils/utils_test.go +++ b/cmd/utils/utils_test.go @@ -232,6 +232,14 @@ func TestFileExists(t *testing.T) { }) } +func TestDirExists(t *testing.T) { + assert := assert.New(t) + + assert.NotPanics(func() { + assert.False(utils.DirExists("invalid\x00path")) + }) +} + func TestEnsureDir(t *testing.T) { assert := assert.New(t) t.Run("test if directory gets created", func(t *testing.T) { diff --git a/cmd/xml/pidx.go b/cmd/xml/pidx.go index 2ca929a9..56343dfe 100644 --- a/cmd/xml/pidx.go +++ b/cmd/xml/pidx.go @@ -5,6 +5,7 @@ package xml import ( "encoding/xml" + "net/url" "path" "path/filepath" "strings" @@ -445,6 +446,11 @@ func (p *PidxXML) Read() error { } for _, pdsc := range p.Pindex.Pdscs { + invalidFields := pdsc.invalidFields() + if len(invalidFields) > 0 { + log.Warnf("Skipping invalid pdsc entry %q: invalid %s", pdsc.YamlPackID(), strings.Join(invalidFields, ", ")) + continue + } pdsc.Version = utils.SemverStripMeta(pdsc.Version) pdsc.computeIsDeprecated(p.deprecatedDate) key := pdsc.Key() @@ -460,6 +466,36 @@ func (p *PidxXML) Read() error { return nil } +func (p *PdscTag) invalidFields() []string { + invalidFields := []string{} + if p.URL != "" { + if _, err := url.Parse(p.URL); err != nil { + invalidFields = append(invalidFields, "url") + } + } + if !utils.IsPackVendorNameValid(p.Vendor) { + invalidFields = append(invalidFields, "vendor") + } + if !utils.IsPackNameValid(p.Name) { + invalidFields = append(invalidFields, "name") + } + if p.Version != "" && !utils.IsPackVersionValid(p.Version) { + invalidFields = append(invalidFields, "version") + } + if p.Deprecated != "" { + if _, err := time.Parse("2006-01-02", p.Deprecated); err != nil { + invalidFields = append(invalidFields, "deprecated") + } + } + if p.Replacement != "" { + parts := strings.Split(p.Replacement, ".") + if len(parts) > 2 || !utils.IsPackNameValid(parts[len(parts)-1]) || (len(parts) == 2 && !utils.IsPackVendorNameValid(parts[0])) { + invalidFields = append(invalidFields, "replacement") + } + } + return invalidFields +} + // Write writes the PidxXML data to the file specified by p.fileName. // It appends the pdsc tags from p.pdscList to p.Pindex.Pdscs before writing, // and truncates p.Pindex.Pdscs after writing to prepare for potential future writes. diff --git a/cmd/xml/pidx_test.go b/cmd/xml/pidx_test.go index 53ba630d..6c5f8a40 100644 --- a/cmd/xml/pidx_test.go +++ b/cmd/xml/pidx_test.go @@ -4,8 +4,10 @@ package xml_test import ( + "bytes" "io" "os" + "path/filepath" "strings" "testing" "time" @@ -13,6 +15,7 @@ import ( errs "github.com/open-cmsis-pack/cpackget/cmd/errors" "github.com/open-cmsis-pack/cpackget/cmd/utils" "github.com/open-cmsis-pack/cpackget/cmd/xml" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -411,6 +414,38 @@ func TestPidxXML(t *testing.T) { assert.Contains(err.Error(), fileName) }) + t.Run("test reading PIDX skips invalid entries", func(t *testing.T) { + fileName := filepath.Join(t.TempDir(), "index.pidx") + content := ` + + TestVendor + https://example.com/ + 2026-09-14T00:00:00Z + + + + + +` + assert.NoError(os.WriteFile(fileName, []byte(content), 0600)) + + var output bytes.Buffer + originalOutput := log.StandardLogger().Out + log.SetOutput(&output) + defer log.SetOutput(originalOutput) + + pidx := xml.NewPidxXML(fileName, false) + assert.NoError(pidx.Read()) + assert.Len(pidx.ListPdscTags(), 2) + assert.Len(pidx.FindPdscTags(xml.PdscTag{Vendor: "TheVendor", Name: "PackOne", Version: "1.2.3"}), 1) + assert.Len(pidx.FindPdscTags(xml.PdscTag{Vendor: "TheVendor", Name: "PackTwo", Version: "2.0.0"}), 1) + assert.Empty(pidx.FindPdscTags(xml.PdscTag{Vendor: "Bad Vendor", Name: "Bad.Pack", Version: "invalid"})) + assert.Contains(output.String(), "Skipping invalid pdsc entry") + for _, field := range []string{"url", "vendor", "name", "version", "deprecated", "replacement"} { + assert.Contains(output.String(), field) + } + }) + t.Run("test Read wraps XML parsing errors with filename", func(t *testing.T) { fileName := "../../testdata/MalformedPack.pidx" pidx := xml.NewPidxXML(fileName, false)