Allow downloading index and signature in a single tar.bz2 archive (#1734)

* Allow downloading index and signature in a tar.bz2 archive

* Make some configurations resilient to empty settings

* Added tests

* fix comment

* Apply suggestions from code review

Co-authored-by: Umberto Baldi <[email protected]>

* Allow .tar.bz2 package index URL

* Fix errors in timeout checks

* Fixed check for stale package_index.json files

Co-authored-by: Umberto Baldi <[email protected]>

Author: cmaglie

Diff for: arduino/cores/packagemanager/package_manager.go

@@ -237,7 +237,11 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) (
 
 // LoadPackageIndex loads a package index by looking up the local cached file from the specified URL
 func (pm *PackageManager) LoadPackageIndex(URL *url.URL) error {
-	indexPath := pm.IndexDir.Join(path.Base(URL.Path))
+	indexFileName := path.Base(URL.Path)
+	if strings.HasSuffix(indexFileName, ".tar.bz2") {
+		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2") + ".json"
+	}
+	indexPath := pm.IndexDir.Join(indexFileName)
 	index, err := packageindex.LoadIndex(indexPath)
 	if err != nil {
 		return fmt.Errorf(tr("loading json index file %[1]s: %[2]s"), indexPath, err)

Diff for: arduino/resources/index.go

@@ -16,6 +16,7 @@
 package resources
 
 import (
+	"context"
 	"net/url"
 	"path"
 	"strings"
@@ -25,6 +26,8 @@ import (
 	"github.com/arduino/arduino-cli/arduino/security"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
+	"github.com/codeclysm/extract/v3"
+	"github.com/sirupsen/logrus"
 	"go.bug.st/downloader/v2"
 )
 
@@ -56,8 +59,43 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 		return &arduino.FailedDownloadError{Message: tr("Error downloading index '%s'", res.URL), Cause: err}
 	}
 
+	var signaturePath, tmpSignaturePath *paths.Path
+	hasSignature := false
+
 	// Expand the index if it is compressed
-	if strings.HasSuffix(indexFileName, ".gz") {
+	if strings.HasSuffix(indexFileName, ".tar.bz2") {
+		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2") + ".json" // == package_index.json
+		signatureFileName := indexFileName + ".sig"
+		signaturePath = destDir.Join(signatureFileName)
+
+		// .tar.bz2 archive may contain both index and signature
+
+		// Extract archive in a tmp/archive subdirectory
+		f, err := tmpIndexPath.Open()
+		if err != nil {
+			return &arduino.PermissionDeniedError{Message: tr("Error opening %s", tmpIndexPath), Cause: err}
+		}
+		defer f.Close()
+		tmpArchivePath := tmp.Join("archive")
+		_ = tmpArchivePath.MkdirAll()
+		if err := extract.Bz2(context.Background(), f, tmpArchivePath.String(), nil); err != nil {
+			return &arduino.PermissionDeniedError{Message: tr("Error extracting %s", tmpIndexPath), Cause: err}
+		}
+
+		// Look for index.json
+		tmpIndexPath = tmpArchivePath.Join(indexFileName)
+		if !tmpIndexPath.Exist() {
+			return &arduino.NotFoundError{Message: tr("Invalid archive: file %{1}s not found in archive %{2}s", indexFileName, tmpArchivePath.Base())}
+		}
+
+		// Look for signature
+		if t := tmpArchivePath.Join(signatureFileName); t.Exist() {
+			tmpSignaturePath = t
+			hasSignature = true
+		} else {
+			logrus.Infof("No signature %s found in package index archive %s", signatureFileName, tmpArchivePath.Base())
+		}
+	} else if strings.HasSuffix(indexFileName, ".gz") {
 		indexFileName = strings.TrimSuffix(indexFileName, ".gz") // == package_index.json
 		tmpUnzippedIndexPath := tmp.Join(indexFileName)
 		if err := paths.GUnzip(tmpIndexPath, tmpUnzippedIndexPath); err != nil {
@@ -67,7 +105,6 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 	}
 
 	// Check the signature if needed
-	var signaturePath, tmpSignaturePath *paths.Path
 	if res.SignatureURL != nil {
 		// Compose signature URL
 		signatureFileName := path.Base(res.SignatureURL.Path)
@@ -79,6 +116,10 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 			return &arduino.FailedDownloadError{Message: tr("Error downloading index signature '%s'", res.SignatureURL), Cause: err}
 		}
 
+		hasSignature = true
+	}
+
+	if hasSignature {
 		// Check signature...
 		if valid, _, err := security.VerifyArduinoDetachedSignature(tmpIndexPath, tmpSignaturePath); err != nil {
 			return &arduino.PermissionDeniedError{Message: tr("Error verifying signature"), Cause: err}
@@ -109,12 +150,12 @@ func (res *IndexResource) Download(destDir *paths.Path, downloadCB rpc.DownloadP
 	if err := tmpIndexPath.CopyTo(indexPath); err != nil {
 		return &arduino.PermissionDeniedError{Message: tr("Error saving downloaded index"), Cause: err}
 	}
-	if res.SignatureURL != nil {
+	if hasSignature {
 		if err := tmpSignaturePath.CopyTo(signaturePath); err != nil {
 			return &arduino.PermissionDeniedError{Message: tr("Error saving downloaded index signature"), Cause: err}
 		}
 	}
-	oldIndex.Remove()
-	oldSignature.Remove()
+	_ = oldIndex.Remove()
+	_ = oldSignature.Remove()
 	return nil
 }

Diff for: arduino/resources/resources_test.go

@@ -18,6 +18,9 @@ package resources
 import (
 	"crypto"
 	"encoding/hex"
+	"net"
+	"net/http"
+	"net/url"
 	"testing"
 
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
@@ -110,3 +113,38 @@ func TestDownloadAndChecksums(t *testing.T) {
 	_, err = r.TestLocalArchiveChecksum(tmp)
 	require.Error(t, err)
 }
+
+func TestIndexDownloadAndSignatureWithinArchive(t *testing.T) {
+	// Spawn test webserver
+	mux := http.NewServeMux()
+	fs := http.FileServer(http.Dir("testdata"))
+	mux.Handle("/", fs)
+	server := &http.Server{Handler: mux}
+	ln, err := net.Listen("tcp", "127.0.0.1:")
+	require.NoError(t, err)
+	defer ln.Close()
+	go server.Serve(ln)
+
+	validIdxURL, err := url.Parse("http://" + ln.Addr().String() + "/valid/package_index.tar.bz2")
+	require.NoError(t, err)
+	idxResource := &IndexResource{URL: validIdxURL}
+	destDir, err := paths.MkTempDir("", "")
+	require.NoError(t, err)
+	defer destDir.RemoveAll()
+	err = idxResource.Download(destDir, func(curr *rpc.DownloadProgress) {})
+	require.NoError(t, err)
+	require.True(t, destDir.Join("package_index.json").Exist())
+	require.True(t, destDir.Join("package_index.json.sig").Exist())
+
+	invalidIdxURL, err := url.Parse("http://" + ln.Addr().String() + "/invalid/package_index.tar.bz2")
+	require.NoError(t, err)
+	invIdxResource := &IndexResource{URL: invalidIdxURL}
+	invDestDir, err := paths.MkTempDir("", "")
+	require.NoError(t, err)
+	defer invDestDir.RemoveAll()
+	err = invIdxResource.Download(invDestDir, func(curr *rpc.DownloadProgress) {})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "invalid signature")
+	require.False(t, invDestDir.Join("package_index.json").Exist())
+	require.False(t, invDestDir.Join("package_index.json.sig").Exist())
+}

Diff for: arduino/resources/testdata/invalid/package_index.tar.bz2

@@ -134,10 +134,9 @@ func indexesNeedUpdating(duration string) bool {
 
 	now := time.Now()
 	modTimeThreshold, err := time.ParseDuration(duration)
-	// Not the most elegant way of handling this error
-	// but it does its job
 	if err != nil {
-		modTimeThreshold, _ = time.ParseDuration("24h")
+		feedback.Error(tr("Invalid timeout: %s", err))
+		os.Exit(errorcodes.ErrBadArgument)
 	}
 
 	urls := []string{globals.DefaultIndexURL}
@@ -153,7 +152,18 @@ func indexesNeedUpdating(duration string) bool {
 			continue
 		}
 
-		coreIndexPath := indexpath.Join(path.Base(URL.Path))
+		// should handle:
+		// - package_index.json
+		// - package_index.json.sig
+		// - package_index.json.gz
+		// - package_index.tar.bz2
+		indexFileName := path.Base(URL.Path)
+		indexFileName = strings.TrimSuffix(indexFileName, ".tar.bz2")
+		indexFileName = strings.TrimSuffix(indexFileName, ".gz")
+		indexFileName = strings.TrimSuffix(indexFileName, ".sig")
+		indexFileName = strings.TrimSuffix(indexFileName, ".json")
+		// and obtain package_index.json as result
+		coreIndexPath := indexpath.Join(indexFileName + ".json")
 		if coreIndexPath.NotExist() {
 			return true
 		}

Diff for: arduino/resources/testdata/valid/package_index.tar.bz2

@@ -26,5 +26,5 @@ var (
 	// VersionInfo contains all info injected during build
 	VersionInfo = version.NewInfo(filepath.Base(os.Args[0]))
 	// DefaultIndexURL is the default index url
-	DefaultIndexURL = "https://downloads.arduino.cc/packages/package_index.json"
+	DefaultIndexURL = "https://downloads.arduino.cc/packages/package_index.tar.bz2"
 )

Diff for: cli/core/search.go

@@ -434,7 +434,7 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rp
 		indexResource := resources.IndexResource{
 			URL: URL,
 		}
-		if strings.HasSuffix(URL.Host, "arduino.cc") {
+		if strings.HasSuffix(URL.Host, "arduino.cc") && strings.HasSuffix(URL.Path, ".json") {
 			indexResource.SignatureURL, _ = url.Parse(u) // should not fail because we already parsed it
 			indexResource.SignatureURL.Path += ".sig"
 		}

Diff for: cli/globals/globals.go

@@ -26,7 +26,10 @@ import (
 
 // UserAgent returns the user agent (mainly used by HTTP clients)
 func UserAgent(settings *viper.Viper) string {
-	subComponent := settings.GetString("network.user_agent_ext")
+	subComponent := ""
+	if settings != nil {
+		subComponent = settings.GetString("network.user_agent_ext")
+	}
 	if subComponent != "" {
 		subComponent = " " + subComponent
 	}
@@ -41,7 +44,7 @@ func UserAgent(settings *viper.Viper) string {
 
 // NetworkProxy returns the proxy configuration (mainly used by HTTP clients)
 func NetworkProxy(settings *viper.Viper) (*url.URL, error) {
-	if !settings.IsSet("network.proxy") {
+	if settings == nil || !settings.IsSet("network.proxy") {
 		return nil, nil
 	}
 	if proxyConfig := settings.GetString("network.proxy"); proxyConfig == "" {

Diff for: commands/instances.go

@@ -30,7 +30,7 @@
   "official": true,
   "package": {
     "maintainer": "Arduino",
-    "url": "https://downloads.arduino.cc/packages/package_index.json",
+    "url": "https://downloads.arduino.cc/packages/package_index.tar.bz2",
     "website_url": "http://www.arduino.cc/",
     "email": "[email protected]",
     "name": "arduino",

Diff for: configuration/network.go

@@ -201,7 +201,7 @@ def test_core_install_without_updateindex(run_command):
     # Download samd core pinned to 1.8.6
     result = run_command(["core", "install", "arduino:[email protected]"])
     assert result.ok
-    assert "Downloading index: package_index.json downloaded" in result.stdout
+    assert "Downloading index: package_index.tar.bz2 downloaded" in result.stdout
 
 
 @pytest.mark.skipif(

Diff for: test/test_board.py

@@ -21,8 +21,7 @@ def test_update(run_command):
     assert res.ok
     lines = [l.strip() for l in res.stdout.splitlines()]
 
-    assert "Downloading index: package_index.json downloaded" in lines
-    assert "Downloading index signature: package_index.json.sig downloaded" in lines
+    assert "Downloading index: package_index.tar.bz2 downloaded" in lines
     assert "Downloading index: library_index.json.gz downloaded" in lines
     assert "Downloading index signature: library_index.json.sig downloaded" in lines
 
@@ -45,8 +44,7 @@ def test_update_showing_outdated(run_command):
     assert result.ok
     lines = [l.strip() for l in result.stdout.splitlines()]
 
-    assert "Downloading index: package_index.json downloaded" in lines
-    assert "Downloading index signature: package_index.json.sig downloaded" in lines
+    assert "Downloading index: package_index.tar.bz2 downloaded" in lines
     assert "Downloading index: library_index.json.gz downloaded" in lines
     assert "Downloading index signature: library_index.json.sig downloaded" in lines
     assert lines[-5].startswith("Arduino AVR Boards")