Require an explicit payload on POST /api/v1/jobs
An omitted payload was silently replaced with {}, contrary to the
fail-fast and no-silent-fallback rules. It is now refused with 400
invalid_request "payload: required"; a null payload stays refused.
Kinds that take no input, such as noop, are sent {}.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+10
-9
@@ -17,8 +17,8 @@ import (
|
|||||||
// maxJobBody bounds a POST /jobs request body.
|
// maxJobBody bounds a POST /jobs request body.
|
||||||
const maxJobBody = 64 << 10
|
const maxJobBody = 64 << 10
|
||||||
|
|
||||||
// jobRequest is the POST /jobs body. Payload is optional; an omitted
|
// jobRequest is the POST /jobs body. Payload is required; a job kind that
|
||||||
// payload is the empty object.
|
// takes no input is sent the empty object {}.
|
||||||
type jobRequest struct {
|
type jobRequest struct {
|
||||||
Kind string `json:"kind"`
|
Kind string `json:"kind"`
|
||||||
Payload json.RawMessage `json:"payload"`
|
Payload json.RawMessage `json:"payload"`
|
||||||
@@ -55,8 +55,9 @@ func postJob(logger *slog.Logger, runner *jobs.Runner) http.HandlerFunc {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// decodeJobRequest reads a single JSON object with a non-empty kind,
|
// decodeJobRequest reads a single JSON object with a non-empty kind and a
|
||||||
// refusing unknown fields, a null payload and trailing data.
|
// payload, refusing unknown fields, an omitted or null payload and trailing
|
||||||
|
// data.
|
||||||
func decodeJobRequest(body io.Reader) (jobRequest, error) {
|
func decodeJobRequest(body io.Reader) (jobRequest, error) {
|
||||||
data, err := io.ReadAll(body)
|
data, err := io.ReadAll(body)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -70,7 +71,7 @@ func decodeJobRequest(body io.Reader) (jobRequest, error) {
|
|||||||
dec := json.NewDecoder(bytes.NewReader(data))
|
dec := json.NewDecoder(bytes.NewReader(data))
|
||||||
dec.DisallowUnknownFields()
|
dec.DisallowUnknownFields()
|
||||||
if err := dec.Decode(&req); err != nil {
|
if err := dec.Decode(&req); err != nil {
|
||||||
return jobRequest{}, errors.New("request body must be a JSON object with kind and an optional payload")
|
return jobRequest{}, errors.New("request body must be a JSON object with kind and payload")
|
||||||
}
|
}
|
||||||
if _, err := dec.Token(); err != io.EOF {
|
if _, err := dec.Token(); err != io.EOF {
|
||||||
return jobRequest{}, errors.New("request body must be a single JSON object")
|
return jobRequest{}, errors.New("request body must be a single JSON object")
|
||||||
@@ -78,10 +79,10 @@ func decodeJobRequest(body io.Reader) (jobRequest, error) {
|
|||||||
if req.Kind == "" {
|
if req.Kind == "" {
|
||||||
return jobRequest{}, errors.New("kind: required")
|
return jobRequest{}, errors.New("kind: required")
|
||||||
}
|
}
|
||||||
switch {
|
if req.Payload == nil {
|
||||||
case req.Payload == nil:
|
return jobRequest{}, errors.New("payload: required")
|
||||||
req.Payload = json.RawMessage(`{}`)
|
}
|
||||||
case bytes.Equal(req.Payload, []byte("null")):
|
if bytes.Equal(req.Payload, []byte("null")) {
|
||||||
return jobRequest{}, errors.New("payload: must not be null")
|
return jobRequest{}, errors.New("payload: must not be null")
|
||||||
}
|
}
|
||||||
return req, nil
|
return req, nil
|
||||||
|
|||||||
@@ -183,24 +183,6 @@ func TestPostJobThenGetProgressAndResult(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestPostJobWithoutPayloadEnqueues(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
release := make(chan struct{})
|
|
||||||
close(release)
|
|
||||||
runner, _ := startedRunner(t, release, nil)
|
|
||||||
router := newTestRouterWith(t, "dev", newTestSettings(t), runner)
|
|
||||||
|
|
||||||
rec, body := serve(t, router, signedIn(http.MethodPost, "/api/v1/jobs", strings.NewReader(`{"kind":"draft"}`)))
|
|
||||||
if rec.Code != http.StatusAccepted {
|
|
||||||
t.Fatalf("status = %d, want 202: %s", rec.Code, rec.Body)
|
|
||||||
}
|
|
||||||
id, _ := body["id"].(string)
|
|
||||||
done := pollJob(t, router, id, func(b map[string]any) bool { return b["status"] == "succeeded" })
|
|
||||||
if resultDays(done) != 7 {
|
|
||||||
t.Errorf("finished body = %v, want result {\"days\":7}", done)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestPostJobRefusesBadRequests(t *testing.T) {
|
func TestPostJobRefusesBadRequests(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
runner, _ := startedRunner(t, nil, nil)
|
runner, _ := startedRunner(t, nil, nil)
|
||||||
@@ -209,15 +191,16 @@ func TestPostJobRefusesBadRequests(t *testing.T) {
|
|||||||
for _, tc := range []struct {
|
for _, tc := range []struct {
|
||||||
name, body, code, message string
|
name, body, code, message string
|
||||||
}{
|
}{
|
||||||
{"unknown kind", `{"kind":"no_such_kind"}`, "unknown_kind", "unknown job kind"},
|
{"unknown kind", `{"kind":"no_such_kind","payload":{}}`, "unknown_kind", "unknown job kind"},
|
||||||
{"payload for another type", `{"kind":"draft","payload":{"days":3}}`, "invalid_payload", "payload does not match the job kind"},
|
{"payload for another type", `{"kind":"draft","payload":{"days":3}}`, "invalid_payload", "payload does not match the job kind"},
|
||||||
{"payload not an object", `{"kind":"draft","payload":[1]}`, "invalid_payload", "payload does not match the job kind"},
|
{"payload not an object", `{"kind":"draft","payload":[1]}`, "invalid_payload", "payload does not match the job kind"},
|
||||||
|
{"missing payload", `{"kind":"draft"}`, "invalid_request", "payload: required"},
|
||||||
{"null payload", `{"kind":"draft","payload":null}`, "invalid_request", "payload: must not be null"},
|
{"null payload", `{"kind":"draft","payload":null}`, "invalid_request", "payload: must not be null"},
|
||||||
{"missing kind", `{"payload":{}}`, "invalid_request", "kind: required"},
|
{"missing kind", `{"payload":{}}`, "invalid_request", "kind: required"},
|
||||||
{"empty body", ``, "invalid_request", "request body must be a JSON object with kind and an optional payload"},
|
{"empty body", ``, "invalid_request", "request body must be a JSON object with kind and payload"},
|
||||||
{"not an object", `["draft"]`, "invalid_request", "request body must be a JSON object with kind and an optional payload"},
|
{"not an object", `["draft"]`, "invalid_request", "request body must be a JSON object with kind and payload"},
|
||||||
{"unknown field", `{"kind":"draft","extra":1}`, "invalid_request", "request body must be a JSON object with kind and an optional payload"},
|
{"unknown field", `{"kind":"draft","payload":{},"extra":1}`, "invalid_request", "request body must be a JSON object with kind and payload"},
|
||||||
{"trailing data", `{"kind":"draft"} {}`, "invalid_request", "request body must be a single JSON object"},
|
{"trailing data", `{"kind":"draft","payload":{}} {}`, "invalid_request", "request body must be a single JSON object"},
|
||||||
} {
|
} {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
@@ -240,7 +223,7 @@ func TestPostJobRefusesBadRequests(t *testing.T) {
|
|||||||
func TestPostJobNeedsSessionAndCSRF(t *testing.T) {
|
func TestPostJobNeedsSessionAndCSRF(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
router := newTestRouter(t, "dev")
|
router := newTestRouter(t, "dev")
|
||||||
const body = `{"kind":"noop"}`
|
const body = `{"kind":"noop","payload":{}}`
|
||||||
|
|
||||||
rec := httptest.NewRecorder()
|
rec := httptest.NewRecorder()
|
||||||
router.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/v1/jobs", strings.NewReader(body)))
|
router.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/api/v1/jobs", strings.NewReader(body)))
|
||||||
|
|||||||
Reference in New Issue
Block a user