diff --git a/src/frontend/apiserver/controllers/jobs.go b/src/frontend/apiserver/controllers/jobs.go index f761136a..62374139 100644 --- a/src/frontend/apiserver/controllers/jobs.go +++ b/src/frontend/apiserver/controllers/jobs.go @@ -1307,6 +1307,9 @@ var ( // to 400). func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err error) { logger.Debug("Enter preCheckJobRequest") + // bug-1953: the name only decides n and isArray; every submission then + // goes through the same checks and conversions below + n, isArray = 1, false if req.QueueName == "" { req.QueueName = "normal" } else { @@ -1343,27 +1346,26 @@ func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err req.JobName = arrayName logger.Debug("preCheckJobRequest parsed array job name", zap.String("jobName", req.JobName), zap.Int("arraySize", count)) - return count, true, nil - } - - // Names without brackets keep the existing path (1.4). - arrayName, idx, slotLimit, err := createArrayName(req.JobName) - if err != nil { - logger.Error("job name format is invalid", zap.Error(err)) - return 0, false, errors.New("job array list format is invalid: " + err.Error()) - } - - if arrayName != "" { - if err = isValidArrayName(arrayName); err != nil { - logger.Error(err.Error()) + n, isArray = count, true + } else { + // Names without brackets keep the existing path (1.4). + arrayName, idx, slotLimit, err := createArrayName(req.JobName) + if err != nil { + logger.Error("job name format is invalid", zap.Error(err)) return 0, false, errors.New("job array list format is invalid: " + err.Error()) } - } - req.JobName = arrayName - logger.Debug("preCheckJobRequest parsed job name is: ", zap.String("jobName", req.JobName), - zap.Any("indices", idx), zap.Any("slotLimit", slotLimit)) - return 1, false, nil + if arrayName != "" { + if err = isValidArrayName(arrayName); err != nil { + logger.Error(err.Error()) + return 0, false, errors.New("job array list format is invalid: " + err.Error()) + } + } + + req.JobName = arrayName + logger.Debug("preCheckJobRequest parsed job name is: ", zap.String("jobName", req.JobName), + zap.Any("indices", idx), zap.Any("slotLimit", slotLimit)) + } } else { if req.Command != "" { if len(req.Command) > 4094 { @@ -1531,7 +1533,7 @@ func preCheckJobRequest(req *models.JobSubmitRequest) (n int, isArray bool, err req.LaunchType = launchType } - return 1, false, nil + return n, isArray, nil } // It returns minTasks, maxTasks, and an error if parsing fails or constraints are violated. diff --git a/src/frontend/apiserver/controllers/jobs_test.go b/src/frontend/apiserver/controllers/jobs_test.go index 9332c41e..93b598f2 100644 --- a/src/frontend/apiserver/controllers/jobs_test.go +++ b/src/frontend/apiserver/controllers/jobs_test.go @@ -270,3 +270,36 @@ func TestPreCheckJobRequest_RejectsMalformedResreq(t *testing.T) { t.Fatalf("malformed resreq must be rejected, got err=%v", err) } } + +// bug-1953: a job name (plain or array) must not skip the checks and +// conversions every other submission gets. +func TestPreCheckJobRequest_NamedJobsGetFullValidation(t *testing.T) { + resetJobSubmitState() + origCfg := utils.JobYamlConfig + utils.JobYamlConfig = &utils.JobConfig{MaxArraySize: 1000} + t.Cleanup(func() { utils.JobYamlConfig = origCfg }) + + for _, name := range []string{"", "myjob", "arr[1-3]"} { + req := &models.JobSubmitRequest{JobName: name, Command: "sleep 1"} + req.ShellLimit.CPULimit = 60000 + n, isArray, err := preCheckJobRequest(req) + if err != nil { + t.Fatalf("name %q: unexpected error %v", name, err) + } + if req.CPULimit.Seconds != 60 { + t.Errorf("name %q: cpu limit not converted: got %d s, want 60", name, req.CPULimit.Seconds) + } + if name == "arr[1-3]" && (n != 3 || !isArray || req.JobName != "arr") { + t.Errorf("array name: got n=%d isArray=%v name=%q, want 3 true \"arr\"", n, isArray, req.JobName) + } + if name == "myjob" && (n != 1 || isArray) { + t.Errorf("plain name: got n=%d isArray=%v, want 1 false", n, isArray) + } + + bad := &models.JobSubmitRequest{JobName: name, Command: "sleep 1", + ProjectName: strings.Repeat("p", 512)} + if _, _, err := preCheckJobRequest(bad); err == nil { + t.Errorf("name %q: an over-long project name was accepted", name) + } + } +}