From d6d4eb208738128665265bbb07f0cf5f7b3cc15d Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 28 Jun 2019 15:29:39 -0700 Subject: [PATCH] fix some tests and some config behavior to prevent null dereference errors and incorrect precedence between iso checksum and iso checksum url --- common/iso_config.go | 25 ++++++++++++++++--------- common/iso_config_test.go | 9 +++++++-- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/common/iso_config.go b/common/iso_config.go index 47bfa94e3..1a71aa388 100644 --- a/common/iso_config.go +++ b/common/iso_config.go @@ -58,13 +58,19 @@ func (c *ISOConfig) Prepare(ctx *interpolate.Context) (warnings []string, errs [ } if c.ISOChecksumURL != "" { - if strings.HasSuffix(strings.ToLower(c.ISOChecksumURL), ".iso") { - errs = append(errs, fmt.Errorf("Error parsing checksum:"+ - " .iso is not a valid checksum extension")) + if c.ISOChecksum != "" { + warnings = append(warnings, "You have provided both an "+ + "iso_checksum and an iso_checksum_url. Discarding the "+ + "iso_checksum_url and using the checksum.") + } else { + if strings.HasSuffix(strings.ToLower(c.ISOChecksumURL), ".iso") { + errs = append(errs, fmt.Errorf("Error parsing checksum:"+ + " .iso is not a valid checksum extension")) + } + // go-getter auto-parses checksum files + c.ISOChecksumType = "file" + c.ISOChecksum = c.ISOChecksumURL } - // go-getter auto-parses checksum files - c.ISOChecksumType = "file" - c.ISOChecksum = c.ISOChecksumURL } if c.ISOChecksum == "" { @@ -86,11 +92,12 @@ func (c *ISOConfig) Prepare(ctx *interpolate.Context) (warnings []string, errs [ Getters: getter.Getters, } cksum, err := gc.ChecksumFromFile(c.ISOChecksumURL, u) - if err != nil { + if cksum == nil || err != nil { errs = append(errs, fmt.Errorf("Couldn't extract checksum from checksum file")) + } else { + c.ISOChecksumType = cksum.Type + c.ISOChecksum = hex.EncodeToString(cksum.Value) } - c.ISOChecksumType = cksum.Type - c.ISOChecksum = hex.EncodeToString(cksum.Value) } return warnings, errs diff --git a/common/iso_config_test.go b/common/iso_config_test.go index d8a236899..b04e0933f 100644 --- a/common/iso_config_test.go +++ b/common/iso_config_test.go @@ -75,11 +75,16 @@ func TestISOConfigPrepare_ISOChecksum(t *testing.T) { func TestISOConfigPrepare_ISOChecksumURLBad(t *testing.T) { i := testISOConfig() i.ISOChecksumURL = "file:///not_read" + i.ISOChecksum = "shouldoverride" // Test ISOChecksum overrides url warns, err := i.Prepare(nil) - if len(warns) > 0 && len(err) > 0 { - t.Fatalf("bad: %#v, %#v", warns, err) + if len(warns) != 1 { + t.Fatalf("Bad: should have warned because both checksum and " + + "checksumURL are set.") + } + if len(err) > 0 { + t.Fatalf("Bad; should have warned but not errored.") } // Test that we won't try to read an iso into memory because of a user