diff --git a/commands/sketch/new.go b/commands/sketch/new.go index b2871c48d95..4e8134a5b6c 100644 --- a/commands/sketch/new.go +++ b/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 +} diff --git a/commands/sketch/new_test.go b/commands/sketch/new_test.go new file mode 100644 index 00000000000..88d2e426012 --- /dev/null +++ b/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) + } +} diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index b7b267ab1e1..9c9cb4b068c 100644 --- a/docs/UPGRADING.md +++ b/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 diff --git a/internal/cli/sketch/new.go b/internal/cli/sketch/new.go index 3b361f3d8ea..2f76d691b4c 100644 --- a/internal/cli/sketch/new.go +++ b/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 { diff --git a/internal/integrationtest/sketch/sketch_test.go b/internal/integrationtest/sketch/sketch_test.go index 496236b31d1..69854540536 100644 --- a/internal/integrationtest/sketch/sketch_test.go +++ b/internal/integrationtest/sketch/sketch_test.go @@ -73,6 +73,16 @@ func TestSketchNew(t *testing.T) { require.FileExists(t, currentSketchPath.Join(sketchName).String()+".ino") } +func TestSketchNewEmptyName(t *testing.T) { + // testing that we fail nicely. It panicked in the past + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + sketchName := "" + _, _, err := cli.Run("sketch", "new", sketchName) + require.Error(t, err, "Can't create sketch: sketch name cannot be empty") +} + func verifyZipContainsSketchExcludingBuildDir(t *testing.T, files []*zip.File) { require.Len(t, files, 8) require.Equal(t, paths.New("sketch_simple", "doc.txt").String(), files[0].Name)