Merge branch 'master' into feat/purge-build-cache

Author: Bikappa

arduino/cores/fqbn.go

@@ -76,6 +76,26 @@ func (fqbn *FQBN) String() string {
 	return res
 }
 
+// Match check if the target FQBN corresponds to the receiver one.
+// The core parts are checked for exact equality while board options are loosely
+// matched: the set of boards options of the target must be fully contained within
+// the one of the receiver and their values must be equal.
+func (fqbn *FQBN) Match(target *FQBN) bool {
+	if fqbn.StringWithoutConfig() != target.StringWithoutConfig() {
+		return false
+	}
+
+	searchedProperties := target.Configs.Clone()
+	actualConfigs := fqbn.Configs.AsMap()
+	for neededKey, neededValue := range searchedProperties.AsMap() {
+		targetValue, hasKey := actualConfigs[neededKey]
+		if !hasKey || targetValue != neededValue {
+			return false
+		}
+	}
+	return true
+}
+
 // StringWithoutConfig returns the FQBN without the Config part
 func (fqbn *FQBN) StringWithoutConfig() string {
 	return fqbn.Package + ":" + fqbn.PlatformArch + ":" + fqbn.BoardID

arduino/cores/fqbn_test.go

@@ -121,3 +121,37 @@ func TestFQBN(t *testing.T) {
 		"properties.Map{\n  \"cpu\": \"atmega\",\n  \"speed\": \"1000\",\n  \"extra\": \"core=arduino\",\n}",
 		f.Configs.Dump())
 }
+
+func TestMatch(t *testing.T) {
+	expectedMatches := [][]string{
+		{"arduino:avr:uno", "arduino:avr:uno"},
+		{"arduino:avr:uno", "arduino:avr:uno:opt1=1,opt2=2"},
+		{"arduino:avr:uno:opt1=1", "arduino:avr:uno:opt1=1,opt2=2"},
+		{"arduino:avr:uno:opt1=1,opt2=2", "arduino:avr:uno:opt1=1,opt2=2"},
+		{"arduino:avr:uno:opt3=3,opt1=1,opt2=2", "arduino:avr:uno:opt2=2,opt3=3,opt1=1,opt4=4"},
+	}
+
+	for _, pair := range expectedMatches {
+		a, err := ParseFQBN(pair[0])
+		require.NoError(t, err)
+		b, err := ParseFQBN(pair[1])
+		require.NoError(t, err)
+		require.True(t, b.Match(a))
+	}
+
+	expectedMismatches := [][]string{
+		{"arduino:avr:uno", "arduino:avr:due"},
+		{"arduino:avr:uno", "arduino:avr:due:opt1=1,opt2=2"},
+		{"arduino:avr:uno:opt1=1", "arduino:avr:uno"},
+		{"arduino:avr:uno:opt1=1,opt2=", "arduino:avr:uno:opt1=1,opt2=3"},
+		{"arduino:avr:uno:opt1=1,opt2=2", "arduino:avr:uno:opt2=2"},
+	}
+
+	for _, pair := range expectedMismatches {
+		a, err := ParseFQBN(pair[0])
+		require.NoError(t, err)
+		b, err := ParseFQBN(pair[1])
+		require.NoError(t, err)
+		require.False(t, b.Match(a))
+	}
+}

commands/board/list.go

@@ -205,6 +205,15 @@ func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartError
 	}
 	defer release()
 
+	var fqbnFilter *cores.FQBN
+	if f := req.GetFqbn(); f != "" {
+		var err error
+		fqbnFilter, err = cores.ParseFQBN(f)
+		if err != nil {
+			return nil, nil, &arduino.InvalidFQBNError{Cause: err}
+		}
+	}
+
 	dm := pme.DiscoveryManager()
 	discoveryStartErrors = dm.Start()
 	time.Sleep(time.Duration(req.GetTimeout()) * time.Millisecond)
@@ -222,11 +231,27 @@ func List(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartError
 			Port:           port.ToRPC(),
 			MatchingBoards: boards,
 		}
-		retVal = append(retVal, b)
+
+		if fqbnFilter == nil || hasMatchingBoard(b, fqbnFilter) {
+			retVal = append(retVal, b)
+		}
 	}
 	return retVal, discoveryStartErrors, nil
 }
 
+func hasMatchingBoard(b *rpc.DetectedPort, fqbnFilter *cores.FQBN) bool {
+	for _, detectedBoard := range b.MatchingBoards {
+		detectedFqbn, err := cores.ParseFQBN(detectedBoard.Fqbn)
+		if err != nil {
+			continue
+		}
+		if detectedFqbn.Match(fqbnFilter) {
+			return true
+		}
+	}
+	return false
+}
+
 // Watch returns a channel that receives boards connection and disconnection events.
 // It also returns a callback function that must be used to stop and dispose the watch.
 func Watch(req *rpc.BoardListWatchRequest) (<-chan *rpc.BoardListWatchResponse, func(), error) {

commands/sketch/new.go

@@ -18,6 +18,7 @@ package sketch
 import (
 	"context"
 	"errors"
+	"regexp"
 
 	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/globals"
@@ -34,6 +35,10 @@ void loop() {
 }
 `)
 
+// sketchNameMaxLength could be part of the regex, but it's intentionally left out for clearer error reporting
+var sketchNameMaxLength = 63
+var sketchNameValidationRegex = regexp.MustCompile(`^[0-9a-zA-Z][0-9a-zA-Z_\.-]*$`)
+
 // NewSketch creates a new sketch via gRPC
 func NewSketch(ctx context.Context, req *rpc.NewSketchRequest) (*rpc.NewSketchResponse, error) {
 	var sketchesDir string
@@ -42,6 +47,11 @@ func NewSketch(ctx context.Context, req *rpc.NewSketchRequest) (*rpc.NewSketchRe
 	} else {
 		sketchesDir = configuration.Settings.GetString("directories.User")
 	}
+
+	if err := validateSketchName(req.SketchName); err != nil {
+		return nil, err
+	}
+
 	sketchDirPath := paths.New(sketchesDir).Join(req.SketchName)
 	if err := sketchDirPath.MkdirAll(); err != nil {
 		return nil, &arduino.CantCreateSketchError{Cause: err}
@@ -59,3 +69,20 @@ func NewSketch(ctx context.Context, req *rpc.NewSketchRequest) (*rpc.NewSketchRe
 
 	return &rpc.NewSketchResponse{MainFile: sketchMainFilePath.String()}, nil
 }
+
+func validateSketchName(name string) error {
+	if name == "" {
+		return &arduino.CantCreateSketchError{Cause: errors.New(tr("sketch name cannot be empty"))}
+	}
+	if len(name) > sketchNameMaxLength {
+		return &arduino.CantCreateSketchError{Cause: errors.New(tr("sketch name too long (%d characters). Maximum allowed length is %d",
+			len(name),
+			sketchNameMaxLength))}
+	}
+	if !sketchNameValidationRegex.MatchString(name) {
+		return &arduino.CantCreateSketchError{Cause: errors.New(tr("invalid sketch name \"%s\". Required pattern %s",
+			name,
+			sketchNameValidationRegex.String()))}
+	}
+	return nil
+}

commands/sketch/new_test.go

@@ -0,0 +1,82 @@
+package sketch
+
+import (
+	"context"
+	"testing"
+
+	"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	"github.com/stretchr/testify/require"
+)
+
+func Test_SketchNameWrongPattern(t *testing.T) {
+	invalidNames := []string{
+		"&",
+		"",
+		".hello",
+		"_hello",
+		"-hello",
+		"hello*",
+		"||||||||||||||",
+		",`hack[}attempt{];",
+	}
+	for _, name := range invalidNames {
+		_, err := NewSketch(context.Background(), &commands.NewSketchRequest{
+			SketchName: name,
+			SketchDir:  t.TempDir(),
+		})
+		require.NotNil(t, err)
+
+		require.Error(t, err, `Can't create sketch: invalid sketch name "%s". Required pattern %s`,
+			name,
+			sketchNameValidationRegex)
+	}
+}
+
+func Test_SketchNameEmpty(t *testing.T) {
+	emptyName := ""
+	_, err := NewSketch(context.Background(), &commands.NewSketchRequest{
+		SketchName: emptyName,
+		SketchDir:  t.TempDir(),
+	})
+	require.NotNil(t, err)
+
+	require.Error(t, err, `Can't create sketch: sketch name cannot be empty`)
+}
+
+func Test_SketchNameTooLong(t *testing.T) {
+	tooLongName := make([]byte, sketchNameMaxLength+1)
+	for i := range tooLongName {
+		tooLongName[i] = 'a'
+	}
+	_, err := NewSketch(context.Background(), &commands.NewSketchRequest{
+		SketchName: string(tooLongName),
+		SketchDir:  t.TempDir(),
+	})
+	require.NotNil(t, err)
+
+	require.Error(t, err, `Can't create sketch: sketch name too long (%d characters). Maximum allowed length is %d`,
+		len(tooLongName),
+		sketchNameMaxLength)
+}
+
+func Test_SketchNameOk(t *testing.T) {
+	lengthLimitName := make([]byte, sketchNameMaxLength)
+	for i := range lengthLimitName {
+		lengthLimitName[i] = 'a'
+	}
+	validNames := []string{
+		"h",
+		"h.ello",
+		"h..ello-world",
+		"h..ello-world.",
+		"hello_world__",
+		string(lengthLimitName),
+	}
+	for _, name := range validNames {
+		_, err := NewSketch(context.Background(), &commands.NewSketchRequest{
+			SketchName: name,
+			SketchDir:  t.TempDir(),
+		})
+		require.Nil(t, err)
+	}
+}

docs/UPGRADING.md

@@ -4,6 +4,14 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.30.0
 
+### Sketch name validation
+
+The sketch name submitted via the `sketch new` command of the CLI or the gRPC command
+`cc.arduino.cli.commands.v1.NewSketch` are now validated. The applied rules follow the
+[sketch specifications](https://arduino.github.io/arduino-cli/dev/sketch-specification).
+
+Existing sketch names violating the new constraint need to be updated.
+
 ### `daemon` CLI command's `--ip` flag removal
 
 The `daemon` CLI command no longer allows to set a custom ip for the gRPC communication. Currently there is not enough

internal/cli/board/list.go

@@ -16,10 +16,12 @@
 package board
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"sort"
 
+	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/cores"
 	"github.com/arduino/arduino-cli/commands/board"
 	"github.com/arduino/arduino-cli/internal/cli/arguments"
@@ -47,6 +49,7 @@ func initListCommand() *cobra.Command {
 	}
 
 	timeoutArg.AddToCommand(listCommand)
+	fqbn.AddToCommand(listCommand)
 	listCommand.Flags().BoolVarP(&watch, "watch", "w", false, tr("Command keeps running and prints list of connected boards whenever there is a change."))
 
 	return listCommand
@@ -63,14 +66,19 @@ func runListCommand(cmd *cobra.Command, args []string) {
 		return
 	}
 
-	ports, discvoeryErrors, err := board.List(&rpc.BoardListRequest{
+	ports, discoveryErrors, err := board.List(&rpc.BoardListRequest{
 		Instance: inst,
 		Timeout:  timeoutArg.Get().Milliseconds(),
+		Fqbn:     fqbn.String(),
 	})
+	var invalidFQBNErr *arduino.InvalidFQBNError
+	if errors.As(err, &invalidFQBNErr) {
+		feedback.Fatal(tr(err.Error()), feedback.ErrBadArgument)
+	}
 	if err != nil {
 		feedback.Warning(tr("Error detecting boards: %v", err))
 	}
-	for _, err := range discvoeryErrors {
+	for _, err := range discoveryErrors {
 		feedback.Warning(tr("Error starting discovery: %v", err))
 	}
 	feedback.PrintResult(result{ports})

internal/cli/sketch/new.go

@@ -49,16 +49,33 @@ func initNewCommand() *cobra.Command {
 func runNewCommand(args []string, overwrite bool) {
 	logrus.Info("Executing `arduino-cli sketch new`")
 	// Trim to avoid issues if user creates a sketch adding the .ino extesion to the name
-	sketchName := args[0]
-	trimmedSketchName := strings.TrimSuffix(sketchName, globals.MainFileValidExtension)
-	sketchDirPath, err := paths.New(trimmedSketchName).Abs()
-	if err != nil {
-		feedback.Fatal(tr("Error creating sketch: %v", err), feedback.ErrGeneric)
+	inputSketchName := args[0]
+	trimmedSketchName := strings.TrimSuffix(inputSketchName, globals.MainFileValidExtension)
+
+	var sketchDir string
+	var sketchName string
+	var sketchDirPath *paths.Path
+	var err error
+
+	if trimmedSketchName == "" {
+		// `paths.New` returns nil with an empty string so `paths.Abs` panics.
+		// if the name is empty we rely on the "new" command to fail nicely later
+		// with the same logic in grpc and cli flows
+		sketchDir = ""
+		sketchName = ""
+	} else {
+		sketchDirPath, err = paths.New(trimmedSketchName).Abs()
+		if err != nil {
+			feedback.Fatal(tr("Error creating sketch: %v", err), feedback.ErrGeneric)
+		}
+		sketchDir = sketchDirPath.Parent().String()
+		sketchName = sketchDirPath.Base()
 	}
+
 	_, err = sk.NewSketch(context.Background(), &rpc.NewSketchRequest{
 		Instance:   nil,
-		SketchName: sketchDirPath.Base(),
-		SketchDir:  sketchDirPath.Parent().String(),
+		SketchName: sketchName,
+		SketchDir:  sketchDir,
 		Overwrite:  overwrite,
 	})
 	if err != nil {

internal/integrationtest/board/board_test.go

@@ -91,6 +91,39 @@ func TestBoardList(t *testing.T) {
 		MustBeEmpty()
 }
 
+func TestBoardListWithFqbnFilter(t *testing.T) {
+	if os.Getenv("CI") != "" {
+		t.Skip("VMs have no serial ports")
+	}
+
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	_, _, err := cli.Run("core", "update-index")
+	require.NoError(t, err)
+	stdout, _, err := cli.Run("board", "list", "-b", "foo:bar:baz", "--format", "json")
+	require.NoError(t, err)
+	// this is a bit of a passpartout test, it actually filters the "bluetooth boards" locally
+	// but it would succeed even if the filtering wasn't working properly
+	// TODO: find a way to simulate connected boards or create a unit test which
+	// mocks or initializes multiple components
+	requirejson.Parse(t, stdout).
+		MustBeEmpty()
+}
+
+func TestBoardListWithFqbnFilterInvalid(t *testing.T) {
+	if os.Getenv("CI") != "" {
+		t.Skip("VMs have no serial ports")
+	}
+
+	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
+	defer env.CleanUp()
+
+	_, stderr, err := cli.Run("board", "list", "-b", "yadayada", "--format", "json")
+	require.Error(t, err)
+	requirejson.Query(t, stderr, ".error", `"Invalid FQBN: not an FQBN: yadayada"`)
+}
+
 func TestBoardListWithInvalidDiscovery(t *testing.T) {
 	env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
 	defer env.CleanUp()