Skip to content

Commit 6d76780

Browse files
committed
fix(studio): reject unsafe keyframe values
1 parent ad5229a commit 6d76780

9 files changed

Lines changed: 226 additions & 56 deletions

File tree

packages/core/package.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@
5454
"import": "./src/studio-api/helpers/draftMarkers.ts",
5555
"types": "./src/studio-api/helpers/draftMarkers.ts"
5656
},
57+
"./studio-api/finite-mutation": {
58+
"import": "./src/studio-api/helpers/finiteMutation.ts",
59+
"types": "./src/studio-api/helpers/finiteMutation.ts"
60+
},
5761
"./text": {
5862
"import": "./src/text/index.ts",
5963
"types": "./src/text/index.ts"
@@ -133,6 +137,10 @@
133137
"import": "./dist/studio-api/helpers/draftMarkers.js",
134138
"types": "./dist/studio-api/helpers/draftMarkers.d.ts"
135139
},
140+
"./studio-api/finite-mutation": {
141+
"import": "./dist/studio-api/helpers/finiteMutation.js",
142+
"types": "./dist/studio-api/helpers/finiteMutation.d.ts"
143+
},
136144
"./text": {
137145
"import": "./dist/text/index.js",
138146
"types": "./dist/text/index.d.ts"
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { describe, expect, it } from "vitest";
2+
import { findNonFiniteNumericFields, findUnsafeMutationValues } from "./finiteMutation";
3+
4+
describe("finiteMutation", () => {
5+
it("reports non-finite numbers before mutation serialization", () => {
6+
expect(
7+
findNonFiniteNumericFields({
8+
type: "set-arc-path",
9+
segments: [{ curviness: Number.NaN, cp1: { x: Infinity, y: 0 } }],
10+
}).map((field) => field.path),
11+
).toEqual(["body.segments[0].curviness", "body.segments[0].cp1.x"]);
12+
});
13+
14+
it("treats null as unsafe because JSON serializes NaN and Infinity to null", () => {
15+
expect(
16+
findUnsafeMutationValues({
17+
type: "update-property",
18+
property: "x",
19+
value: null,
20+
}),
21+
).toEqual([{ path: "body.value", reason: "null" }]);
22+
});
23+
});
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
export interface NonFiniteNumericField {
2+
path: string;
3+
value: number;
4+
}
5+
6+
export interface UnsafeMutationValue {
7+
path: string;
8+
reason: "non-finite-number" | "null";
9+
}
10+
11+
export function findNonFiniteNumericFields(value: unknown, path = "body"): NonFiniteNumericField[] {
12+
if (typeof value === "number") {
13+
return Number.isFinite(value) ? [] : [{ path, value }];
14+
}
15+
if (!value || typeof value !== "object") return [];
16+
if (Array.isArray(value)) {
17+
return value.flatMap((item, index) => findNonFiniteNumericFields(item, `${path}[${index}]`));
18+
}
19+
return Object.entries(value).flatMap(([key, item]) =>
20+
findNonFiniteNumericFields(item, `${path}.${key}`),
21+
);
22+
}
23+
24+
export function findUnsafeMutationValues(value: unknown, path = "body"): UnsafeMutationValue[] {
25+
if (value === null) return [{ path, reason: "null" }];
26+
if (typeof value === "number") {
27+
return Number.isFinite(value) ? [] : [{ path, reason: "non-finite-number" }];
28+
}
29+
if (!value || typeof value !== "object") return [];
30+
if (Array.isArray(value)) {
31+
return value.flatMap((item, index) => findUnsafeMutationValues(item, `${path}[${index}]`));
32+
}
33+
return Object.entries(value).flatMap(([key, item]) =>
34+
findUnsafeMutationValues(item, `${path}.${key}`),
35+
);
36+
}

packages/core/src/studio-api/routes/files.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { afterEach, describe, expect, it } from "vitest";
22
import { Hono } from "hono";
3-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
3+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
44
import { tmpdir } from "node:os";
55
import { join } from "node:path";
66
import { registerFileRoutes } from "./files";
@@ -171,6 +171,32 @@ tl.fromTo("#box", { opacity: 0, x: -50 }, { opacity: 1, x: 0, duration: 1.5, eas
171171
expect(result.parsed.animations[0].fromProperties?.x).toBe(-50);
172172
});
173173

174+
it("rejects serialized non-finite mutation values before writing source", async () => {
175+
const projectDir = createProjectDir();
176+
writeHtml(projectDir, "comp.html", FROMTO_COMP);
177+
const app = new Hono();
178+
registerFileRoutes(app, createAdapter(projectDir));
179+
180+
const anim = await getFirstAnimation(app, "comp.html");
181+
const before = readFileSync(join(projectDir, "comp.html"), "utf-8");
182+
const res = await app.request("http://localhost/projects/demo/gsap-mutations/comp.html", {
183+
method: "POST",
184+
headers: { "Content-Type": "application/json" },
185+
body: JSON.stringify({
186+
type: "update-property",
187+
animationId: anim.id,
188+
property: "x",
189+
value: Number.NaN,
190+
}),
191+
});
192+
const payload = (await res.json()) as { error?: string; fields?: string[] };
193+
194+
expect(res.status).toBe(400);
195+
expect(payload.error).toContain("unsafe numeric");
196+
expect(payload.fields).toContain("body.value");
197+
expect(readFileSync(join(projectDir, "comp.html"), "utf-8")).toBe(before);
198+
});
199+
174200
it("update-from-property returns 400 for a non-fromTo animation", async () => {
175201
const projectDir = createProjectDir();
176202
const TO_COMP = `<!DOCTYPE html><html><body><script data-hyperframes-gsap>

packages/core/src/studio-api/routes/files.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { isAudioFile } from "../helpers/mime.js";
1717
import { generateWaveformCache } from "../helpers/waveform.js";
1818
import { validateUploadedMediaBuffer } from "../helpers/mediaValidation.js";
1919
import { isSafePath } from "../helpers/safePath.js";
20+
import { findUnsafeMutationValues } from "../helpers/finiteMutation.js";
2021
import type { GsapAnimation } from "../../parsers/gsapSerialize.js";
2122
import {
2223
removeElementFromHtml,
@@ -1077,6 +1078,16 @@ export function registerFileRoutes(api: Hono, adapter: StudioApiAdapter): void {
10771078
if (!body || !body.type) {
10781079
return c.json({ error: "mutation type required" }, 400);
10791080
}
1081+
const unsafeFields = findUnsafeMutationValues(body);
1082+
if (unsafeFields.length > 0) {
1083+
return c.json(
1084+
{
1085+
error: "mutation contains unsafe numeric values",
1086+
fields: unsafeFields.map((field) => field.path),
1087+
},
1088+
400,
1089+
);
1090+
}
10801091

10811092
let html = readFileSync(res.absPath, "utf-8");
10821093
let block = extractGsapScriptBlock(html);
-20.4 KB
Binary file not shown.
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import type { DomEditSelection } from "../components/editor/domEditingTypes";
2+
3+
export const PROPERTY_DEFAULTS: Record<string, number> = {
4+
opacity: 1,
5+
x: 0,
6+
y: 0,
7+
scale: 1,
8+
scaleX: 1,
9+
scaleY: 1,
10+
rotation: 0,
11+
width: 100,
12+
height: 100,
13+
};
14+
15+
export function ensureElementAddressable(selection: DomEditSelection): {
16+
selector: string;
17+
autoId?: string;
18+
} {
19+
if (selection.id) return { selector: `#${selection.id}` };
20+
if (selection.selector) return { selector: selection.selector };
21+
22+
const el = selection.element;
23+
const doc = el.ownerDocument;
24+
const tag = el.tagName.toLowerCase();
25+
let id = tag;
26+
let n = 1;
27+
while (doc.getElementById(id)) {
28+
n += 1;
29+
id = `${tag}-${n}`;
30+
}
31+
el.setAttribute("id", id);
32+
return { selector: `#${id}`, autoId: id };
33+
}
34+
35+
export class GsapMutationHttpError extends Error {
36+
constructor(
37+
readonly statusCode: number,
38+
readonly responseBody: unknown,
39+
) {
40+
super(formatGsapMutationHttpErrorMessage(statusCode, responseBody));
41+
this.name = "GsapMutationHttpError";
42+
}
43+
}
44+
45+
function isRecord(value: unknown): value is Record<string, unknown> {
46+
return typeof value === "object" && value !== null;
47+
}
48+
49+
export async function readJsonResponseBody(res: Response): Promise<unknown> {
50+
const contentType = res.headers.get("content-type") ?? "";
51+
if (!contentType.includes("application/json")) {
52+
return await res.text().catch(() => null);
53+
}
54+
return await res.json().catch(() => null);
55+
}
56+
57+
function formatGsapMutationHttpErrorMessage(statusCode: number, body: unknown): string {
58+
if (isRecord(body) && typeof body.error === "string") {
59+
return body.error;
60+
}
61+
return `GSAP mutation failed with status ${statusCode}`;
62+
}
63+
64+
export function formatGsapMutationRejectionToast(error: GsapMutationHttpError): string {
65+
const body = error.responseBody;
66+
if (isRecord(body)) {
67+
const fields = Array.isArray(body.fields)
68+
? body.fields.filter((field): field is string => typeof field === "string")
69+
: [];
70+
const suffix = fields.length > 0 ? ` (${fields.join(", ")})` : "";
71+
return `Couldn't save animation: ${formatGsapMutationHttpErrorMessage(
72+
error.statusCode,
73+
body,
74+
)}${suffix}`;
75+
}
76+
return `Couldn't save animation: ${error.message}`;
77+
}

packages/studio/src/hooks/useDomEditSession.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,7 @@ export function useDomEditSession({
285285
reloadPreview,
286286
onCacheInvalidate: bumpGsapCache,
287287
onFileContentChanged: updateEditingFileContent,
288+
showToast,
288289
});
289290

290291
// ── Commit handlers (delegated to useDomEditCommits) ──

packages/studio/src/hooks/useGsapScriptCommits.ts

Lines changed: 43 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useCallback, useEffect, useRef } from "react";
22
import type { GsapAnimation, ParsedGsap } from "@hyperframes/core/gsap-parser";
3+
import { findNonFiniteNumericFields } from "@hyperframes/core/studio-api/finite-mutation";
34
import type { DomEditSelection } from "../components/editor/domEditingTypes";
45
import type { EditHistoryKind } from "../utils/editHistory";
56
import { applySoftReload } from "../utils/gsapSoftReload";
@@ -11,43 +12,13 @@ import {
1112
readKeyframeSnapshot,
1213
writeKeyframeCache,
1314
} from "./gsapKeyframeCacheHelpers";
14-
15-
const PROPERTY_DEFAULTS: Record<string, number> = {
16-
opacity: 1,
17-
x: 0,
18-
y: 0,
19-
scale: 1,
20-
scaleX: 1,
21-
scaleY: 1,
22-
rotation: 0,
23-
width: 100,
24-
height: 100,
25-
};
26-
27-
/**
28-
* Ensures the element has an id so it can be targeted by a GSAP selector.
29-
* If the element already has an id or a CSS selector, returns those.
30-
* Otherwise mints a unique id and sets it on the live element.
31-
*/
32-
function ensureElementAddressable(selection: DomEditSelection): {
33-
selector: string;
34-
autoId?: string;
35-
} {
36-
if (selection.id) return { selector: `#${selection.id}` };
37-
if (selection.selector) return { selector: selection.selector };
38-
39-
const el = selection.element;
40-
const doc = el.ownerDocument;
41-
const tag = el.tagName.toLowerCase();
42-
let id = tag;
43-
let n = 1;
44-
while (doc.getElementById(id)) {
45-
n += 1;
46-
id = `${tag}-${n}`;
47-
}
48-
el.setAttribute("id", id);
49-
return { selector: `#${id}`, autoId: id };
50-
}
15+
import {
16+
GsapMutationHttpError,
17+
ensureElementAddressable,
18+
formatGsapMutationRejectionToast,
19+
PROPERTY_DEFAULTS,
20+
readJsonResponseBody,
21+
} from "./gsapScriptCommitHelpers";
5122

5223
interface MutationResult {
5324
ok: boolean;
@@ -62,21 +33,19 @@ async function mutateGsapScript(
6233
projectId: string,
6334
sourceFile: string,
6435
mutation: Record<string, unknown>,
65-
): Promise<MutationResult | null> {
66-
try {
67-
const res = await fetch(
68-
`/api/projects/${encodeURIComponent(projectId)}/gsap-mutations/${encodeURIComponent(sourceFile)}`,
69-
{
70-
method: "POST",
71-
headers: { "Content-Type": "application/json" },
72-
body: JSON.stringify(mutation),
73-
},
74-
);
75-
if (!res.ok) return null;
76-
return (await res.json()) as MutationResult;
77-
} catch {
78-
return null;
36+
): Promise<MutationResult> {
37+
const res = await fetch(
38+
`/api/projects/${encodeURIComponent(projectId)}/gsap-mutations/${encodeURIComponent(sourceFile)}`,
39+
{
40+
method: "POST",
41+
headers: { "Content-Type": "application/json" },
42+
body: JSON.stringify(mutation),
43+
},
44+
);
45+
if (!res.ok) {
46+
throw new GsapMutationHttpError(res.status, await readJsonResponseBody(res));
7947
}
48+
return (await res.json()) as MutationResult;
8049
}
8150
interface GsapScriptCommitsParams {
8251
projectIdRef: React.MutableRefObject<string | null>;
@@ -94,6 +63,7 @@ interface GsapScriptCommitsParams {
9463
reloadPreview: () => void;
9564
onCacheInvalidate: () => void;
9665
onFileContentChanged?: (path: string, content: string) => void;
66+
showToast?: (message: string, tone?: "error" | "info") => void;
9767
}
9868
const DEBOUNCE_MS = 150;
9969

@@ -107,6 +77,7 @@ export function useGsapScriptCommits({
10777
reloadPreview,
10878
onCacheInvalidate,
10979
onFileContentChanged,
80+
showToast,
11081
}: GsapScriptCommitsParams) {
11182
const pendingPropertyEditRef = useRef<{
11283
selection: DomEditSelection;
@@ -131,11 +102,27 @@ export function useGsapScriptCommits({
131102
) => {
132103
const pid = projectIdRef.current;
133104
if (!pid) return;
105+
const nonFiniteFields = findNonFiniteNumericFields(mutation);
106+
if (nonFiniteFields.length > 0) {
107+
showToast?.(
108+
"Couldn't read element layout — try again at a different playhead time",
109+
"error",
110+
);
111+
if (options.skipReload) return;
112+
throw new Error(
113+
`Mutation contains non-finite numeric values: ${nonFiniteFields.map((field) => field.path).join(", ")}`,
114+
);
115+
}
134116
const targetPath = selection.sourceFile || activeCompPath || "index.html";
135-
const result = await mutateGsapScript(pid, targetPath, mutation);
136-
if (!result) {
117+
let result: MutationResult;
118+
try {
119+
result = await mutateGsapScript(pid, targetPath, mutation);
120+
} catch (error) {
121+
if (error instanceof GsapMutationHttpError) {
122+
showToast?.(formatGsapMutationRejectionToast(error), "error");
123+
}
137124
if (options.skipReload) return;
138-
throw new Error(`Mutation failed: ${mutation.type}`);
125+
throw error;
139126
}
140127

141128
if (result.changed === false) {
@@ -195,6 +182,7 @@ export function useGsapScriptCommits({
195182
reloadPreview,
196183
onCacheInvalidate,
197184
onFileContentChanged,
185+
showToast,
198186
],
199187
);
200188
const flushPendingPropertyEdit = useCallback(() => {
@@ -488,7 +476,7 @@ export function useGsapScriptCommits({
488476
return commitMutation(
489477
selection,
490478
{ type: "convert-to-keyframes", animationId, resolvedFromValues },
491-
{ label: "Convert to keyframes" },
479+
{ label: "Convert to keyframes", softReload: true },
492480
);
493481
},
494482
[commitMutation],

0 commit comments

Comments
 (0)