Skip to content

Reject duplicate JSON object keys in request bodies #939

Description

@hanzei

Background

Go 1.27 (#938) backs encoding/json with the new encoding/json/v2 implementation. v1 semantics are preserved, so nothing changed on the wire — but 1.27 also exposes composable Options that let us opt into individual v2 strictness rules without migrating to the v2 API.

One of those is worth taking: rejecting duplicate object member names. Today a request body like

{"title":"first","title":"second"}

is accepted, and the last value silently wins. That is v1 behaviour and is the JSON spec's undefined-behaviour corner. It fits the "unintentional internal overload" half of the threat model in CLAUDE.md — a client with a payload-construction bug (mobile's offline replay queue building a patch twice, say) currently gets a silent last-write-wins instead of a clear 400.

Proposal

Decoding happens in exactly one place, server/internal/handlers/validation.go:20:

return json.NewDecoder(r.Body).Decode(v)

Switch that to jsonv2.UnmarshalRead with v1 semantics retained everywhere except duplicate names:

strict := jsonv2.JoinOptions(
    jsonv1.DefaultOptionsV1(),
    jsontext.AllowDuplicateNames(false),
)

Then map the resulting error to a 400 alongside the existing malformed-body handling.

Why this combination

Verified on Go 1.27 against Jot's actual response shapes:

  • Marshal output is byte-identical to v1. null stays null for nil slices, omitempty keeps v1 semantics (so false/0 stay omitted). No client sees a payload change.
  • Case-insensitive field matching is retained. Bare v2 makes matching case-sensitive and then silently drops mismatched fields with no error — the one v2 behaviour that could cause quiet data loss. DefaultOptionsV1() keeps v1 matching.
  • Duplicate names error. jsontext: duplicate object member name "id".

So the blast radius is limited to requests that are already malformed.

Scope

  • server/internal/handlers/validation.go — the decode call and error mapping
  • A test asserting a duplicate-key body returns 400 rather than last-write-wins
  • Encoding (server/internal/server/server.go:474) is deliberately not touched — this is request-side hardening only

Explicitly out of scope

Full migration to the encoding/json/v2 API. That is a separate, API-breaking discussion: it flips nil slices from null to [] (which would actually fix Note.Labels, since shared/src/types.ts declares labels: Label[] as non-nullable), always emits bool/number fields tagged omitempty (7 such tags today, mostly MCP tool inputs), and makes field matching case-sensitive. It needs a coordinated webapp/mobile change and the API-breaking callout CLAUDE.md requires.

One prerequisite check before anyone attempts that larger migration: confirm no client sends case-mismatched keys. The TS type declarations line up with the Go tags, but every axios call site has not been audited.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions