diff --git a/internal/server/decode_json_nul_test.go b/internal/server/decode_json_nul_test.go index 331971678..7392a98a2 100644 --- a/internal/server/decode_json_nul_test.go +++ b/internal/server/decode_json_nul_test.go @@ -1024,3 +1024,29 @@ func TestOAuthRegisterRefusesNULBody(t *testing.T) { t.Errorf("the refusal should name the cause, got %s", rr.Body.String()) } } + +// TestBodyDecodesNULMatchesKeysLikeTheDecoder is codex round 16: encoding/json +// matches an incoming key to a struct field by an exact match first and a +// CASE-INSENSITIVE one otherwise, so `{"Fields":...}` lands in +// ItemCreate.Fields exactly as `{"fields":...}` does. A case-sensitive lookup +// in the walk skipped the nested check for a body the handler then accepted, +// and the database answered the original 500. +// +// Measured before the fix: `fields` refused, `Fields` and `FIELDS` accepted. +func TestBodyDecodesNULMatchesKeysLikeTheDecoder(t *testing.T) { + inner := `{"k":"a` + escNULLiteral + `b"}` + for _, key := range []string{"fields", "Fields", "FIELDS", "fIeLdS", "Schema", "TAGS"} { + body := `{"title":"x","` + key + `":` + jsonEncode(t, inner) + `}` + if !bodyDecodesNUL([]byte(body)) { + t.Errorf("key %q reaches the same struct field as its lower-case spelling and must be "+ + "walked the same way", key) + } + } + + // Control: a key that is not a wire key in ANY casing stays caller data, + // so this is case-insensitive matching rather than matching everything. + notAKey := `{"title":"x","Notes":` + jsonEncode(t, inner) + `}` + if bodyDecodesNUL([]byte(notAKey)) { + t.Error("an unlisted key must not be treated as JSON-encoded whatever its casing") + } +} diff --git a/internal/server/middleware_request_text.go b/internal/server/middleware_request_text.go index b38af233f..2895a3e97 100644 --- a/internal/server/middleware_request_text.go +++ b/internal/server/middleware_request_text.go @@ -478,6 +478,28 @@ var jsonEncodedFieldKeys = map[string]bool{ "traits": true, } +// isJSONEncodedFieldKey matches a wire key the way the DECODER that consumes +// it does: case-insensitively. +// +// encoding/json matches an incoming key to a struct field by an exact match +// first and a CASE-INSENSITIVE one otherwise, so `{"Fields":...}` and +// `{"FIELDS":...}` both land in ItemCreate.Fields. A case-SENSITIVE lookup +// here therefore skipped the nested walk for a body the handler went on to +// accept, and the database answered the original 500 (codex round 16, +// measured: `fields` refused, `Fields` and `FIELDS` accepted). +// +// This is the same defect shape as the rest of BUG-2803 — a check that agrees +// with one layer's rules while the layer that actually consumes the value +// uses different ones — and it is the reason this predicate is a function +// rather than a bare map index: the map is the vocabulary, the MATCHING RULE +// belongs to the consumer. +func isJSONEncodedFieldKey(k string) bool { + if jsonEncodedFieldKeys[k] { + return true + } + return jsonEncodedFieldKeys[strings.ToLower(k)] +} + // valueDecodesNUL walks a decoded request body for a string that contains a // NUL, descending into a JSON document carried as a string exactly once. // @@ -520,7 +542,7 @@ func valueDecodesNUL(v any, inUserData bool) bool { if strings.ContainsRune(k, 0) { return true } - if !inUserData && jsonEncodedFieldKeys[k] { + if !inUserData && isJSONEncodedFieldKey(k) { if str, isString := sub.(string); isString { // BOTH checks. The nested walk answers "is there an // escape inside the document this string carries"; it