fix(apiserver): reject a malformed rusage[] at job submit (bug-1744)

Gitea #47: `xcecli jobs submit -R "rusage[mem=100000000a0]"` was accepted
with a normal "Job <...> is submitted" reply. The apiserver only
length-checked resreq; the real PEG grammar ran later, asynchronously, in
the scheduler's core_build_job(), which logged a server-side WARN and
dropped the job -- it then sat PEND forever with no sign of why.

- libxce: xce_check_rusage_syntax(resreq, &errmsg) runs parse_rusage() on
  every rusage[...] clause (extracted the way section.peg does: up to the
  first ']'), so primary, compound and alternative requests are all
  covered. No resource manager needed, so select[]/order[] resource names
  are still only checked by the scheduler (the documented scope).
- apiserver/backend: ValidateResreqSyntax() cgo wrapper (resreq_check.go);
  the !cgo stub returns nil (no job can be submitted without CGO anyway).
- controllers/jobs.go preCheckJobRequest(): calls it after the length
  check via a package var, so a malformed request is refused up front.

Tests: lib test_check_rusage_syntax (RED before: the reported input was
accepted) and backend TestValidateResreqSyntax (RED before) through the
real grammar; controllers TestPreCheckJobRequest_RejectsMalformedResreq
checks the call site with an injected validator (written with the call
site, so no separate RED). libxce, apiserver backend (cgo) and controllers
tests pass. apiserver/middleware TestLoadJobs fails here and on the
baseline alike: it needs a live apiserver on :8080.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/frontend/apiserver/backend/clib_test.go

@@ -286,3 +286,20 @@ func TestClibSource_ClusterAndClientConfigsArePackageLevel(t *testing.T) {
 			"runtime life (see status report CONCERNS for the precise finding)")
 	}
 }
+
+// TestValidateResreqSyntax (bug-1744): the real libxce rusage grammar accepts
+// well-formed requests and rejects the reported malformed one.
+func TestValidateResreqSyntax(t *testing.T) {
+	for _, ok := range []string{"", "rusage[mem=100]",
+		"select[type==any] rusage[mem=100, swp=2]", "select[type==any]"} {
+		if err := ValidateResreqSyntax(ok); err != nil {
+			t.Errorf("%q: unexpected error %v", ok, err)
+		}
+	}
+	for _, bad := range []string{"rusage[mem=100000000a0]",
+		"select[type==any] rusage[mem=]", "rusage[mem=1"} {
+		if err := ValidateResreqSyntax(bad); err == nil {
+			t.Errorf("%q: expected a syntax error", bad)
+		}
+	}
+}

src/frontend/apiserver/backend/resreq_check.go

@@ -0,0 +1,41 @@
+//go:build cgo
+
+package backend
+
+/*
+#include <stdlib.h>
+#include "resreq.h"
+*/
+import "C"
+
+import (
+	"fmt"
+	"unsafe"
+)
+
+// ValidateResreqSyntax (bug-1744) checks every rusage[...] clause of a job's
+// resource request with libxce's real rusage grammar (xce_check_rusage_syntax,
+// src/lib/resreq.c) before the job is accepted. The scheduler parses the full
+// request only later, asynchronously, and silently drops a job it cannot
+// parse -- after the submitter was already told it was accepted. Needs no
+// resource manager, so select[]/order[] resource names are not checked here.
+func ValidateResreqSyntax(resreq string) error {
+	if resreq == "" {
+		return nil
+	}
+
+	cs := C.CString(resreq)
+	defer C.free(unsafe.Pointer(cs))
+
+	var cerr *C.char
+	if rc := C.xce_check_rusage_syntax(cs, &cerr); rc < 0 {
+		msg := "malformed clause"
+		if cerr != nil {
+			msg = C.GoString(cerr)
+			C.free(unsafe.Pointer(cerr))
+		}
+		return fmt.Errorf("invalid rusage in resource request %q: %s",
+			resreq, msg)
+	}
+	return nil
+}

src/frontend/apiserver/backend/stub.go

@@ -46,3 +46,8 @@ func Cleanup() {}
 func FetchFile(_ uint64, _ string, _ uint64, _ uint32) (*FetchFileResult, error) {
 	return nil, errNoCGO
 }
+
+// ValidateResreqSyntax (bug-1744) needs libxce's grammar parser; without CGO
+// no job can be submitted anyway (SubmitJob returns errNoCGO), so there is
+// nothing to check.
+func ValidateResreqSyntax(string) error { return nil }

src/frontend/apiserver/controllers/jobs.go

@@ -1530,6 +1530,10 @@ func preCheckJobRestartRequest(req *models.JobRestartRequest) error {
 	return nil
 }
 
+// validateResreqSyntax is the resource-request syntax check used by
+// preCheckJobRequest; a variable so tests can substitute it (bug-1744).
+var validateResreqSyntax = backend.ValidateResreqSyntax
+
 func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) {
 	logger.Debug("Enter preCheckJobRequest")
 	if req.QueueName == "" {
@@ -1603,6 +1607,11 @@ func preCheckJobRequest(req *models.JobSubmitRequest) ([]int, error) {
 		if len(req.Resreq) > 4094 {
 			return nil, errors.New("resreq length exceeds the limit of 4094 characters")
 		}
+		// bug-1744: reject a malformed rusage[] clause now, instead of
+		// accepting the job and letting the scheduler silently drop it.
+		if err := validateResreqSyntax(req.Resreq); err != nil {
+			return nil, err
+		}
 	}
 
 	if req.ShellLimit.CPULimit != 0 {

src/frontend/apiserver/controllers/jobs_test.go

@@ -1,6 +1,7 @@
 package controllers
 
 import (
+	"errors"
 	"apiserver/models"
 	"apiserver/store"
 	"apiserver/utils"
@@ -192,3 +193,32 @@ func TestFoundJob_MatchesRealArrayIdxType(t *testing.T) {
 		t.Fatalf("expected to find 1 job, got %d", len(found))
 	}
 }
+
+// TestPreCheckJobRequest_RejectsMalformedResreq (bug-1744): a resource request
+// the syntax check rejects must fail the submission instead of being accepted
+// and later silently dropped by the scheduler.
+func TestPreCheckJobRequest_RejectsMalformedResreq(t *testing.T) {
+	resetJobSubmitState()
+	store.Queues.Put("normal", models.QueueInfo{QueueName: "normal"})
+
+	var checked string
+	orig := validateResreqSyntax
+	validateResreqSyntax = func(s string) error {
+		checked = s
+		if strings.Contains(s, "100000000a0") {
+			return errors.New("invalid rusage in resource request")
+		}
+		return nil
+	}
+	defer func() { validateResreqSyntax = orig }()
+
+	req := &models.JobSubmitRequest{QueueName: "normal",
+		Resreq: "rusage[mem=100000000a0]"}
+	_, err := preCheckJobRequest(req)
+	if checked != "rusage[mem=100000000a0]" {
+		t.Fatalf("resreq was not syntax-checked (checked %q)", checked)
+	}
+	if err == nil || !strings.Contains(err.Error(), "invalid rusage") {
+		t.Fatalf("malformed resreq must be rejected, got err=%v", err)
+	}
+}

src/include/resreq.h

@@ -305,6 +305,12 @@ extern void kill_section_ctx(section_context_t*);
 extern void kill_resreq_data(xce_resreq_data_t*);
 extern int  parse_section(xce_resreq_data_t*, char*, char**);
 extern int  parse_rusage(xlist_t*, char*, char**);
+/* bug-1744: syntax-check every rusage[...] clause of a resource-request
+ * string with the real rusage grammar -- no resource manager needed, for
+ * front ends validating a submission before accepting it. Returns 1 if all
+ * clauses parse (or there are none), else -EINVAL with *errmsg set (caller
+ * frees). */
+extern int  xce_check_rusage_syntax(const char*, char**);
 extern void kill_rusage_value(rusage_value_t*);
 extern int  parse_resreq_primary(xce_primary_resreq_t*, char*, var, char**);
 extern void kill_primary_resreq(xce_primary_resreq_t*);

src/lib/resreq.c

@@ -185,6 +185,68 @@ int parse_rusage(xlist_t* rusages, char* data, char** errmsg)
     return rc;
 }
 
+/* bug-1744: see resreq.h. Mirrors section.peg's own extraction -- a clause
+ * runs from "rusage[" to the first "]" -- so every rusage clause of a
+ * primary, compound or alternative request is checked exactly as the
+ * scheduler's parse_resreq() will later parse it. */
+int xce_check_rusage_syntax(const char* resreq, char** errmsg)
+{
+    const char* p;
+
+    if ((resreq == NULL) || (errmsg == NULL)) {
+        return -EINVAL;
+    }
+    *errmsg = NULL;
+
+    for (p = strstr(resreq, "rusage["); p != NULL;
+         p = strstr(p, "rusage[")) {
+        const char* start = p + strlen("rusage[");
+        const char* end   = strchr(start, ']');
+        char*       body;
+        const char* q;
+        xlist_t     rusages;
+        int         rc, i;
+
+        if (end == NULL) {
+            *errmsg = strdup("unterminated rusage[ clause");
+            return -EINVAL;
+        }
+
+        for (q = start; (q < end) && ((*q == ' ') || (*q == '\t')); q++)
+            ;
+        if (q == end) { /* empty clause: parse_resreq_primary skips it too */
+            p = end + 1;
+            continue;
+        }
+
+        body = strndup(start, (size_t)(end - start));
+        if (body == NULL) {
+            return -ENOMEM;
+        }
+
+        xlist_init(&rusages, 4);
+        rc = parse_rusage(&rusages, body, errmsg);
+        for (i = 0; i < rusages.count; i++) {
+            rusage_value_t* v = (rusage_value_t*)xlist_get(&rusages, i);
+
+            kill_rusage_value(v);
+            FREEUP(v);
+        }
+        xlist_fini(&rusages);
+        free(body);
+
+        if (rc < 0) {
+            if (*errmsg == NULL) {
+                *errmsg = strdup("invalid rusage[] clause");
+            }
+            return rc;
+        }
+        p = end + 1;
+    }
+
+    return 1;
+}
+
 /* Depth pre-scan: count '!' and '(' as depth increments.
  * Returns 0 if depth is within limit, -1 if exceeded. */
 static int check_select_depth(const char* input, int max_depth)

src/lib/testsuite/test_resreq.c

@@ -1,3 +1,4 @@
+#include <errno.h>
 #include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -481,11 +482,49 @@ void test_parse_section_span_not_in_select(void)
     FREEUP(errmsg);
 }
 
+/* bug-1744: the apiserver's pre-submit check. Valid rusage clauses (and
+ * requests with none) pass; a malformed one -- the reported
+ * "rusage[mem=100000000a0]" -- is rejected with a message. */
+void test_check_rusage_syntax(void)
+{
+    const char* good[] = {
+        "rusage[mem=100]",
+        "select[type==any] rusage[mem=100, swp=2]",
+        "select[type==any] order[r15m]",
+        "",
+        "{rusage[mem=1]} || {rusage[mem=2]}",
+    };
+    const char* bad[] = {
+        "rusage[mem=100000000a0]",
+        "select[type==any] rusage[mem=]",
+        "{rusage[mem=1]} || {rusage[mem=x]}",
+        "rusage[mem=1",
+    };
+    char*  errmsg;
+    size_t i;
+
+    for (i = 0; i < sizeof(good) / sizeof(good[0]); i++) {
+        errmsg = NULL;
+        TEST_ASSERT_EQUAL_INT_MESSAGE(1, xce_check_rusage_syntax(good[i], &errmsg),
+                                      good[i]);
+        TEST_ASSERT_NULL(errmsg);
+    }
+    for (i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) {
+        errmsg = NULL;
+        TEST_ASSERT_EQUAL_INT_MESSAGE(-EINVAL,
+                                      xce_check_rusage_syntax(bad[i], &errmsg),
+                                      bad[i]);
+        TEST_ASSERT_NOT_NULL_MESSAGE(errmsg, bad[i]);
+        free(errmsg);
+    }
+}
+
 void test_resreq(void)
 {
     printf("\n>> resreq test\n\n");
 
     RUN_TEST(test_parse_section1);
+    RUN_TEST(test_check_rusage_syntax);
     RUN_TEST(test_parse_section2);
     RUN_TEST(test_parse_section_fail);
     RUN_TEST(test_parse_section3);