mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-26 06:19:49 +00:00
fix(server): match wire keys the way the decoder does (BUG-2803)
Codex round 16, asked whether this change is consistent with its siblings
in the same file and extensible by someone who did not write it. It found
a live bypass instead.
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":...} land in ItemCreate.Fields exactly as {"fields":...} does.
The walk looked the key up case-SENSITIVELY, so it skipped the nested
document for a body the handler went on to accept, and the database
answered the original 500.
Measured before the fix: `fields` refused, `Fields` and `FIELDS`
accepted.
This is the same defect shape as everything else in this unit — a check
that agrees with one layer's rules while the layer that actually consumes
the value uses different ones — which is why the fix is a PREDICATE
rather than a wider map: the map is the vocabulary, and the matching RULE
belongs to the consumer. Someone adding a key should not also have to
remember to add its spellings.
The test drives six spellings including mixed case, with a control
asserting an unlisted key stays caller data in any casing, so this is
case-insensitive matching rather than matching everything.
Claude-Session: https://claude.ai/code/session_011T365kP1N9V88y15HxL4YN
This commit is contained in:
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user