From 955ea61e2f106bad69b428730165d00ef9a45c6c Mon Sep 17 00:00:00 2001 From: ubergesundheit Date: Tue, 16 Feb 2021 21:35:00 +0100 Subject: [PATCH 1/2] Make archive validation error messages friendlier --- arduino/resources/checksums.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/arduino/resources/checksums.go b/arduino/resources/checksums.go index ce9d13fe92b..f96e17658a4 100644 --- a/arduino/resources/checksums.go +++ b/arduino/resources/checksums.go @@ -34,6 +34,9 @@ import ( // TestLocalArchiveChecksum test if the checksum of the local archive match the checksum of the DownloadResource func (r *DownloadResource) TestLocalArchiveChecksum(downloadDir *paths.Path) (bool, error) { + if r.Checksum == "" { + return false, fmt.Errorf("missing checksum for: %s", r.ArchiveFileName) + } split := strings.SplitN(r.Checksum, ":", 2) if len(split) != 2 { return false, fmt.Errorf("invalid checksum format: %s", r.Checksum) @@ -69,7 +72,12 @@ func (r *DownloadResource) TestLocalArchiveChecksum(downloadDir *paths.Path) (bo if _, err := io.Copy(algo, file); err != nil { return false, fmt.Errorf("computing hash: %s", err) } - return bytes.Compare(algo.Sum(nil), digest) == 0, nil + + if bytes.Compare(algo.Sum(nil), digest) != 0 { + return false, fmt.Errorf("archive hash differs from hash in index") + } + + return true, nil } // TestLocalArchiveSize test if the local archive size match the DownloadResource size @@ -82,7 +90,11 @@ func (r *DownloadResource) TestLocalArchiveSize(downloadDir *paths.Path) (bool, if err != nil { return false, fmt.Errorf("getting archive info: %s", err) } - return info.Size() == r.Size, nil + if info.Size() != r.Size { + return false, fmt.Errorf("fetched archive size differs from size specified in index") + } + + return true, nil } // TestLocalArchiveIntegrity checks for integrity of the local archive. @@ -94,7 +106,7 @@ func (r *DownloadResource) TestLocalArchiveIntegrity(downloadDir *paths.Path) (b } if ok, err := r.TestLocalArchiveSize(downloadDir); err != nil { - return false, fmt.Errorf("teting archive size: %s", err) + return false, fmt.Errorf("testing archive size: %s", err) } else if !ok { return false, nil } @@ -163,5 +175,9 @@ func CheckDirChecksum(root string) (bool, error) { if err != nil { return false, err } - return file.Checksum == checksum, nil + if file.Checksum != checksum { + return false, fmt.Errorf("Checksum differs from checksum in package.json") + } + + return true, nil } From 687428c5a6b4d82b254c51edf2ac5b488ec29668 Mon Sep 17 00:00:00 2001 From: ubergesundheit Date: Wed, 17 Feb 2021 19:25:06 +0100 Subject: [PATCH 2/2] change behaviour a little + fix tests --- arduino/resources/helpers.go | 25 +++++++++++-------------- arduino/resources/resources_test.go | 29 +++++++++++++++-------------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/arduino/resources/helpers.go b/arduino/resources/helpers.go index c735ff36559..3b7937849ba 100644 --- a/arduino/resources/helpers.go +++ b/arduino/resources/helpers.go @@ -44,26 +44,23 @@ func (r *DownloadResource) IsCached(downloadDir *paths.Path) (bool, error) { // Download a DownloadResource. func (r *DownloadResource) Download(downloadDir *paths.Path, config *downloader.Config) (*downloader.Downloader, error) { - cached, err := r.TestLocalArchiveIntegrity(downloadDir) - if err != nil { - return nil, fmt.Errorf("testing local archive integrity: %s", err) - } - if cached { - // File is cached, nothing to do here - return nil, nil - } - path, err := r.ArchivePath(downloadDir) if err != nil { return nil, fmt.Errorf("getting archive path: %s", err) } - if stats, err := path.Stat(); os.IsNotExist(err) { + if _, err := path.Stat(); os.IsNotExist(err) { // normal download - } else if err == nil && stats.Size() > r.Size { - // file is bigger than expected, retry download... - if err := path.Remove(); err != nil { - return nil, fmt.Errorf("removing corrupted archive file: %s", err) + } else if err == nil { + // check local file integrity + ok, err := r.TestLocalArchiveIntegrity(downloadDir) + if err != nil || !ok { + if err := path.Remove(); err != nil { + return nil, fmt.Errorf("removing corrupted archive file: %s", err) + } + } else { + // File is cached, nothing to do here + return nil, nil } } else if err == nil { // resume download diff --git a/arduino/resources/resources_test.go b/arduino/resources/resources_test.go index 326f34c11f3..fc22e0ecaa8 100644 --- a/arduino/resources/resources_test.go +++ b/arduino/resources/resources_test.go @@ -26,19 +26,21 @@ import ( ) func TestDownloadAndChecksums(t *testing.T) { + testFileName := "core.zip" tmp, err := paths.MkTempDir("", "") require.NoError(t, err) defer tmp.RemoveAll() - testFile := tmp.Join("cache", "index.html") + testFile := tmp.Join("cache", testFileName) + // taken from test/testdata/test_index.json r := &DownloadResource{ - ArchiveFileName: "index.html", + ArchiveFileName: testFileName, CachePath: "cache", - Checksum: "SHA-256:e021e1a223d03069d5f08dea25a58ca445a7376d9bdf980f756034f118449e66", - Size: 1119, - URL: "https://downloads.arduino.cc/index.html", + Checksum: "SHA-256:6a338cf4d6d501176a2d352c87a8d72ac7488b8c5b82cdf2a4e2cef630391092", + Size: 486, + URL: "https://raw.githubusercontent.com/arduino/arduino-cli/master/test/testdata/core.zip", } - digest, err := hex.DecodeString("e021e1a223d03069d5f08dea25a58ca445a7376d9bdf980f756034f118449e66") + digest, err := hex.DecodeString("6a338cf4d6d501176a2d352c87a8d72ac7488b8c5b82cdf2a4e2cef630391092") require.NoError(t, err) downloadAndTestChecksum := func() { @@ -73,10 +75,14 @@ func TestDownloadAndChecksums(t *testing.T) { // Download if cached file has less data (resume) data, err = testFile.ReadFile() require.NoError(t, err) - err = testFile.WriteFile(data[:1000]) + err = testFile.WriteFile(data[:100]) require.NoError(t, err) downloadAndTestChecksum() + r.Checksum = "" + _, err = r.TestLocalArchiveChecksum(tmp) + require.Error(t, err) + r.Checksum = "BOH:12312312312313123123123123123123" _, err = r.TestLocalArchiveChecksum(tmp) require.Error(t, err) @@ -89,21 +95,16 @@ func TestDownloadAndChecksums(t *testing.T) { _, err = r.TestLocalArchiveChecksum(tmp) require.Error(t, err) - r.Checksum = "SHA-1:c007e47637cc6ad6176e7d94aeffc232ee34c1c1" + r.Checksum = "SHA-1:16e1495aff482f2650733e1661d5f7c69040ec13" res, err := r.TestLocalArchiveChecksum(tmp) require.NoError(t, err) require.True(t, res) - r.Checksum = "MD5:2e388576eefd92a15967868d5f566f29" + r.Checksum = "MD5:3bcc3aab6842ff124df6a5cfba678ca1" res, err = r.TestLocalArchiveChecksum(tmp) require.NoError(t, err) require.True(t, res) - r.Checksum = "MD5:12312312312312312312313131231231" - res, err = r.TestLocalArchiveChecksum(tmp) - require.NoError(t, err) - require.False(t, res) - _, err = r.TestLocalArchiveChecksum(paths.New("/not-existent")) require.Error(t, err)