Skip to content

ingest: a single-object body silently drops everything after the first object #561

Description

@EricAndrechek

What

Declare application/json and POST a body with more than one object in it:

POST /v1/ingest?table=clicks
Content-Type: application/json

{"page":"/first"}{"page":"/second"}

Response is 200 {"ok":true}. One record is published. The rest of the body
is discarded, and nothing in the response, the logs, or a counter says so.

Pinned today by TestIngest_Unlabeled_ConcatenatedObjects_FirstOnly
(internal/api/ingest_test.go), which asserts exactly this: 200, ok: true,
one message containing /first, and no /second.

Why it is worth an issue

This is pre-existing and deliberately preserved — it is the historical
single-object behavior, and #552 kept it rather than changing arity handling in
a PR about headers. It is not a regression.

But #552 spent its whole diff closing the accidental routes to this same
harm. A proxy joining two Content-Type headers, or repeating the header line
with a different value, both used to resolve to application/json silently and
truncate an NDJSON batch to its first record. Both are now refused. The
argument for refusing them is that silent record loss is not an acceptable
outcome — and that argument does not stop at the accidental routes. A caller
who declares application/json and sends NDJSON reaches the identical outcome
directly, with the header guards working exactly as designed.

So the endpoint now holds two positions at once: silent truncation is
unacceptable when reached via a malformed header, and fine when reached via a
well-formed one.

Suggested fix

After decoding the single top-level object, require the remainder of the body
to be whitespace, and reject with 400 naming the problem otherwise —
something like "trailing content after the JSON object; send
application/x-ndjson to ingest multiple records". json.Decoder already
tracks the offset, so the check is cheap and needs no second parse.

That would be a behavior change for anyone relying on the current
truncation, which is why it does not belong in #552: that PR's contract is that
the only behavior changes are the ones its CHANGELOG entry names. Worth its own
change with its own entry.

Provenance

Raised in the chtypes-side review of #552, filed rather than fixed in-branch.
Related: #544 (in-flight ingest byte budget), also deliberately deferred out of
this stack.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions