Skip to content

Commit 5545814

Browse files
committed
core: harden setDataAt/unsetDataAt against edge cases
Improvements on top of the lodash/fp/set replacement: - Store "__proto__" segments as own properties via defineProperty and traverse own properties only, so such keys neither corrupt the container's prototype nor get dropped. Clone containers key-by-key because downleveled object spreads assign instead of define. - Read the updater's old data with resolveData instead of lodash get, so reads use the same literal path semantics as writes. - Fall back to lodash's index heuristic when creating missing containers without schema type information. - Match lodash's isIndex semantics with a strict index regex instead of Number() coercion. - Avoid strict-mode TypeErrors when unsetting non-index array properties (e.g. "length") and return the same reference when there is nothing to unset. - Treat an empty path as addressing the root, and replace non-object root data instead of spreading it. Also documents the path semantics change in MIGRATION.md and adds unit tests for setDataAt/unsetDataAt.
1 parent 1fb3daf commit 5545814

5 files changed

Lines changed: 320 additions & 62 deletions

File tree

MIGRATION.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ return this.t(label, label) as string;
4040

4141
This does not affect the Composition API where `Translator` is accessed directly from a `ComputedRef`.
4242

43+
### Data update paths treat all segments literally
44+
45+
Data updates (e.g. dispatched `update` actions) previously wrote to the form data via lodash's `set`/`unset`, which interpret bracket notation and array indices in paths.
46+
This corrupted data for property names that look like lodash path syntax, for example numeric property names like `"15"` were turned into array indices and names containing brackets like `"prop[0]"` were split up (see [#2397](https://github.com/eclipsesource/jsonforms/issues/2397) and [#2102](https://github.com/eclipsesource/jsonforms/issues/2102)).
47+
48+
Updates now use the new `setDataAt`/`unsetDataAt` utilities of `@jsonforms/core`, which split paths on `.` and treat every segment as a literal property name, matching how JSON Forms resolves values for display.
49+
When a missing intermediate container is created, the JSON Schema decides whether it becomes an array or an object; without schema type information, a numeric follow-up segment creates an array, as before.
50+
51+
If you dispatch update actions yourself, make sure to use dot-separated paths (e.g. `update('list.0.name', ...)`) instead of lodash bracket syntax (e.g. `update('list[0].name', ...)`), which is no longer interpreted.
52+
4353
### Angular support now targets Angular 20 to 22
4454

4555
When using JSON Forms 3.8, your Angular application now needs to target Angular 20, 21 or 22.

packages/core/src/reducers/core.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
*/
2525

2626
import cloneDeep from 'lodash/cloneDeep';
27-
import get from 'lodash/get';
2827
import isEqual from 'lodash/isEqual';
28+
import { resolveData } from '../util/resolvers';
2929
import { setDataAt, unsetDataAt } from '../util/setData';
3030
import {
3131
CoreActions,
@@ -237,7 +237,7 @@ export const coreReducer: Reducer<JsonFormsCore, CoreActions> = (
237237
errors,
238238
};
239239
} else {
240-
const oldData: any = get(state.data, action.path);
240+
const oldData: any = resolveData(state.data, action.path);
241241
const newData = action.updater(cloneDeep(oldData));
242242
let newState: any;
243243
if (newData !== undefined) {

packages/core/src/util/setData.ts

Lines changed: 97 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,17 @@
2626
import type { JsonSchema } from '../models';
2727
import { encode } from './path';
2828
import { resolveSchema } from './resolvers';
29-
import { hasType } from './util';
29+
import { deriveTypes, hasType } from './util';
3030

3131
const splitPath = (path: string): string[] => path.split('.');
3232

33+
/**
34+
* Whether the segment addresses an array element, i.e. is a canonical
35+
* non-negative integer without leading zeros.
36+
*/
37+
const isIndexSegment = (segment: string): boolean =>
38+
/^(?:0|[1-9]\d*)$/.test(segment);
39+
3340
/**
3441
* Walks one step in the schema along the given data path segment, so we can
3542
* tell whether a missing intermediate container should be created as an
@@ -51,21 +58,64 @@ const stepSchema = (
5158
return resolveSchema(schema, pointer, rootSchema);
5259
};
5360

61+
const assign = (container: any, segment: string, value: any): any => {
62+
if (segment === '__proto__') {
63+
// A plain assignment would overwrite the container's prototype instead
64+
// of creating an own property.
65+
Object.defineProperty(container, segment, {
66+
value,
67+
writable: true,
68+
enumerable: true,
69+
configurable: true,
70+
});
71+
} else {
72+
container[segment] = value;
73+
}
74+
return container;
75+
};
76+
5477
const cloneContainer = (data: any): any => {
5578
if (Array.isArray(data)) {
5679
return [...data];
5780
}
58-
return { ...(data ?? {}) };
81+
if (typeof data === 'object' && data !== null) {
82+
// Not a spread because downleveled object spreads assign instead of
83+
// defining properties, which mishandles own "__proto__" properties.
84+
const clone: { [key: string]: any } = {};
85+
for (const key of Object.keys(data)) {
86+
assign(clone, key, data[key]);
87+
}
88+
return clone;
89+
}
90+
return {};
5991
};
6092

61-
const assign = (container: any, segment: string, value: any): any => {
62-
if (Array.isArray(container)) {
63-
const index = Number(segment);
64-
container[Number.isInteger(index) ? index : (segment as any)] = value;
65-
} else {
66-
container[segment] = value;
93+
/**
94+
* Looks up the own property `segment` of `data`, ignoring inherited
95+
* properties like `__proto__`, mirroring the semantics of `resolveData`.
96+
*/
97+
const ownPropertyValue = (data: any, segment: string): any =>
98+
data != null && Object.prototype.hasOwnProperty.call(data, segment)
99+
? data[segment]
100+
: undefined;
101+
102+
/**
103+
* Determines the container to create for a missing child. The schema takes
104+
* precedence; without any schema type information the next path segment
105+
* decides, mirroring the behavior of lodash's `set`.
106+
*/
107+
const createInitialContainer = (
108+
childSchema: JsonSchema | undefined,
109+
nextSegment: string
110+
): any => {
111+
const types = childSchema ? deriveTypes(childSchema) : [];
112+
if (types.includes('array')) {
113+
return [];
67114
}
68-
return container;
115+
if (types.length > 0) {
116+
return {};
117+
}
118+
return isIndexSegment(nextSegment) ? [] : {};
69119
};
70120

71121
/**
@@ -74,19 +124,21 @@ const assign = (container: any, segment: string, value: any): any => {
74124
* Numeric path segments and segments containing bracket notation are treated
75125
* as plain object property names. The optional `rootSchema` is consulted when
76126
* a new intermediate container has to be created, so that arrays are still
77-
* created where the schema declares an array type.
127+
* created where the schema declares an array type. Without schema type
128+
* information, a canonical numeric follow-up segment creates an array.
129+
*
130+
* An empty `path` addresses the root, i.e. `value` itself is returned.
78131
*/
79132
export const setDataAt = (
80133
data: any,
81134
path: string,
82135
value: any,
83136
rootSchema?: JsonSchema
84137
): any => {
85-
const segments = splitPath(path);
86-
if (segments.length === 0) {
138+
if (path === '') {
87139
return value;
88140
}
89-
return doSet(data, segments, 0, value, rootSchema, rootSchema);
141+
return doSet(data, splitPath(path), 0, value, rootSchema, rootSchema);
90142
};
91143

92144
const doSet = (
@@ -98,39 +150,26 @@ const doSet = (
98150
rootSchema: JsonSchema | undefined
99151
): any => {
100152
const segment = segments[index];
101-
const childSchema = stepSchema(currentSchema, segment, rootSchema);
102153
const container = cloneContainer(data);
103154

104155
if (index === segments.length - 1) {
105156
return assign(container, segment, value);
106157
}
107158

108-
const existingChild = data?.[segment];
109-
let nextValue;
110-
if (
111-
existingChild !== undefined &&
112-
existingChild !== null &&
113-
typeof existingChild === 'object'
114-
) {
115-
nextValue = doSet(
116-
existingChild,
117-
segments,
118-
index + 1,
119-
value,
120-
childSchema,
121-
rootSchema
122-
);
123-
} else {
124-
const initial = hasType(childSchema, 'array') ? [] : {};
125-
nextValue = doSet(
126-
initial,
127-
segments,
128-
index + 1,
129-
value,
130-
childSchema,
131-
rootSchema
132-
);
133-
}
159+
const childSchema = stepSchema(currentSchema, segment, rootSchema);
160+
const existingChild = ownPropertyValue(data, segment);
161+
const child =
162+
existingChild !== null && typeof existingChild === 'object'
163+
? existingChild
164+
: createInitialContainer(childSchema, segments[index + 1]);
165+
const nextValue = doSet(
166+
child,
167+
segments,
168+
index + 1,
169+
value,
170+
childSchema,
171+
rootSchema
172+
);
134173
return assign(container, segment, nextValue);
135174
};
136175

@@ -139,49 +178,47 @@ const doSet = (
139178
*
140179
* Numeric path segments and bracket notation in segments are treated as
141180
* plain object property names, mirroring the semantics of {@link setDataAt}.
181+
* Unsetting an array element leaves a hole, i.e. the array is not compacted.
182+
* If there is nothing to unset at `path`, `data` is returned unchanged.
142183
*/
143184
export const unsetDataAt = (data: any, path: string): any => {
144-
const segments = splitPath(path);
145-
if (segments.length === 0) {
185+
if (path === '') {
146186
return data;
147187
}
148-
return doUnset(data, segments, 0);
188+
return doUnset(data, splitPath(path), 0);
149189
};
150190

151191
const doUnset = (data: any, segments: string[], index: number): any => {
152-
if (data === undefined || data === null) {
192+
if (data === null || typeof data !== 'object') {
153193
return data;
154194
}
155195
const segment = segments[index];
156196
if (index === segments.length - 1) {
197+
if (!Object.prototype.hasOwnProperty.call(data, segment)) {
198+
return data;
199+
}
157200
if (Array.isArray(data)) {
158-
const container = [...data];
159-
const numericIndex = Number(segment);
160-
if (Number.isInteger(numericIndex)) {
161-
delete container[numericIndex];
201+
// Non-index own properties of arrays (e.g. `length`) are not form data
202+
// and deleting them could throw in strict mode.
203+
if (!isIndexSegment(segment)) {
204+
return data;
162205
}
206+
const container = [...data];
207+
delete container[Number(segment)];
163208
return container;
164209
}
165-
if (!Object.prototype.hasOwnProperty.call(data, segment)) {
166-
return data;
167-
}
168-
const container: { [key: string]: any } = { ...data };
210+
const container = cloneContainer(data);
169211
delete container[segment];
170212
return container;
171213
}
172214

173-
const existingChild = data[segment];
174-
if (
175-
existingChild === undefined ||
176-
existingChild === null ||
177-
typeof existingChild !== 'object'
178-
) {
215+
const existingChild = ownPropertyValue(data, segment);
216+
if (existingChild === null || typeof existingChild !== 'object') {
179217
return data;
180218
}
181219
const nextValue = doUnset(existingChild, segments, index + 1);
182220
if (nextValue === existingChild) {
183221
return data;
184222
}
185-
const container = cloneContainer(data);
186-
return assign(container, segment, nextValue);
223+
return assign(cloneContainer(data), segment, nextValue);
187224
};

packages/core/test/reducers/core.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,41 @@ test('core reducer - update - unset works for numeric and bracket-containing key
860860
});
861861
});
862862

863+
test('core reducer - update - updater receives the current value for special property names', (t) => {
864+
const schema: JsonSchema = {
865+
type: 'object',
866+
properties: {
867+
'test[0]': { type: 'string' },
868+
'group-key': {
869+
type: 'object',
870+
properties: {
871+
'15': { type: 'string' },
872+
},
873+
},
874+
},
875+
};
876+
877+
const before: JsonFormsCore = {
878+
data: { 'test[0]': 'a', 'group-key': { '15': 'x' } },
879+
schema,
880+
uischema: { type: 'Label' },
881+
errors: [],
882+
validator: new Ajv().compile(schema),
883+
};
884+
885+
const afterBracket = coreReducer(
886+
before,
887+
update('test[0]', (old) => old + '!')
888+
);
889+
t.is((afterBracket.data as any)['test[0]'], 'a!');
890+
891+
const afterNumeric = coreReducer(
892+
afterBracket,
893+
update('group-key.15', (old) => old + '!')
894+
);
895+
t.is((afterNumeric.data as any)['group-key']['15'], 'x!');
896+
});
897+
863898
test('core reducer - updateErrors - should update errors with empty list', (t) => {
864899
const before: JsonFormsCore = {
865900
data: {},

0 commit comments

Comments
 (0)