Repository navigation
fix(uploads): refuse an unsupported video before uploading it - #622
Merged
Merged
Conversation
_promptAttachKind returned 'video' for any video/* MIME, so an Android 3gp or an mkv streamed up to 1GB before the ?kind=video route refused it with a 400. Mirror the server's own video-container set so those are filtered client-side and get the existing unsupported-file toast at once. Follows the maintainer's follow-up suggestion on Ark0N#606.
Owner
|
Merged and shipped in 1.42.0. Thanks @JDProfresh! Matching the server's exact five video containers on the client, so a 1 GB upload headed for a 400 never starts, is the right fix. I checked it in a real file chooser: And yes please to the follow-up you offered: a toast for the refused files in a mixed selection, and for the Compose and paste-dialog pickers. |
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
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.
_promptAttachKindreturnedvideofor anyvideo/*MIME, so an Android.3gp(video/3gpp) or an.mkv(video/x-matroska) streamed all the way to disk, up to the 1GB cap, before the?kind=videoroute refused it with a 400.I mirror the server's own video admission in the client: a video MIME now counts only when it names one of the five containers the route accepts (
video/mp4,video/quicktime,video/x-m4v,video/webm,video/ogg), the same set asVIDEO_ATTACHMENT_EXTENSIONS(mp4/webm/mov/m4v/ogv). Anything else falls through to the existing extension check, which already rejects 3gp and mkv, so an unsupported clip is filtered client-side and gets the existing unsupported-file toast right away instead of after a long upload. An admitted container still classifies as video by MIME (iOS hands.movover asvideo/quicktime) or, when a file manager supplies no MIME at all, by its name's extension, exactly as before.This is the follow-up you suggested on #606.
Tests: extended
test/toolbar-attach.test.tsto pin that the five admitted MIME containers returnvideo, and that avideo/3gppand avideo/x-matroskareturn null while an admitted extension still rescues an odd MIME. I confirmed the new case goes red when the guard is reverted.Verified: full
npm testgate green (11259 passed, 12 skipped), plus typecheck, lint, format:check, check:frontend-syntax and check:browser-excludes.One gap this leaves, which is older than this PR and not specific to video: when a selection mixes a file the server takes with one it does not (a photo and an mkv, or on master today a photo and a pdf), the refused one is dropped without a toast, and the Compose and paste-dialog pickers never toast for a refused file at all. The toast only fires when every file is refused. I kept this PR to the one line; happy to send partial-rejection toasts as their own change if you want them.
Note: #621 (documents, sent alongside) adds a
documentbranch to the end of this same function, so whichever of the two lands second needs a trivial rebase on the adjacent lines.