[skip-changelog] Avoid getting locked up in a perpetual symlink loop when loading a library (#2146)

* Improve library examples loading process by filtering out unneeded directories

* Add tests for the changes

arduino/libraries/libraries_test.go

@@ -17,7 +17,9 @@ package libraries
 
 import (
 	"encoding/json"
+	"os"
 	"testing"
+	"time"
 
 	paths "github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/require"
@@ -85,3 +87,69 @@ func TestLibrariesLoader(t *testing.T) {
 		require.True(t, lib.IsLegacy)
 	}
 }
+
+func TestSymlinkLoop(t *testing.T) {
+	// Set up directory structure of test library.
+	testLib := paths.New("testdata", "TestLib")
+	examplesPath := testLib.Join("examples")
+	require.NoError(t, examplesPath.Mkdir())
+	defer examplesPath.RemoveAll()
+
+	// It's probably most friendly for contributors using Windows to create the symlinks needed for the test on demand.
+	err := os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer1").String())
+	require.NoError(t, err, "This test must be run as administrator on Windows to have symlink creation privilege.")
+	// It's necessary to have multiple symlinks to a parent directory to create the loop.
+	err = os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer2").String())
+	require.NoError(t, err)
+
+	// The failure condition is Load() never returning, testing for which requires setting up a timeout.
+	done := make(chan bool)
+	go func() {
+		_, err = Load(testLib, User)
+		done <- true
+	}()
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		require.FailNow(t, "Load didn't complete in the allocated time.")
+	}
+	require.Error(t, err)
+}
+
+func TestLegacySymlinkLoop(t *testing.T) {
+	// Set up directory structure of test library.
+	testLib := paths.New("testdata", "LegacyLib")
+	examplesPath := testLib.Join("examples")
+	require.NoError(t, examplesPath.Mkdir())
+	defer examplesPath.RemoveAll()
+
+	// It's probably most friendly for contributors using Windows to create the symlinks needed for the test on demand.
+	err := os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer1").String())
+	require.NoError(t, err, "This test must be run as administrator on Windows to have symlink creation privilege.")
+	// It's necessary to have multiple symlinks to a parent directory to create the loop.
+	err = os.Symlink(examplesPath.Join("..").String(), examplesPath.Join("UpGoer2").String())
+	require.NoError(t, err)
+
+	// The failure condition is Load() never returning, testing for which requires setting up a timeout.
+	done := make(chan bool)
+	go func() {
+		_, err = Load(testLib, User)
+		done <- true
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(2 * time.Second):
+		require.FailNow(t, "Load didn't complete in the allocated time.")
+	}
+	require.Error(t, err)
+}
+
+func TestLoadExamples(t *testing.T) {
+	example, err := paths.New(".", "testdata", "TestLibExamples", "examples", "simple").Abs()
+	require.NoError(t, err)
+	lib, err := Load(paths.New("testdata", "TestLibExamples"), User)
+	require.NoError(t, err)
+	require.Len(t, lib.Examples, 1)
+	require.True(t, lib.Examples.Contains(example))
+}

arduino/libraries/loader.go

@@ -20,7 +20,6 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino/globals"
-	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
 	"github.com/pkg/errors"
@@ -176,20 +175,11 @@ func addExamples(lib *Library) error {
 }
 
 func addExamplesToPathList(examplesPath *paths.Path, list *paths.PathList) error {
-	files, err := examplesPath.ReadDir()
+	files, err := examplesPath.ReadDirRecursiveFiltered(nil, paths.AndFilter(paths.FilterDirectories(), filterExamplesDirs))
 	if err != nil {
 		return err
 	}
-	for _, file := range files {
-		_, err := sketch.New(file)
-		if err == nil {
-			list.Add(file)
-		} else if file.IsDir() {
-			if err := addExamplesToPathList(file, list); err != nil {
-				return err
-			}
-		}
-	}
+	list.AddAll(files)
 	return nil
 }
 
@@ -208,3 +198,17 @@ func containsHeaderFile(d *paths.Path) (bool, error) {
 	dirContent.FilterSuffix(headerExtensions...)
 	return len(dirContent) > 0, nil
 }
+
+// filterExamplesDirs filters out any directory that does not contain a ".ino" or ".pde" file
+// with the correct casing
+func filterExamplesDirs(dir *paths.Path) bool {
+	if list, err := dir.ReadDir(); err == nil {
+		list.FilterOutDirs()
+		list.FilterPrefix(dir.Base()+".ino", dir.Base()+".pde")
+		// accept only directories that contain a single correct sketch
+		if list.Len() == 1 {
+			return true
+		}
+	}
+	return false
+}

arduino/libraries/testdata/TestLibExamples/library.properties

+name=TestLib
+version=1.0.3
+author=Arduino
+maintainer=Arduino <info@arduino.cc>
+sentence=A test lib
+paragraph=very powerful!
+category=Device Control
+url=http://www.arduino.cc/en/Reference/TestLib
+architectures=avr
\ No newline at end of file