Fix scheduling arguments (#423)

* fix command line arguments broken on launchd after #420

* fix test on mock handler

* addressing code review comments

schedule/command_arguments.go

@@ -0,0 +1,50 @@
+package schedule
+
+import "strings"
+
+type CommandArguments struct {
+	args []string
+}
+
+func NewCommandArguments(args []string) CommandArguments {
+	return CommandArguments{
+		args: args,
+	}
+}
+
+func (ca CommandArguments) RawArgs() []string {
+	result := make([]string, len(ca.args))
+	copy(result, ca.args)
+	return result
+}
+
+// String returns the arguments as a string, with quotes around arguments that contain spaces
+func (ca CommandArguments) String() string {
+	if len(ca.args) == 0 {
+		return ""
+	}
+
+	var n int
+	for _, elem := range ca.args {
+		n += len(elem) + 3 // add 2 if quotes are needed, plus 1 for the space
+	}
+
+	b := new(strings.Builder)
+	b.Grow(n)
+	ca.writeString(b, ca.args[0])
+	for _, s := range ca.args[1:] {
+		b.WriteString(" ")
+		ca.writeString(b, s)
+	}
+	return b.String()
+}
+
+func (ca CommandArguments) writeString(b *strings.Builder, str string) {
+	if strings.Contains(str, " ") {
+		b.WriteString(`"`)
+		b.WriteString(str)
+		b.WriteString(`"`)
+	} else {
+		b.WriteString(str)
+	}
+}

schedule/command_arguments_test.go

@@ -0,0 +1,59 @@
+package schedule
+
+import (
+	"testing"
+)
+
+func TestRawArgs(t *testing.T) {
+	tests := []struct {
+		name string
+		args []string
+	}{
+		{"empty args", []string{}},
+		{"simple args", []string{"arg1", "arg2"}},
+		{"args with spaces", []string{"C:\\Program Files\\app.exe", "--config", "C:\\My Documents\\config.toml"}},
+		{"args with special chars", []string{"--name", "my-task!", "--config=test.conf"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ca := NewCommandArguments(tt.args)
+			rawArgs := ca.RawArgs()
+			if len(rawArgs) != len(tt.args) {
+				t.Errorf("expected %d raw arguments, got %d", len(tt.args), len(rawArgs))
+			}
+			for i, arg := range tt.args {
+				if rawArgs[i] != arg {
+					t.Errorf("expected raw argument %d to be %s, got %s", i, arg, rawArgs[i])
+				}
+			}
+		})
+	}
+}
+
+func TestString(t *testing.T) {
+	tests := []struct {
+		args     []string
+		expected string
+	}{
+		{[]string{}, ""},
+		{[]string{"arg1"}, "arg1"},
+		{[]string{"arg1 with space"}, `"arg1 with space"`},
+		{[]string{"arg1", "arg2"}, "arg1 arg2"},
+		{[]string{"arg1", "arg with spaces"}, `arg1 "arg with spaces"`},
+		{[]string{"arg1", "arg with spaces", "anotherArg"}, `arg1 "arg with spaces" anotherArg`},
+		{[]string{"--config", "C:\\Program Files\\config.toml"}, `--config "C:\Program Files\config.toml"`},
+		{[]string{"--config", "C:\\Users\\John Doe\\Documents\\config.toml", "--name", "backup task"},
+			`--config "C:\Users\John Doe\Documents\config.toml" --name "backup task"`},
+		{[]string{"--config", "C:\\My Files\\config.toml", "--no-ansi"},
+			`--config "C:\My Files\config.toml" --no-ansi`},
+	}
+
+	for _, test := range tests {
+		ca := NewCommandArguments(test.args)
+		result := ca.String()
+		if result != test.expected {
+			t.Errorf("expected %s, got %s", test.expected, result)
+		}
+	}
+}

schedule/config.go

@@ -14,7 +14,7 @@ type Config struct {
 	Permission              string
 	WorkingDirectory        string
 	Command                 string // path to resticprofile executable
-	Arguments               []string
+	Arguments               CommandArguments
 	Environment             []string
 	JobDescription          string
 	TimerDescription        string
@@ -37,10 +37,12 @@ func NewRemoveOnlyConfig(profileName, commandName string) *Config {
 	}
 }
 
+// SetCommand sets the command details for scheduling. Arguments are automatically
+// processed to ensure proper handling of paths with spaces and special characters.
 func (s *Config) SetCommand(wd, command string, args []string) {
 	s.WorkingDirectory = wd
 	s.Command = command
-	s.Arguments = args
+	s.Arguments = NewCommandArguments(args)
 }
 
 // Priority is either "background" or "standard"

schedule/config_test.go

@@ -14,7 +14,7 @@ func TestScheduleProperties(t *testing.T) {
 		Permission:              "admin",
 		WorkingDirectory:        "home",
 		Command:                 "command",
-		Arguments:               []string{"1", "2"},
+		Arguments:               NewCommandArguments([]string{"1", "2"}),
 		Environment:             []string{"test=dev"},
 		JobDescription:          "job",
 		TimerDescription:        "timer",
@@ -35,7 +35,8 @@ func TestScheduleProperties(t *testing.T) {
 	assert.Equal(t, "admin", schedule.Permission)
 	assert.Equal(t, "command", schedule.Command)
 	assert.Equal(t, "home", schedule.WorkingDirectory)
-	assert.ElementsMatch(t, []string{"1", "2"}, schedule.Arguments)
+	assert.ElementsMatch(t, []string{"1", "2"}, schedule.Arguments.RawArgs())
+	assert.Equal(t, `1 2`, schedule.Arguments.String())
 	assert.Equal(t, []string{"test=dev"}, schedule.Environment)
 	assert.Equal(t, "background", schedule.GetPriority()) // default value
 }

schedule/handler_crond.go

@@ -1,8 +1,6 @@
 package schedule
 
 import (
-	"strings"
-
 	"github.com/creativeprojects/resticprofile/calendar"
 	"github.com/creativeprojects/resticprofile/constants"
 	"github.com/creativeprojects/resticprofile/crond"
@@ -68,7 +66,7 @@ func (h *HandlerCrond) CreateJob(job *Config, schedules []*calendar.Event, permi
 			job.ConfigFile,
 			job.ProfileName,
 			job.CommandName,
-			job.Command+" "+strings.Join(job.Arguments, " "),
+			job.Command+" "+job.Arguments.String(),
 			job.WorkingDirectory,
 		)
 		if h.config.Username != "" {
@@ -92,7 +90,7 @@ func (h *HandlerCrond) RemoveJob(job *Config, permission string) error {
 			job.ConfigFile,
 			job.ProfileName,
 			job.CommandName,
-			job.Command+" "+strings.Join(job.Arguments, " "),
+			job.Command+" "+job.Arguments.String(),
 			job.WorkingDirectory,
 		),
 	}

schedule/handler_darwin.go

@@ -144,7 +144,6 @@ Do you want to start it now?`
 
 func (h *HandlerLaunchd) getLaunchdJob(job *Config, schedules []*calendar.Event) *LaunchdJob {
 	name := getJobName(job.ProfileName, job.CommandName)
-	args := job.Arguments
 	// we always set the log file in the job settings as a default
 	// if changed in the configuration via schedule-log the standard output will be empty anyway
 	logfile := name + ".log"
@@ -165,7 +164,7 @@ func (h *HandlerLaunchd) getLaunchdJob(job *Config, schedules []*calendar.Event)
 	launchdJob := &LaunchdJob{
 		Label:                 name,
 		Program:               job.Command,
-		ProgramArguments:      append([]string{job.Command, "--no-prio"}, args...),
+		ProgramArguments:      append([]string{job.Command, "--no-prio"}, job.Arguments.RawArgs()...),
 		StandardOutPath:       logfile,
 		StandardErrorPath:     logfile,
 		WorkingDirectory:      job.WorkingDirectory,

schedule/handler_systemd.go

@@ -107,7 +107,7 @@ func (h *HandlerSystemd) CreateJob(job *Config, schedules []*calendar.Event, per
 	}
 
 	err := systemd.Generate(systemd.Config{
-		CommandLine:          job.Command + " " + strings.Join(append([]string{"--no-prio"}, job.Arguments...), " "),
+		CommandLine:          job.Command + " " + strings.Join(append([]string{"--no-prio"}, job.Arguments.RawArgs()...), " "),
 		Environment:          job.Environment,
 		WorkingDirectory:     job.WorkingDirectory,
 		Title:                job.ProfileName,

schedule/handler_test.go

@@ -1,22 +1,22 @@
 package schedule
 
 import (
-	"runtime"
 	"testing"
 
+	"github.com/creativeprojects/resticprofile/platform"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestLookupExistingBinary(t *testing.T) {
-	if runtime.GOOS == "windows" {
+	if platform.IsWindows() {
 		t.Skip()
 	}
 	err := lookupBinary("sh", "sh")
 	assert.NoError(t, err)
 }
 
 func TestLookupNonExistingBinary(t *testing.T) {
-	if runtime.GOOS == "windows" {
+	if platform.IsWindows() {
 		t.Skip()
 	}
 	err := lookupBinary("something", "almost_certain_not_to_be_available")

schedule/handler_windows.go

@@ -58,7 +58,7 @@ func (h *HandlerWindows) CreateJob(job *Config, schedules []*calendar.Event, per
 		ProfileName:      job.ProfileName,
 		CommandName:      job.CommandName,
 		Command:          job.Command,
-		Arguments:        job.Arguments,
+		Arguments:        job.Arguments.String(),
 		WorkingDirectory: job.WorkingDirectory,
 		JobDescription:   job.JobDescription,
 	}

schedule_jobs.go

@@ -33,7 +33,7 @@ func scheduleJobs(handler schedule.Handler, profileName string, configs []*confi
 		args := []string{
 			"--no-ansi",
 			"--config",
-			`"` + scheduleConfig.ConfigFile + `"`,
+			scheduleConfig.ConfigFile,
 			"run-schedule",
 			scheduleName,
 		}
@@ -141,7 +141,7 @@ func scheduleToConfig(sched *config.Schedule) *schedule.Config {
 		Permission:              sched.Permission,
 		WorkingDirectory:        "",
 		Command:                 "",
-		Arguments:               []string{},
+		Arguments:               schedule.NewCommandArguments(nil),
 		Environment:             sched.Environment,
 		JobDescription:          "",
 		TimerDescription:        "",

schedule_jobs_test.go

@@ -42,12 +42,13 @@ func TestSimpleScheduleJob(t *testing.T) {
 		mock.AnythingOfType("[]*calendar.Event"),
 		mock.AnythingOfType("string")).
 		RunAndReturn(func(scheduleConfig *schedule.Config, events []*calendar.Event, permission string) error {
-			assert.Equal(t, []string{"--no-ansi", "--config", `"config.file"`, "run-schedule", "backup@profile"}, scheduleConfig.Arguments)
+			assert.Equal(t, []string{"--no-ansi", "--config", `config file`, "run-schedule", "backup@profile"}, scheduleConfig.Arguments.RawArgs())
+			assert.Equal(t, `--no-ansi --config "config file" run-schedule backup@profile`, scheduleConfig.Arguments.String())
 			return nil
 		})
 
 	scheduleConfig := configForJob("backup", "sched")
-	scheduleConfig.ConfigFile = "config.file"
+	scheduleConfig.ConfigFile = "config file"
 	err := scheduleJobs(handler, "profile", []*config.Schedule{scheduleConfig})
 	assert.NoError(t, err)
 }

schtasks/config.go

@@ -4,7 +4,7 @@ type Config struct {
 	ProfileName      string
 	CommandName      string
 	Command          string
-	Arguments        []string
+	Arguments        string
 	WorkingDirectory string
 	JobDescription   string
 }

schtasks/taskscheduler.go

@@ -114,7 +114,7 @@ func createUserTask(config *Config, schedules []*calendar.Event) error {
 	task := taskService.NewTaskDefinition()
 	task.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Principal.LogonType = taskmaster.TASK_LOGON_PASSWORD
@@ -154,7 +154,7 @@ func updateUserTask(task taskmaster.RegisteredTask, config *Config, schedules []
 	task.Definition.Actions = make([]taskmaster.Action, 0, 1)
 	task.Definition.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_PASSWORD
@@ -210,7 +210,7 @@ func createUserLoggedOnTask(config *Config, schedules []*calendar.Event) error {
 	task := taskService.NewTaskDefinition()
 	task.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Principal.LogonType = taskmaster.TASK_LOGON_INTERACTIVE_TOKEN
@@ -244,7 +244,7 @@ func updateUserLoggedOnTask(task taskmaster.RegisteredTask, config *Config, sche
 	task.Definition.Actions = make([]taskmaster.Action, 0, 1)
 	task.Definition.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_INTERACTIVE_TOKEN
@@ -278,7 +278,7 @@ func createSystemTask(config *Config, schedules []*calendar.Event) error {
 	task := taskService.NewTaskDefinition()
 	task.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Principal.LogonType = taskmaster.TASK_LOGON_SERVICE_ACCOUNT
@@ -307,7 +307,7 @@ func updateSystemTask(task taskmaster.RegisteredTask, config *Config, schedules
 	task.Definition.Actions = make([]taskmaster.Action, 0, 1)
 	task.Definition.AddAction(taskmaster.ExecAction{
 		Path:       config.Command,
-		Args:       strings.Join(config.Arguments, " "),
+		Args:       config.Arguments,
 		WorkingDir: config.WorkingDirectory,
 	})
 	task.Definition.Principal.LogonType = taskmaster.TASK_LOGON_SERVICE_ACCOUNT

schtasks/taskscheduler_test.go

@@ -349,7 +349,7 @@ func TestCreationOfTasks(t *testing.T) {
 				ProfileName:      "test",
 				CommandName:      strconv.Itoa(count),
 				Command:          "echo",
-				Arguments:        []string{"hello"},
+				Arguments:        "hello there",
 				WorkingDirectory: "C:\\",
 				JobDescription:   fixture.description,
 			}