Skip to content

Commit f1aa37c

Browse files
istarkovistarkov
authored
fix: Skip unsafe attribute names on generation (#4216)
## Description We can also do not skip but do `<Dom {...{"1unsafe": value}} />` BTW that looks useless. closes #3573 closes webstudio-is/webstudio-community#190 (comment) ## Steps for reproduction 1. click button 2. expect xyz ## Code Review - [ ] hi @kof, I need you to do - conceptual review (architecture, feature-correctness) - detailed review (read every line) - test it on preview ## Before requesting a review - [ ] made a self-review - [ ] added inline comments where things may be not obvious (the "why", not "what") ## Before merging - [ ] tested locally and on preview environment (preview dev login: 5de6) - [ ] updated [test cases](https://github.com/webstudio-is/webstudio/blob/main/apps/builder/docs/test-cases.md) document - [ ] added tests - [ ] if any new env variables are added, added them to `.env` file
1 parent 0ceaac9 commit f1aa37c

File tree

6 files changed

+128
-41
lines changed

6 files changed

+128
-41
lines changed

‎apps/builder/app/builder/features/settings-panel/props-section/props-section.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import {
99
Flex,
1010
Box,
1111
} from "@webstudio-is/design-system";
12-
import { descendantComponent } from "@webstudio-is/react-sdk";
12+
import {
13+
descendantComponent,
14+
isAttributeNameSafe,
15+
} from "@webstudio-is/react-sdk";
1316
import {
1417
$propValuesByInstanceSelector,
1518
$propsIndex,
@@ -21,7 +24,6 @@ import { renderControl } from "../controls/combined";
2124
import { usePropsLogic, type PropAndMeta } from "./use-props-logic";
2225
import { Row } from "../shared";
2326
import { serverSyncStore } from "~/shared/sync";
24-
import { isAttributeNameSafe } from "~/shared/dom-utils";
2527

2628
type Item = {
2729
name: string;

‎apps/builder/app/shared/dom-utils.ts‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -192,39 +192,3 @@ export const getAllElementsBoundingBox = (elements: Element[]): DOMRect => {
192192
width: right - left,
193193
});
194194
};
195-
196-
/**
197-
* Copyright (c) Meta Platforms, Inc. and affiliates.
198-
*
199-
* This source code is licensed under the MIT license found in the
200-
* LICENSE file in the root directory of this source tree.
201-
*
202-
* https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js
203-
*/
204-
const attributeNameStartChar =
205-
":A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
206-
const attributeNameChar =
207-
attributeNameStartChar + "\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
208-
209-
const validAttributeNameRegex = new RegExp(
210-
// eslint-disable-next-line no-misleading-character-class
211-
"^[" + attributeNameStartChar + "][" + attributeNameChar + "]*$"
212-
);
213-
214-
const illegalAttributeNameCache = new Map<string, boolean>();
215-
const validatedAttributeNameCache = new Map<string, boolean>();
216-
217-
export const isAttributeNameSafe = (attributeName: string) => {
218-
if (validatedAttributeNameCache.has(attributeName)) {
219-
return true;
220-
}
221-
if (illegalAttributeNameCache.has(attributeName)) {
222-
return false;
223-
}
224-
if (validAttributeNameRegex.test(attributeName)) {
225-
validatedAttributeNameCache.set(attributeName, true);
226-
return true;
227-
}
228-
illegalAttributeNameCache.set(attributeName, true);
229-
return false;
230-
};

‎packages/react-sdk/src/component-generator.test.tsx‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -948,3 +948,54 @@ test("generate resource prop", () => {
948948
"
949949
`);
950950
});
951+
952+
test("skip unsafe properties", () => {
953+
expect(
954+
generateWebstudioComponent({
955+
classesMap: new Map(),
956+
scope: createScope(),
957+
name: "Page",
958+
rootInstanceId: "body",
959+
parameters: [],
960+
dataSources: new Map(),
961+
indexesWithinAncestors: new Map(),
962+
963+
instances: toMap([
964+
{ type: "instance", id: "body", component: "Body", children: [] },
965+
]),
966+
props: toMap([
967+
{
968+
id: "unsafeProp",
969+
instanceId: "body",
970+
name: "",
971+
type: "string",
972+
value: "unsafe",
973+
},
974+
975+
{
976+
id: "unsafeProp-2",
977+
instanceId: "body",
978+
name: "1-numeric-unsafe",
979+
type: "string",
980+
value: "unsafe",
981+
},
982+
983+
{
984+
id: "unsafeProp-3",
985+
instanceId: "body",
986+
name: "click.prevent",
987+
type: "string",
988+
value: "unsafe",
989+
},
990+
]),
991+
})
992+
).toEqual(
993+
validateJSX(
994+
clear(`
995+
const Page = () => {
996+
return <Body />
997+
}
998+
`)
999+
)
1000+
);
1001+
});

‎packages/react-sdk/src/component-generator.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
decodeDataSourceVariable,
1414
transpileExpression,
1515
} from "@webstudio-is/sdk";
16-
import { indexAttribute, showAttribute } from "./props";
16+
import { indexAttribute, isAttributeNameSafe, showAttribute } from "./props";
1717
import { collectionComponent, descendantComponent } from "./core-components";
1818
import type { IndexesWithinAncestors } from "./instance-utils";
1919

@@ -181,6 +181,11 @@ export const generateJsxElement = ({
181181
dataSources,
182182
usedDataSources,
183183
});
184+
185+
if (isAttributeNameSafe(prop.name) === false) {
186+
continue;
187+
}
188+
184189
// show prop controls conditional rendering and need to be handled separately
185190
if (prop.name === showAttribute) {
186191
// prevent generating unnecessary condition

‎packages/react-sdk/src/props.test.ts‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { test, expect } from "@jest/globals";
1+
import { test, expect, describe } from "@jest/globals";
22
import type { Pages, Prop } from "@webstudio-is/sdk";
3-
import { normalizeProps } from "./props";
3+
import { isAttributeNameSafe, normalizeProps } from "./props";
44

55
const pagesBase: Pages = {
66
meta: {},
@@ -289,3 +289,30 @@ test("normalize page prop with path and hash into string", () => {
289289
idProp,
290290
]);
291291
});
292+
293+
describe("isAttributeNameSafe", () => {
294+
test("should return true for valid attribute names", () => {
295+
expect(isAttributeNameSafe("data-test")).toBe(true);
296+
expect(isAttributeNameSafe("aria-label")).toBe(true);
297+
expect(isAttributeNameSafe("class")).toBe(true);
298+
expect(isAttributeNameSafe("ns:class")).toBe(true);
299+
});
300+
301+
test("should return false for invalid attribute names", () => {
302+
expect(isAttributeNameSafe("123class")).toBe(false);
303+
expect(isAttributeNameSafe("class.name")).toBe(false);
304+
expect(isAttributeNameSafe(":bad")).toBe(false);
305+
expect(isAttributeNameSafe(" ")).toBe(false);
306+
expect(isAttributeNameSafe("hello world")).toBe(false);
307+
});
308+
309+
test("should return true for cached valid attribute names", () => {
310+
isAttributeNameSafe("data-cached");
311+
expect(isAttributeNameSafe("data-cached")).toBe(true);
312+
});
313+
314+
test("should return false for cached invalid attribute names", () => {
315+
isAttributeNameSafe("1-invalid-cached");
316+
expect(isAttributeNameSafe("1-invalid-cached")).toBe(false);
317+
});
318+
});

‎packages/react-sdk/src/props.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,41 @@ export const showAttribute = "data-ws-show" as const;
129129
export const indexAttribute = "data-ws-index" as const;
130130
export const collapsedAttribute = "data-ws-collapsed" as const;
131131
export const textContentAttribute = "data-ws-text-content" as const;
132+
133+
/**
134+
* Copyright (c) Meta Platforms, Inc. and affiliates.
135+
*
136+
* This source code is licensed under the MIT license found in the
137+
* LICENSE file in the root directory of this source tree.
138+
*
139+
* https://github.com/facebook/react/blob/main/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js
140+
*/
141+
const attributeNameStartChar =
142+
"A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
143+
// original: ":A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD";
144+
const attributeNameChar =
145+
attributeNameStartChar + ":\\-0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
146+
// original: attributeNameStartChar + "\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040";
147+
148+
const validAttributeNameRegex = new RegExp(
149+
// eslint-disable-next-line no-misleading-character-class
150+
"^[" + attributeNameStartChar + "][" + attributeNameChar + "]*$"
151+
);
152+
153+
const illegalAttributeNameCache = new Map<string, boolean>();
154+
const validatedAttributeNameCache = new Map<string, boolean>();
155+
156+
export const isAttributeNameSafe = (attributeName: string) => {
157+
if (validatedAttributeNameCache.has(attributeName)) {
158+
return true;
159+
}
160+
if (illegalAttributeNameCache.has(attributeName)) {
161+
return false;
162+
}
163+
if (validAttributeNameRegex.test(attributeName)) {
164+
validatedAttributeNameCache.set(attributeName, true);
165+
return true;
166+
}
167+
illegalAttributeNameCache.set(attributeName, true);
168+
return false;
169+
};

0 commit comments

Comments
 (0)