fix(article): reject cover uploads carrying more than one file - #216
Conversation
An article holds exactly one cover: there is a single cover_image_key column, so the limit already existed in the data model. The upload endpoint did not honour it honestly, though - request.file() keeps the first part and discards the rest without a word, so a client that posted three images got a 200 and no way to tell which one was stored. Measured before the change: 1 file -> 200 3 files -> 200 (first kept, rest silently dropped) 5 files -> 200 (same) The handler now iterates the parts, consumes the first, and refuses if a second one follows. The check runs before anything reaches storage, so a rejected request leaves no orphaned object in the bucket. After: 1 file -> 200 2 files -> 400 MediaLimitExceededError 5 files -> 400 MediaLimitExceededError 0 files -> 400 NoMediaProvidedError Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
CI caught a broken test file on the first push — fixed and re-pushed. The multipart helper I added ended up with literal newlines where CRLF escapes belonged, so Worth naming why nothing local caught it, because it is the same gap I flagged on #215:
So a test file can be syntactically broken and every local check still passes. This is the second time it has bitten in this session. Verified before re-pushing this time, two ways:
A |
…217) #216 merged while its CI was red, so main carries a test file that cannot be parsed: the multipart helper has literal newlines where CRLF escapes belong, which leaves Buffer.from("<newline>") as an unterminated string. vitest collects zero tests from the file and reports the suite as failing, so the E2E job on main is currently red. This is the fix that was already on the PR branch but had not reached the PR when it was merged. Verified two ways before pushing, since none of the local checks cover test files - build type-checks tsconfig.build.json which excludes tests/, test:unit does not run e2e, and eslint ignores tests/**: tsc -p tsconfig.json --noEmit no errors for this file esbuild <file> exits 0, both new test names in the output Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
🎉 This PR is included in version 1.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
What does this PR do?
Makes the one-cover-per-article rule explicit at the upload endpoint.
The limit already existed in the data model —
articleshas a singlecover_image_keycolumn, so a second cover cannot be attached. What was missing was the endpoint saying so.request.file()keeps the first part and discards the rest without a word, so a client that posted three images received a200and no way to tell which one was stored.Measured, before
After
MediaLimitExceededErrorMediaLimitExceededErrorNoMediaProvidedErrorHow
The handler iterates the multipart parts rather than taking the first: it consumes the first file, then checks whether another follows. The check runs before anything reaches storage, so a rejected request leaves no orphaned object in the bucket.
Note the ordering matters — the first part's buffer has to be consumed before the iterator can advance, which is why the size and magic-byte validation still happens on exactly the file that would have been stored.
Not included, deliberately
I had proposed a separate
POST /articles/mediaendpoint for images embedded in the markdown body. Dropped at the author's direction: externalURLs already work, and the body is never rendered server-side, so there is no server-side sink. Cover images stay the only upload path.Verification
lint,format:check,buildclean; 763 unit tests pass.Two new e2e cases assert that two files and three files are both refused. Verified end to end against the dev database with the table above.
Type of Change
Checklist
feature/,fix/,chore/,docs/)🤖 Generated with Claude Code