Skip to content

Commit c335cdb

Browse files
bgn42bgn42
committed
Many more checks in validate function and better error messages
1 parent 94717da commit c335cdb

File tree

2 files changed

+85
-78
lines changed

2 files changed

+85
-78
lines changed

‎cmd/errors/errors.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,18 @@ var (
3232
ErrBadPackVersion = errors.New("bad pack version: cannot add a version with build metadata")
3333

3434
// Errors related to package content
35-
ErrPdscFileNotFound = errors.New("pdsc not found")
36-
ErrPackNotInstalled = errors.New("pack not installed")
37-
ErrPdscEntryExists = errors.New("pdsc already in index")
38-
ErrPdscEntryNotFound = errors.New("pdsc not found in index")
39-
ErrEula = errors.New("user does not agree with the pack's license")
40-
ErrExtractEula = errors.New("user wants to extract embedded license only")
41-
ErrLicenseNotFound = errors.New("embedded license not found")
42-
ErrPackRootNotFound = errors.New("no CMSIS Pack Root directory specified. Either the environment CMSIS_PACK_ROOT needs to be set or the path specified using the command line option -R/--pack-root string")
43-
ErrPackRootDoesNotExist = errors.New("the specified CMSIS Pack Root directory does NOT exist! Please take a moment to review if the value is correct or create a new one via `cpackget init` command")
44-
ErrPdscFileTooDeepInPack = errors.New("pdsc file is too deep in pack file")
35+
ErrPdscFileNotFound = errors.New("pdsc not found")
36+
ErrPackNotInstalled = errors.New("pack not installed")
37+
ErrPdscEntryExists = errors.New("pdsc already in index")
38+
ErrPdscEntryNotFound = errors.New("pdsc not found in index")
39+
ErrEula = errors.New("user does not agree with the pack's license")
40+
ErrExtractEula = errors.New("user wants to extract embedded license only")
41+
ErrLicenseNotFound = errors.New("embedded license not found")
42+
ErrPackRootNotFound = errors.New("no CMSIS Pack Root directory specified. Either the environment CMSIS_PACK_ROOT needs to be set or the path specified using the command line option -R/--pack-root string")
43+
ErrPackRootDoesNotExist = errors.New("the specified CMSIS Pack Root directory does NOT exist! Please take a moment to review if the value is correct or create a new one via `cpackget init` command")
44+
ErrPdscFileTooDeepInPack = errors.New("pdsc file is too deep in pack file")
45+
ErrMultiplePdscFilesInPack = errors.New("multiple pdsc files found in pack file, cannot determine which one to use. Please remove the extra pdsc files")
46+
ErrPdscWrongName = errors.New("pdsc file has wrong name, it should be <PackID>.pdsc")
4547

4648
// Errors related to network
4749
ErrBadRequest = errors.New("bad request")

‎cmd/installer/pack.go‎

Lines changed: 73 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -214,105 +214,110 @@ func (p *PackType) validate() error {
214214
log.Debug("Validating pack")
215215
var err error
216216
myPdscFileName := p.PdscFileName()
217+
var validPdscFiles []*zip.File
218+
219+
// Single pass: validate all files and collect valid PDSC files
217220
for _, file := range p.zipReader.File {
218221
ext := strings.ToLower(filepath.Ext(file.Name))
222+
223+
// Ensure all file paths do not contain ".."
224+
if strings.Contains(file.Name, "..") {
225+
if ext == PdscExtension {
226+
log.Errorf("File %q invalid file path", file.Name)
227+
return errs.ErrInvalidFilePath
228+
} else {
229+
return errs.ErrInsecureZipFileName
230+
}
231+
}
232+
219233
if ext == PdscExtension {
220234
// Check if pack was compressed in a subfolder
221235
subfoldersCount := strings.Count(file.Name, "/") + strings.Count(file.Name, "\\")
222236
if subfoldersCount > 1 {
223-
err = errs.ErrPdscFileTooDeepInPack
224-
} else if subfoldersCount == 1 {
225-
p.Subfolder = filepath.Dir(file.Name)
237+
err = errs.ErrPdscFileTooDeepInPack // save for later decision if error or warning
226238
} else {
227-
p.Subfolder = "/" // only for marking that there is a .pdsc file in the root of the pack
228-
}
239+
if !strings.EqualFold(file.Name, myPdscFileName) {
240+
if err != nil {
241+
log.Warnf("Pack %q contains an additional .pdsc file in a deeper subfolder, this may cause issues", p.path)
242+
}
243+
return fmt.Errorf("%q: %w", file.Name, errs.ErrPdscWrongName)
244+
}
245+
// Collect valid PDSC files for processing
246+
validPdscFiles = append(validPdscFiles, file)
229247

230-
// Ensure the file path does not contain ".."
231-
if strings.Contains(file.Name, "..") {
232-
log.Errorf("File %q invalid file path", file.Name)
233-
return errs.ErrInvalidFilePath
234-
}
235-
} else {
236-
if strings.Contains(file.Name, "..") {
237-
return errs.ErrInsecureZipFileName
248+
if subfoldersCount == 1 {
249+
p.Subfolder = filepath.Dir(file.Name)
250+
}
238251
}
239252
}
240253
}
254+
241255
if err != nil {
242-
if len(p.Subfolder) != 0 {
243-
log.Warnf("Pack %q contains multiple .pdsc files in different levels, this may cause issues", p.path)
256+
if len(validPdscFiles) > 0 {
257+
if len(validPdscFiles) > 1 {
258+
return errs.ErrMultiplePdscFilesInPack
259+
}
260+
log.Warnf("Pack %q contains an additional .pdsc file in a deeper subfolder, this may cause issues", p.path)
244261
} else {
245262
return err
246263
}
247264
}
248-
p.Subfolder = "" // reset subfolder to empty string
249-
for _, file := range p.zipReader.File {
250-
ext := strings.ToLower(filepath.Ext(file.Name))
251-
if ext == PdscExtension {
252-
// Check if pack was compressed in a subfolder
253-
subfoldersCount := strings.Count(file.Name, "/") + strings.Count(file.Name, "\\")
254-
if subfoldersCount > 1 {
255-
continue
256-
} else if subfoldersCount == 1 {
257-
p.Subfolder = filepath.Dir(file.Name)
258-
}
259265

260-
// Ensure the file path does not contain ".."
261-
if strings.Contains(file.Name, "..") {
262-
log.Errorf("File %q invalid file path", file.Name)
263-
return errs.ErrInvalidFilePath
264-
}
266+
if len(validPdscFiles) > 1 {
267+
return errs.ErrMultiplePdscFilesInPack
268+
}
265269

266-
myPdscFileName = p.PackID() + filepath.Ext(file.Name)
270+
if len(validPdscFiles) == 0 {
271+
log.Errorf("%q not found in %q", myPdscFileName, p.path)
272+
return errs.ErrPdscFileNotFound
273+
}
267274

268-
// Read pack's pdsc
269-
tmpPdscFileName := filepath.Join(os.TempDir(), utils.RandStringBytes(10))
270-
defer os.RemoveAll(tmpPdscFileName)
275+
// Process the first valid PDSC file found
276+
file := validPdscFiles[0]
271277

272-
if err := utils.SecureInflateFile(file, tmpPdscFileName, ""); err != nil {
273-
return err
274-
}
275-
276-
p.Pdsc = xml.NewPdscXML(filepath.Join(tmpPdscFileName, file.Name)) // #nosec
277-
if err := p.Pdsc.Read(); err != nil {
278-
return err
279-
}
278+
// Read pack's pdsc
279+
tmpPdscFileName := filepath.Join(os.TempDir(), utils.RandStringBytes(10))
280+
defer os.RemoveAll(tmpPdscFileName)
280281

281-
// Sanity check: make sure the version being installed actually exists in the PDSC file
282-
version := p.GetVersion()
283-
latestVersion := p.Pdsc.LatestVersion()
282+
if err := utils.SecureInflateFile(file, tmpPdscFileName, ""); err != nil {
283+
return err
284+
}
284285

285-
log.Debugf("Making sure %s is the latest release in %s", version, myPdscFileName)
286+
p.Pdsc = xml.NewPdscXML(filepath.Join(tmpPdscFileName, file.Name)) // #nosec
287+
if err := p.Pdsc.Read(); err != nil {
288+
return err
289+
}
286290

287-
if utils.SemverCompare(version, latestVersion) != 0 {
288-
releaseTag := p.Pdsc.FindReleaseTagByVersion(version)
289-
if releaseTag == nil {
290-
log.Errorf("The pack's pdsc (%s) has no release tag matching version %q", myPdscFileName, version)
291-
return errs.ErrPackVersionNotFoundInPdsc
292-
}
291+
// Sanity check: make sure the version being installed actually exists in the PDSC file
292+
version := p.GetVersion()
293+
latestVersion := p.Pdsc.LatestVersion()
293294

294-
log.Errorf("The latest release (%s) in pack's pdsc (%s) does not match pack version %q", latestVersion, myPdscFileName, version)
295-
return errs.ErrPackVersionNotLatestReleasePdsc
296-
}
295+
log.Debugf("Making sure %s is the latest release in %s", version, myPdscFileName)
297296

298-
p.Pdsc.FileName = file.Name
297+
if utils.SemverCompare(version, latestVersion) != 0 {
298+
releaseTag := p.Pdsc.FindReleaseTagByVersion(version)
299+
if releaseTag == nil {
300+
log.Errorf("The pack's pdsc (%s) has no release tag matching version %q", myPdscFileName, version)
301+
return errs.ErrPackVersionNotFoundInPdsc
302+
}
299303

300-
pdscFileName := p.PdscFileName() // destination file name in .Download directory
301-
pdscFilePath := filepath.Join(tmpPdscFileName, file.Name) // #nosec
302-
newPdscFileName := p.PdscFileNameWithVersion()
304+
log.Errorf("The latest release (%s) in pack's pdsc (%s) does not match pack version %q", latestVersion, myPdscFileName, version)
305+
return errs.ErrPackVersionNotLatestReleasePdsc
306+
}
303307

304-
if !p.IsPublic {
305-
_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.LocalDir, pdscFileName))
306-
}
308+
p.Pdsc.FileName = file.Name
307309

308-
_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.DownloadDir, newPdscFileName))
310+
pdscFileName := p.PdscFileName() // destination file name in .Download directory
311+
pdscFilePath := filepath.Join(tmpPdscFileName, file.Name) // #nosec
312+
newPdscFileName := p.PdscFileNameWithVersion()
309313

310-
return nil
311-
}
314+
if !p.IsPublic {
315+
_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.LocalDir, pdscFileName))
312316
}
313317

314-
log.Errorf("%q not found in %q", myPdscFileName, p.path)
315-
return errs.ErrPdscFileNotFound
318+
_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.DownloadDir, newPdscFileName))
319+
320+
return nil
316321
}
317322

318323
// purge removes all cached files matching the pattern derived from the PackType's

0 commit comments

Comments
 (0)