Fix GIF recording crash when a dweet reassigns c - #547
Open
MattNotarangelo wants to merge 1 commit into
Open
Conversation
A dweet that reassigns the global `c` (e.g. using it as a scratch
variable inside an expression like `x.fillRect(X,Y,c=8+15*b,c)`)
overwrites `window.c` with a non-canvas value. Recording then fails
because `gifctx.drawImage(c, ...)` is called with a number instead of
the canvas element, throwing:
Uncaught TypeError: Failed to execute 'drawImage' on
'CanvasRenderingContext2D': The provided value is not of type
'(CSSImageValue or HTMLCanvasElement or ...)'.
Capture the canvas in a script-scope `const __c` (invisible to dweet
`new Function(...)` bodies) and use that for recording. The dweet's
own use of `c` as a scratch variable still works as before; only the
recording path is hardened.
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.
What
A dweet that reassigns the global
c(the canvas) breaks GIF recording. Example offender — usescas a scratch variable insidefillRect:Trying to record this dweet throws:
…because
c=8+15*bfalls through towindow.c(the dweet body runs asnew Function("t", …), where free identifiers resolve via the global object), overwriting the canvas reference with a number. The recording loop then callsgifctx.drawImage(c, …)with that number.Fix
Capture the canvas in a script-scope
const __cnext to the existingvar c = document.querySelector("#c").constdeclarations at script top level live in script scope and are not properties ofwindow, so they're invisible tonew Function(…)bodies and can't be reassigned by dweets. The recording path uses__cinstead ofc.The dweet's own use of
cas a scratch variable still works exactly as before —window.cremains writable, the dweet's math is unchanged, only the page's recording path is hardened.Diff
3 substantive changes, 7 lines added / 3 removed:
const __c = c;next to the canvas grabMath.max(__c.width, …)in the GIF-record start handlergifctx.drawImage(__c, …)in the per-frame recorderTesting
Tested locally