Let the emoji upload limit be the one the owner set (GRYT-689) - #91
Merged
Conversation
`emojiMaxBytes` clamps anywhere from 64 KB to 200 MB and defaults to 100 MB.
The multer instance both emoji routes shared was a fixed 50 MB. So an owner
who set anything above that got a number the server would never honour, and
the request died inside multer with a generic error instead of reaching the
size check that has something useful to say.
This is the same two-ceilings bug `uploadToDisk` in `uploads.ts` already
fixed for general uploads, and the comment there says so: "There used to be
two independent ceilings — a fixed 200 MB here and whatever the server was
configured for — and the lower one silently won." Same shape, same fix. The
emoji routes now build multer per request from `getServerConfig()`.
Memory storage stays. Every emoji is re-encoded through sharp immediately,
so a temp file would be written and deleted for nothing, which is the reason
`uploads.ts` gives for avatars.
Emoji do not get the upload route's unlimited-at-zero branch: `clampBytes`
holds this setting to a 64 KB floor, so zero cannot be set. The guard is
there for a zero written straight into the row, where `{ fileSize: 0 }`
would refuse every upload.
Docs said "capped at 50 MB" and are corrected in docs#78, which should land
with this rather than before it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
emojiMaxBytesclamps between 64 KB and 200 MB and defaults to 100 MB. The multer instance both emoji routes shared was a fixed 50 MB. An owner who set anything above that got a number the server would never honour, and the request died inside multer with a generic error rather than reaching the size check that has something useful to say.This is the same bug
uploads.tsalready fixed for general uploads, and its comment describes it exactly:Same shape, same fix.
emojiUpload(fields)builds multer per request fromgetServerConfig(), mirroringuploadToDisk(field)next door. BothPOST /api/emojisandPOST /api/emojis/stageuse it.What to look at
uploads.tsgives for avatars. The consequence is that an owner who sets 200 MB is choosing to hold 200 MB per file in memory. That was already true of the avatar route, and it is now reachable for emoji.clampBytesholdsemojiMaxBytesto a 64 KB floor, so zero cannot be set through settings — unlikeuploadMaxBytes, which hasclampBytesAllowingZero. ThemaxBytes > 0check is defensive for a zero written straight into the row, where{ fileSize: 0 }would refuse every upload. Say the word if you would rather emoji got the unlimited option too; that is a one-line change inserverSettings.tsand a bigger decision.filesfield still has nomaxCount, so a batch request can carry many files at the new per-file limit. That is unchanged by this PR and predates it, but it is more load-bearing now than at a fixed 50 MB.tsc --noEmitclean.Related
docs#78 drops the "capped at 50 MB" sentence docs#77 added. It is correct only once this merges, so merge this one first.
🤖 Generated with Claude Code