From daac8b959f6c3fd94ba3b54f99841380215023ed Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 28 Jun 2013 22:34:43 -0400 Subject: [PATCH] builder/common: Fix nil deref on race condition [GH-52] When verifying large files, the checksum would take a long time and downloader would be nil. When the goroutine asked for progress, it would raise a nil exception. --- builder/common/download.go | 10 ++++++++++ builder/virtualbox/step_download_iso.go | 5 ++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/builder/common/download.go b/builder/common/download.go index fead9d4d9..ff59faea4 100644 --- a/builder/common/download.go +++ b/builder/common/download.go @@ -7,6 +7,7 @@ import ( "fmt" "hash" "io" + "log" "net/http" "net/url" "os" @@ -73,6 +74,7 @@ func (d *DownloadClient) Cancel() { func (d *DownloadClient) Get() (string, error) { // If we already have the file and it matches, then just return the target path. if verify, _ := d.VerifyChecksum(d.config.TargetPath); verify { + log.Println("Initial checksum matched, no download needed.") return d.config.TargetPath, nil } @@ -81,6 +83,8 @@ func (d *DownloadClient) Get() (string, error) { return "", err } + log.Printf("Parsed URL: %#v", url) + // Files when we don't copy the file are special cased. var finalPath string if url.Scheme == "file" && !d.config.CopyFile { @@ -101,6 +105,7 @@ func (d *DownloadClient) Get() (string, error) { } defer f.Close() + log.Printf("Downloading: %s", url.String()) err = d.downloader.Download(f, url) } @@ -117,6 +122,10 @@ func (d *DownloadClient) Get() (string, error) { // PercentProgress returns the download progress as a percentage. func (d *DownloadClient) PercentProgress() uint { + if d.downloader == nil { + return 0 + } + return uint((float64(d.downloader.Progress()) / float64(d.downloader.Total())) * 100) } @@ -133,6 +142,7 @@ func (d *DownloadClient) VerifyChecksum(path string) (bool, error) { } defer f.Close() + log.Printf("Verifying checksum of %s", path) d.config.Hash.Reset() io.Copy(d.config.Hash, f) return bytes.Compare(d.config.Hash.Sum(nil), d.config.Checksum) == 0, nil diff --git a/builder/virtualbox/step_download_iso.go b/builder/virtualbox/step_download_iso.go index d4b67fa18..bcb75acf1 100644 --- a/builder/virtualbox/step_download_iso.go +++ b/builder/virtualbox/step_download_iso.go @@ -35,9 +35,7 @@ func (s *stepDownloadISO) Run(state map[string]interface{}) multistep.StepAction checksum, err := hex.DecodeString(config.ISOMD5) if err != nil { - err := fmt.Errorf("Error parsing checksum: %s", err) - state["error"] = err - ui.Error(err.Error()) + state["error"] = fmt.Errorf("Error parsing checksum: %s", err) return multistep.ActionHalt } @@ -111,6 +109,7 @@ DownloadWaitLoop: } defer sourceF.Close() + log.Printf("Copying ISO to temp location: %s", tempdir) if _, err := io.Copy(f, sourceF); err != nil { state["error"] = fmt.Errorf("Error copying ISO: %s", err) return multistep.ActionHalt