feat(job-array-one-based-index): parse submit index expressions (task 6.1)
ParseSubmitIndexExpr accepts only name[1-N] and name[1] (leading zeros allowed) and otherwise reports the lowest-numbered broken rule with its specific message: interactive arrays (2.1), invalid name or malformed expression (2.2), lists/steps (2.3), %N (2.4), start 0 (2.5), reversed (2.6), start above 1 (2.7), above the limit (2.8). Oversized numbers saturate for the bound checks while reversal compares the digit strings, so the rule order holds for any input. Not yet wired into CreateJob (task 6.2). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
3a7e3890e2
commit
0318ac5f44
GPG key ID:
80533536B0EB651C
3 changed files with 359 additions and 1 deletions
204
src/frontend/apiserver/controllers/jobarray.go
Normal file
204
src/frontend/apiserver/controllers/jobarray.go
Normal file
|
|
@ -0,0 +1,204 @@
|
|||
package controllers
|
||||
|
||||
import "fmt"
|
||||
|
||||
// ArrayExprError is a rejection of a submitted job-array index expression.
|
||||
// Rule names the requirement ("2.1" .. "2.8") that the expression breaks;
|
||||
// Msg is the user-facing message.
|
||||
type ArrayExprError struct {
|
||||
Rule string
|
||||
Msg string
|
||||
}
|
||||
|
||||
func (e *ArrayExprError) Error() string { return e.Msg }
|
||||
|
||||
// maxArrayNameLen is the longest array name the array-name rule accepts.
|
||||
const maxArrayNameLen = 4094
|
||||
|
||||
// exprNumSaturate is the value an index-expression number saturates at. It is
|
||||
// far above any array size limit, so a saturated number is never mistaken for
|
||||
// one in range, and the rule order (2.5-2.8) still decides which rule to report.
|
||||
// It fits in a 32-bit int, as does every intermediate value of readExprNum.
|
||||
// Saturated values lose their order, so order comparisons (2.6) use the digit
|
||||
// strings instead (see digitsLess).
|
||||
const exprNumSaturate = 1 << 30
|
||||
|
||||
type exprItem struct {
|
||||
first, last int
|
||||
firstDigits, lastDigits string // digits with leading zeros stripped
|
||||
hasStep bool
|
||||
}
|
||||
|
||||
// digitsLess reports whether the decimal number a is less than b. Both are
|
||||
// digit strings with leading zeros stripped, so a shorter string is smaller
|
||||
// and equal-length strings compare lexicographically.
|
||||
func digitsLess(a, b string) bool {
|
||||
if len(a) != len(b) {
|
||||
return len(a) < len(b)
|
||||
}
|
||||
return a < b
|
||||
}
|
||||
|
||||
func isArrayNameChar(c byte) bool {
|
||||
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
|
||||
(c >= '0' && c <= '9') || c == '.' || c == '_' || c == '-'
|
||||
}
|
||||
|
||||
func isValidSubmitArrayName(s string) bool {
|
||||
if len(s) < 1 || len(s) > maxArrayNameLen {
|
||||
return false
|
||||
}
|
||||
for i := 0; i < len(s); i++ {
|
||||
if !isArrayNameChar(s[i]) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// readExprNum reads one or more decimal digits at s[pos:]. v saturates at
|
||||
// exprNumSaturate; digits is the full digit run with leading zeros stripped
|
||||
// (empty for zero), which keeps the exact value for order comparisons.
|
||||
// ok is false when there is no digit at pos.
|
||||
func readExprNum(s string, pos int) (v int, digits string, next int, ok bool) {
|
||||
next = pos
|
||||
for next < len(s) && s[next] >= '0' && s[next] <= '9' {
|
||||
if v > exprNumSaturate/10 {
|
||||
v = exprNumSaturate
|
||||
} else {
|
||||
v = v*10 + int(s[next]-'0')
|
||||
if v > exprNumSaturate {
|
||||
v = exprNumSaturate
|
||||
}
|
||||
}
|
||||
next++
|
||||
}
|
||||
start := pos
|
||||
for start < next && s[start] == '0' {
|
||||
start++
|
||||
}
|
||||
return v, s[start:next], next, next > pos
|
||||
}
|
||||
|
||||
// parseExprBody parses the part of an index expression after the name,
|
||||
// starting at the '['. It returns the items and whether a "%N" suffix was
|
||||
// present; ok is false when the text does not follow the grammar of 2.2.
|
||||
func parseExprBody(s string) (items []exprItem, hasLimit bool, ok bool) {
|
||||
pos := 1 // past '['
|
||||
for {
|
||||
var it exprItem
|
||||
var good bool
|
||||
it.first, it.firstDigits, pos, good = readExprNum(s, pos)
|
||||
if !good {
|
||||
return nil, false, false
|
||||
}
|
||||
it.last, it.lastDigits = it.first, it.firstDigits
|
||||
if pos < len(s) && s[pos] == '-' {
|
||||
it.last, it.lastDigits, pos, good = readExprNum(s, pos+1)
|
||||
if !good {
|
||||
return nil, false, false
|
||||
}
|
||||
if pos < len(s) && s[pos] == ':' {
|
||||
if _, _, pos, good = readExprNum(s, pos+1); !good {
|
||||
return nil, false, false
|
||||
}
|
||||
it.hasStep = true
|
||||
}
|
||||
}
|
||||
items = append(items, it)
|
||||
if pos < len(s) && s[pos] == ',' {
|
||||
pos++
|
||||
continue
|
||||
}
|
||||
break
|
||||
}
|
||||
if pos >= len(s) || s[pos] != ']' {
|
||||
return nil, false, false
|
||||
}
|
||||
pos++
|
||||
if pos < len(s) && s[pos] == '%' {
|
||||
var good bool
|
||||
if _, _, pos, good = readExprNum(s, pos+1); !good {
|
||||
return nil, false, false
|
||||
}
|
||||
hasLimit = true
|
||||
}
|
||||
if pos != len(s) {
|
||||
return nil, false, false
|
||||
}
|
||||
return items, hasLimit, true
|
||||
}
|
||||
|
||||
func exprErr(rule, format string, args ...any) (string, int, bool, *ArrayExprError) {
|
||||
return "", 0, false, &ArrayExprError{Rule: rule, Msg: fmt.Sprintf(format, args...)}
|
||||
}
|
||||
|
||||
// ParseSubmitIndexExpr checks a submitted job name that contains '[' or ']'
|
||||
// (names without them keep the existing createArrayName path, 1.4). Only
|
||||
// name[1-N] (1 <= N <= limit) and name[1] are accepted; leading zeros are
|
||||
// allowed (1.5). The rules are checked in requirement order 2.1 .. 2.8 and the
|
||||
// first failure is returned (2.9). On success it returns the array name, the
|
||||
// element count and isArray == true.
|
||||
func ParseSubmitIndexExpr(name string, interactive bool, limit int) (
|
||||
arrayName string, n int, isArray bool, err *ArrayExprError) {
|
||||
// 2.1
|
||||
if interactive {
|
||||
return exprErr("2.1", "job name '%s': interactive jobs cannot be arrays", name)
|
||||
}
|
||||
|
||||
// 2.2: the name rule applies to the part before the first '['
|
||||
// (the whole name if it has no '[').
|
||||
open := -1
|
||||
for i := 0; i < len(name); i++ {
|
||||
if name[i] == '[' {
|
||||
open = i
|
||||
break
|
||||
}
|
||||
}
|
||||
prefix := name
|
||||
if open >= 0 {
|
||||
prefix = name[:open]
|
||||
}
|
||||
if !isValidSubmitArrayName(prefix) {
|
||||
return exprErr("2.2",
|
||||
"job name '%s': the array name is invalid; it must be 1 to %d characters from [a-zA-Z0-9._-]",
|
||||
name, maxArrayNameLen)
|
||||
}
|
||||
var items []exprItem
|
||||
var hasLimit, ok bool
|
||||
if open >= 0 {
|
||||
items, hasLimit, ok = parseExprBody(name[open:])
|
||||
}
|
||||
if !ok {
|
||||
return exprErr("2.2",
|
||||
"job name '%s': the array index expression is malformed; expected name[1-N] or name[1]", name)
|
||||
}
|
||||
|
||||
// 2.3
|
||||
if len(items) > 1 || items[0].hasStep {
|
||||
return exprErr("2.3",
|
||||
"job name '%s': only a single range name[1-N] is supported (lists and steps are not)", name)
|
||||
}
|
||||
// 2.4
|
||||
if hasLimit {
|
||||
return exprErr("2.4", "job name '%s': running limits are not supported (%%N suffix)", name)
|
||||
}
|
||||
first, last := items[0].first, items[0].last
|
||||
// 2.5
|
||||
if first == 0 {
|
||||
return exprErr("2.5", "job name '%s': element numbers start at 1", name)
|
||||
}
|
||||
// 2.6 (compared on the digit strings so oversized numbers keep their order)
|
||||
if digitsLess(items[0].lastDigits, items[0].firstDigits) {
|
||||
return exprErr("2.6", "job name '%s': the range is reversed (last element number is less than the first)", name)
|
||||
}
|
||||
// 2.7
|
||||
if first > 1 {
|
||||
return exprErr("2.7", "job name '%s': only name[1-N] and name[1] are currently supported", name)
|
||||
}
|
||||
// 2.8 (a saturated number is always above the limit)
|
||||
if last > limit || last >= exprNumSaturate {
|
||||
return exprErr("2.8", "job name '%s': the array size exceeds the array size limit of %d", name, limit)
|
||||
}
|
||||
return prefix, last, true, nil
|
||||
}
|
||||
153
src/frontend/apiserver/controllers/jobarray_test.go
Normal file
153
src/frontend/apiserver/controllers/jobarray_test.go
Normal file
|
|
@ -0,0 +1,153 @@
|
|||
package controllers
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestParseSubmitIndexExpr covers requirements 1.5 and 2.1-2.9 for the
|
||||
// submit-time index expression parser (design: ApiSubmitExpr).
|
||||
func TestParseSubmitIndexExpr(t *testing.T) {
|
||||
longName := strings.Repeat("a", 4094)
|
||||
tooLongName := strings.Repeat("a", 4095)
|
||||
|
||||
type want struct {
|
||||
arrayName string
|
||||
n int
|
||||
rule string // "" means success
|
||||
msgHas []string // substrings the error message must contain
|
||||
}
|
||||
cases := []struct {
|
||||
name string
|
||||
input string
|
||||
interactive bool
|
||||
limit int
|
||||
want want
|
||||
}{
|
||||
// --- success (1.1, 1.2) ---
|
||||
{"range 1-N", "arr[1-3]", false, 1000, want{arrayName: "arr", n: 3}},
|
||||
{"single 1", "arr[1]", false, 1000, want{arrayName: "arr", n: 1}},
|
||||
{"range 1-1", "arr[1-1]", false, 1000, want{arrayName: "arr", n: 1}},
|
||||
{"name charset", "a.B_c-9[1-2]", false, 1000, want{arrayName: "a.B_c-9", n: 2}},
|
||||
{"name 4094 chars", longName + "[1-2]", false, 1000, want{arrayName: longName, n: 2}},
|
||||
// --- leading zeros (1.5) ---
|
||||
{"leading zeros range", "arr[001-0003]", false, 1000, want{arrayName: "arr", n: 3}},
|
||||
{"leading zeros single", "arr[01]", false, 1000, want{arrayName: "arr", n: 1}},
|
||||
{"leading zeros limit", "arr[1-0001000]", false, 1000, want{arrayName: "arr", n: 1000}},
|
||||
{"leading zeros zero", "arr[00-3]", false, 1000, want{rule: "2.5"}},
|
||||
// --- limit edge (2.8) ---
|
||||
{"limit exact", "arr[1-1000]", false, 1000, want{arrayName: "arr", n: 1000}},
|
||||
{"limit plus one", "arr[1-1001]", false, 1000, want{rule: "2.8", msgHas: []string{"1000"}}},
|
||||
{"limit 32767 exact", "arr[1-32767]", false, 32767, want{arrayName: "arr", n: 32767}},
|
||||
{"limit 32767 plus one", "arr[1-32768]", false, 32767, want{rule: "2.8", msgHas: []string{"32767"}}},
|
||||
{"limit huge", "arr[1-99999999999999999999]", false, 1000, want{rule: "2.8", msgHas: []string{"1000"}}},
|
||||
{"limit custom", "arr[1-6]", false, 5, want{rule: "2.8", msgHas: []string{"5"}}},
|
||||
// --- 2.1 interactive ---
|
||||
{"interactive array", "arr[1-3]", true, 1000, want{rule: "2.1", msgHas: []string{"interactive jobs cannot be arrays"}}},
|
||||
{"interactive malformed", "arr[", true, 1000, want{rule: "2.1", msgHas: []string{"interactive jobs cannot be arrays"}}},
|
||||
{"interactive bad name", "a b]", true, 1000, want{rule: "2.1"}},
|
||||
// --- 2.2 invalid name ---
|
||||
{"empty name", "[1-3]", false, 1000, want{rule: "2.2", msgHas: []string{"'[1-3]'", "array name is invalid"}}},
|
||||
{"bad char in name", "a*b[1-3]", false, 1000, want{rule: "2.2", msgHas: []string{"'a*b[1-3]'", "array name is invalid"}}},
|
||||
{"space in name", "a b[1-3]", false, 1000, want{rule: "2.2", msgHas: []string{"array name is invalid"}}},
|
||||
{"leading space", " arr[1-3]", false, 1000, want{rule: "2.2", msgHas: []string{"array name is invalid"}}},
|
||||
{"name too long", tooLongName + "[1-2]", false, 1000, want{rule: "2.2", msgHas: []string{"array name is invalid"}}},
|
||||
{"only close bracket bad name", "a*b]", false, 1000, want{rule: "2.2", msgHas: []string{"array name is invalid"}}},
|
||||
{"bad name beats list", "a/b[1,2]", false, 1000, want{rule: "2.2", msgHas: []string{"array name is invalid"}}},
|
||||
// --- 2.2 malformed ---
|
||||
{"only close bracket", "arr]", false, 1000, want{rule: "2.2", msgHas: []string{"'arr]'", "array name is invalid"}}},
|
||||
{"unterminated", "arr[1-3", false, 1000, want{rule: "2.2", msgHas: []string{"'arr[1-3'", "malformed"}}},
|
||||
{"empty brackets", "arr[]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"trailing text", "arr[1-3]x", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"trailing space", "arr[1-3] ", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"inner space", "arr[1 -3]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"sign", "arr[+1-3]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"negative", "arr[-1]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"empty item", "arr[1,,2]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"trailing comma", "arr[1,]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"missing end", "arr[1-]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"missing step", "arr[1-3:]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"step without range", "arr[1:2]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"pct without number", "arr[1-3]%", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"pct trailing", "arr[1-3]%2x", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"two brackets", "arr[1][2]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"nested bracket", "arr[[1]]", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"malformed beats list", "arr[1,2", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
{"malformed beats zero", "arr[0", false, 1000, want{rule: "2.2", msgHas: []string{"malformed"}}},
|
||||
// --- 2.3 lists / steps ---
|
||||
{"list", "arr[1,2]", false, 1000, want{rule: "2.3", msgHas: []string{"only a single range name[1-N] is supported"}}},
|
||||
{"list of ranges", "arr[1-3,5-6]", false, 1000, want{rule: "2.3", msgHas: []string{"only a single range name[1-N] is supported"}}},
|
||||
{"step", "arr[1-10:2]", false, 1000, want{rule: "2.3", msgHas: []string{"only a single range name[1-N] is supported"}}},
|
||||
{"precedence 0-0:2", "arr[0-0:2]", false, 1000, want{rule: "2.3"}},
|
||||
{"list beats pct", "arr[1,2]%3", false, 1000, want{rule: "2.3"}},
|
||||
// --- 2.4 running limit ---
|
||||
{"pct", "arr[1-3]%2", false, 1000, want{rule: "2.4", msgHas: []string{"running limits are not supported"}}},
|
||||
{"pct beats zero", "arr[0-3]%2", false, 1000, want{rule: "2.4"}},
|
||||
// --- 2.5 start 0 ---
|
||||
{"start zero", "arr[0-3]", false, 1000, want{rule: "2.5", msgHas: []string{"element numbers start at 1"}}},
|
||||
{"single zero", "arr[0]", false, 1000, want{rule: "2.5"}},
|
||||
{"zero beats reversed", "arr[0-0]", false, 1000, want{rule: "2.5"}},
|
||||
{"precedence 0-huge", "arr[0-99999999999999999999]", false, 1000, want{rule: "2.5"}},
|
||||
// --- 2.6 reversed ---
|
||||
{"reversed", "arr[3-1]", false, 1000, want{rule: "2.6", msgHas: []string{"reversed"}}},
|
||||
{"precedence huge-5", "arr[99999999999999999999-5]", false, 1000, want{rule: "2.6"}},
|
||||
{"reversed beats limit", "arr[5000-2000]", false, 1000, want{rule: "2.6"}},
|
||||
// --- 2.7 start > 1 ---
|
||||
{"single 5", "arr[5]", false, 1000, want{rule: "2.7", msgHas: []string{"only name[1-N] and name[1] are currently supported"}}},
|
||||
{"start above 1", "arr[1001-2000]", false, 1000, want{rule: "2.7", msgHas: []string{"only name[1-N] and name[1] are currently supported"}}},
|
||||
{"start 2", "arr[2-3]", false, 1000, want{rule: "2.7"}},
|
||||
{"precedence huge single", "arr[99999999999999999999]", false, 1000, want{rule: "2.7"}},
|
||||
{"huge range equal", "arr[99999999999999999999-99999999999999999999]", false, 1000, want{rule: "2.7"}},
|
||||
{"huge reversed two oversized", "arr[99999999999999999999-99999999999999999998]", false, 1000, want{rule: "2.6"}},
|
||||
{"huge reversed just above 2^40", "arr[1099511627777-1099511627776]", false, 1000, want{rule: "2.6"}},
|
||||
{"huge ascending two oversized", "arr[99999999999999999998-99999999999999999999]", false, 1000, want{rule: "2.7"}},
|
||||
{"huge equal after leading zeros", "arr[0099999999999999999999-99999999999999999999]", false, 1000, want{rule: "2.7"}},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
arrayName, n, isArray, err := ParseSubmitIndexExpr(tc.input, tc.interactive, tc.limit)
|
||||
if tc.want.rule == "" {
|
||||
if err != nil {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) unexpected error: rule %s: %s", tc.input, err.Rule, err.Msg)
|
||||
}
|
||||
if !isArray || arrayName != tc.want.arrayName || n != tc.want.n {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) = (%q, %d, %v), want (%q, %d, true)",
|
||||
tc.input, arrayName, n, isArray, tc.want.arrayName, tc.want.n)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) = (%q, %d, %v, nil), want rule %s",
|
||||
tc.input, arrayName, n, isArray, tc.want.rule)
|
||||
}
|
||||
if err.Rule != tc.want.rule {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) rule = %s (%s), want %s", tc.input, err.Rule, err.Msg, tc.want.rule)
|
||||
}
|
||||
if isArray || n != 0 || arrayName != "" {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) returned (%q, %d, %v) alongside an error", tc.input, arrayName, n, isArray)
|
||||
}
|
||||
if err.Error() != err.Msg {
|
||||
t.Fatalf("Error() = %q, want Msg %q", err.Error(), err.Msg)
|
||||
}
|
||||
for _, s := range tc.want.msgHas {
|
||||
if !strings.Contains(err.Msg, s) {
|
||||
t.Fatalf("ParseSubmitIndexExpr(%q) message %q does not contain %q", tc.input, err.Msg, s)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseSubmitIndexExprErrorAsError checks a non-nil *ArrayExprError
|
||||
// satisfies the error interface (used by preCheckJobRequest / errors.As).
|
||||
func TestParseSubmitIndexExprErrorAsError(t *testing.T) {
|
||||
_, _, _, ae := ParseSubmitIndexExpr("arr[0]", false, 1000)
|
||||
if ae == nil {
|
||||
t.Fatal("expected an error for arr[0]")
|
||||
}
|
||||
var e error = ae
|
||||
if !strings.Contains(e.Error(), "element numbers start at 1") {
|
||||
t.Fatalf("Error() = %q", e.Error())
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue