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:
Rongsong Shen 2026-10-01 11:47:23 +00:00
parent 3a7e3890e2
commit 0318ac5f44
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
3 changed files with 359 additions and 1 deletions

View file

@ -121,7 +121,7 @@
- _Depends: 5.1_
- [ ] 6. API server: array submission
- [ ] 6.1 (P) Parse submit index expressions
- [x] 6.1 (P) Parse submit index expressions
- Accept only `name[1-N]` and `name[1]`; reject interactive arrays, malformed expressions or invalid names (quoting the name), lists and steps, `%N`, start 0, reversed ranges, start above 1 and sizes above the limit, each with its specific message, reporting only the lowest-numbered broken rule
- Saturate oversized numbers so the rule order still decides; accept leading zeros
- Done when the index-expression unit tests pass for every rule, the precedence rows (`[0-0:2]`, `[99999999999999999999]`, `[99999999999999999999-5]`, `[0-99999999999999999999]`), leading zeros, `[1]` vs `[1-1]` vs `[5]` and the limit edge
@ -245,3 +245,4 @@
- 4.3: the controller acks `setpend` (`do_set_preason`, job.c ~3869-3880) and `jobrun` (`handle_jobdispatch`, job.c ~2246-2258) with status 0 before doing the work, so those two harness commands can never report not-found; 4.7/5.13 for them depend on a controller change outside this spec's boundary. `jobinfo`'s not-found is `rc == 0` with no row (`do_reply_joblist`), not -ENOENT.
- 5.1: Go helpers: `ParseHumanJobIDStrict(s) (uint64, error)` returns sentinel `ErrJobIDSyntax` / `ErrJobIDWidth` (callers trim and split `[...]` first; no whitespace accepted); `MakeElementID(key uint64, k int) uint64`; `IsArrayCapableKey(key)`; `GenerateArrayJobID(c, a, n)` (non-cgo stub returns errNoCGO).
- 5.4: handler fixture in `controllers/jobs_handler_test.go` (`newHandlerFixture`, `seedJob`/`seedOrdinaryJob`/`seedArrayElements`/`seedQueue`, `submit`, `controlByID`, `controlByFilter`, `dispatchedIDs`, `submissions`); submit bodies must be client-style JSON maps (marshalled `models.JobSubmitRequest` breaks on zero CustomTime). Handlers are registered without `RequirePermission`; in-handler RBAC/queue/status checks still run.
- 6.1: the design's "saturate oversized numbers" rule alone loses the order of two oversized numbers; `ParseSubmitIndexExpr` compares leading-zero-stripped digit strings for 2.6. Use the same approach for the selection parsers' reversed-range check (7.1, 8.1) if they saturate. Rule tags are strings "2.1".."2.8" on `*ArrayExprError`.

View 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
}

View 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())
}
}