Skip to content

Commit

Permalink
fix: handle empty sketch name
Browse files Browse the repository at this point in the history
  • Loading branch information
Luca Bianconi committed Jan 30, 2023
1 parent ef98bbe commit 35eec5e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 9 deletions.
6 changes: 4 additions & 2 deletions commands/sketch/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ 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]{1}[0-9a-zA-Z_\.-]*$`)
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) {
Expand Down Expand Up @@ -71,12 +71,14 @@ func NewSketch(ctx context.Context, req *rpc.NewSketchRequest) (*rpc.NewSketchRe
}

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,
Expand Down
11 changes: 11 additions & 0 deletions commands/sketch/new_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ func Test_SketchNameWrongPattern(t *testing.T) {
}
}

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 {
Expand Down
31 changes: 24 additions & 7 deletions internal/cli/sketch/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions internal/integrationtest/sketch/sketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 35eec5e

Please sign in to comment.