Skip to content

Refuse a write the field cannot hold, instead of storing "[object Object]" - #26

Closed
pallaoro wants to merge 4 commits into
mainfrom
reject-lossy-writes
Closed

pallaoro wants to merge 4 commits into
mainfrom
reject-lossy-writes

Conversation

@pallaoro

@pallaoro pallaoro commented Sep 4, 2026

Copy link
Copy Markdown
Member

A write this API accepted with a 200 could destroy the value it was given.

String() does not fail, it succeeds with garbage. The entry writer coerced
every incoming value to the column's type, and String({a:1}) is
"[object Object]", String(["a","b"]) is "a,b", Number("abc") is NaN.
So sending an object to a string field returned 200 with a body that already
showed the damage, and the original payload was gone. Objects and arrays now
reach only the two field types that hold structure, json and richtext;
anything else is a 400 naming the fix. Non-numeric input to integer /
decimal is a 400 instead of a stored NaN, and the platform notes column
takes prose only, so a brief cannot be filed as "[object Object]".

The same coercion in a column default is worse, and permanent. A default is
not written through the entry API — it is rendered into the column's DDL and
then applied by SQLite to every row that omits the field. A json field
declared with default: { a: 1 } produced:

"blob" TEXT DEFAULT '[object Object]'

and SQLite has no DDL that changes a column default, so taking one back means
rebuilding the table. Two changes close it from both sides: the content-type routes refuse an object default on a field
that cannot hold one (POST and PATCH alike), and the DDL writer serializes
structure rather than stringifying it. The DDL side is deliberately lossless
rather than strict, because it runs during boot schema-sync over content types
that are already stored — throwing there would turn a bad row written before
this check into an app that will not start.

The rule is also appended to the write routes' descriptions, so it lands in
/api/openapi.json before a client writes, not only in the 400 afterwards.

Verified

The real app, booted against a real SQLite database through the storage
binding, so schema-sync and both route layers run as they do in a deployment.
Every row is main's behaviour → this branch's:

Request Before After
{ title: { a: 1 } } to a string field 200, stored "[object Object]" 400, nothing stored
{ title: ["a","b"] } to a string field 200, stored "a,b" 400
{ count: "abc" } to an integer field 200, stored NaNnull 400
{ notes: { a: 1 } } 200, brief is "[object Object]" 400
PATCH { title: { a: 1 } } 200, overwrote a good value 400, old value survives
json field, default: { a: 1 } DEFAULT '[object Object]' DEFAULT '{"a":1}'
string field, default: { a: 1 } accepted 400 at POST and PATCH

Valid writes are untouched: a json field still stores {"a":1}, a richtext
field still accepts a document object, an integer still round-trips as a
number, and a normal PATCH still lands.

tsc --noEmit reports no new errors — the one pre-existing error is in
routes-uploads.ts and is fixed by #25.

…ect]"

Only `json` and `richtext` columns hold structure. Every other branch of
coerce() ended in String() or Number(), which do not fail on an object — they
succeed with garbage. So an object sent to a `text` or `string` field was
stored as the literal "[object Object]" and answered 200, and a non-numeric
value sent to `integer` became NaN and was stored as NULL. Both are silent,
lossy, and only surface much later, wherever the value is finally read.

A value the declared field cannot hold is now a 400 naming the field, its
type, and the remedy — declare the field `json` if it should hold structure.
Nothing that worked before changes: json/richtext still take objects,
pre-stringified JSON still stores verbatim, and numeric strings still coerce.
The only inputs whose behaviour changes are the ones that were being
destroyed.

Also routed through the same 400: the enum check, which threw a bare Error
and so came back as a 500, and `notes`, which took "[object Object]" as an
author's brief. PATCH now declares the 400 it can return.
A field's `default` is not written through the entry API — it is rendered
straight into the column's DDL, and defaultLiteral() reached the same
`String(value)` the entry path did. So `default: { kind: "chat" }` on a text
field installed `"bento_spec" TEXT DEFAULT '[object Object]'`, and an array
default became 'a,b'.

That is worse than the write-path bug it mirrors. No request ever carries the
value, so no write-time validation can catch it; every row that omits the field
silently gets it; and SQLite cannot ALTER COLUMN to take a default back.

The rule is now refused at the door that installs it — POST and PATCH
/api/content-types answer 400 for an object default on a field that cannot hold
one. defaultLiteral() serializes structure rather than flattening it, which
also makes an object default on a json/richtext field work; it produced
'[object Object]' before.

That second half is deliberately lossless rather than strict: it runs during
boot schema-sync over content types already in the database, so throwing on a
default written before this check existed would turn stale data into a dead
app.

"Which types hold structure" moves to content-types.ts, next to the type union
it describes, now that the entry layer and the schema syncer both need the
same answer.
The set listed only the types that hold structure, so it answered "no" for a
type nobody had classified — including one added later. A new member of
AttributeType would have started silently refusing valid writes to itself.

A total Record<AttributeType, boolean> makes that same edit a compile error
until the new type is answered for. Verified by adding a member: TS2741.
@pallaoro

pallaoro commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Checked this against how other schema-as-data CMSs handle a type-mismatched write, because the strict half of it is a behaviour change on a public API.

Refusing the write is the majority position, and the permissive ones never lose the value. Strapi rejects an object on a string field, and does so by explicitly disabling the cast — yup.string().transform((val, originalVal) => originalVal) — for the same reason this change exists: the default coercion does not fail, it succeeds with garbage. Directus is the permissive end and stringifies any object on any field type, but it stringifies to JSON, so {"a":1} comes back as {"a":1}. Nothing in the field stores a flattened "[object Object]". That was the outlier here, not the 400.

Typed defaults are Strapi's design too. Its Content-Type Builder validates default per type — string/text/richtext against a string validator, json against mixed — at the schema-definition route, which is the same layer and the same rule as the check added here. Payload takes the other side (one DefaultValue union for every field type) and also ships no first-party OpenAPI.

And publishing the rule matters. Directus's generated OpenAPI types each field correctly while its runtime silently stringifies, so the document and the behaviour disagree. Appending the rule to the write routes' descriptions is what keeps that from happening here.

Compatibility

Leniency the change deliberately keeps, verified against the running app:

count: "42" on an integer field 200, stored 42
count: "7.9" 200, stored 7
count: "" / count: null unchanged
unknown field in the body ignored, not rejected
richtext given a JSON string, or the document object both 200, both stored as the string
json given a JSON string 200, stored verbatim

Only genuinely unstorable input is refused. A client sending numeric strings, which is the common form-shaped case, is unaffected.

Two comments claimed SQLite "cannot ALTER COLUMN" to take a default back.
ALTER COLUMN does exist as of 3.53.0, it just only sets and drops NOT NULL —
there is no DDL for a default at all, and undoing one means rebuilding the
table. The point the comments were making survives; the reason given for it
was wrong.
@pallaoro

pallaoro commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Superseded: the same four-file change landed through #24 yesterday, so this branch is now fully contained in main and shows as conflicting. Closing as duplicate — the review and compatibility notes above carry over.

@pallaoro pallaoro closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant