Fix video_thumbnail_animation undefined frame size and zoom 0 - #90
Merged
Conversation
`frame_width` and `frame_height` were guarded with `!== undefined` but interpolated unconditionally, so omitting them produced `vta:10:100:5:undefined:undefined`. Both are required by imgproxy and by the option's type, so validate them like the other numeric arguments. `zoom` used `||` in `getOpt`, so `zoom: 0` fell through to `undefined` and the option was dropped without an error. imgproxy requires zoom factors greater than 0, so reject 0 instead of ignoring it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G8Aa3xeVUCaxKtQJDzZhMo
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.
Two defects found while auditing the codebase for the same class of bug as #89.
1.
video_thumbnail_animationcould emit a literalundefinedframe_widthandframe_heightwere guarded withif (… !== undefined)and then interpolated into the template unconditionally, so omitting them produced a malformed URL with no error.The docs list both as required (
Default: 0:0:0:0:0, no (optional) marker) and theVideoThumbnailAnimationtype declares them as requirednumbers, so they are now validated likestep,delayandframes. TypeScript callers were already prevented from hitting this; vanilla-JS callers now getvideo_thumbnail_animation.frame_width is not a numberinstead of a broken URL.video_thumbnail_tilealready handled its five numeric arguments this way.2.
zoom: 0was silently droppedgetOptusedoptions.zoom || options.z, so azoomof0fell through toundefined,test()returnedfalse, and the option vanished from the URL without an error. The guard was also{ min: 0 }, which would have accepted0had it ever been reached — while the docs state zoom values "must be greater than0".Now
getOptuses??,testchecks!== undefined, and the guard usesminEqual, so0is rejected forzoom,zoom_xandzoom_y.Test-visible change: the error message for non-positive zoom values changes from
can't be less than 0tocan't be less or equal than 0. Two existing assertions were updated accordingly — the only existing tests this PR modifies.Verification
tsc --noEmitclean,eslint src testscleanvitest run→ 97 files / 1100 tests passing (5 new: missingframe_width, missingframe_height,zoom: 0rejected,zoom_x: 0rejected,test({ zoom: 0 })truthy)Related
Follows #89. A third finding from the same audit — falsy values skipping validation in ~9 modules — is a mechanical sweep and comes in a separate PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_01G8Aa3xeVUCaxKtQJDzZhMo