Skip to content

Commit b1387e6

Browse files
authored
lib: use Object.freeze to avoid defensive cloning in SourceMap
PR-URL: #62830 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 57ee682 commit b1387e6

3 files changed

Lines changed: 30 additions & 8 deletions

File tree

doc/api/module.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,10 +1962,20 @@ generated code.
19621962
19631963
#### `sourceMap.payload`
19641964
1965+
<!-- YAML
1966+
changes:
1967+
- version: REPLACEME
1968+
pr-url: https://github.com/nodejs/node/pull/62830
1969+
description: The object is frozen.
1970+
-->
1971+
19651972
* Returns: {Object}
19661973
19671974
Getter for the payload used to construct the [`SourceMap`][] instance.
19681975
1976+
The returned object is frozen with [`Object.freeze()`][], and the same
1977+
reference is returned on every access. Do not mutate the returned object.
1978+
19691979
#### `sourceMap.findEntry(lineOffset, columnOffset)`
19701980
19711981
* `lineOffset` {number} The zero-indexed line number offset in
@@ -2050,6 +2060,7 @@ returned object contains the following keys:
20502060
[`NODE_COMPILE_CACHE_PORTABLE=1`]: cli.md#node_compile_cache_portable1
20512061
[`NODE_DISABLE_COMPILE_CACHE=1`]: cli.md#node_disable_compile_cache1
20522062
[`NODE_V8_COVERAGE=dir`]: cli.md#node_v8_coveragedir
2063+
[`Object.freeze()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze
20532064
[`SourceMap`]: #class-modulesourcemap
20542065
[`initialize`]: #initialize
20552066
[`module.constants.compileCacheStatus`]: #moduleconstantscompilecachestatus

lib/internal/source_map/source_map.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ const {
7171
ArrayPrototypePush,
7272
ArrayPrototypeSlice,
7373
ArrayPrototypeSort,
74+
ObjectFreeze,
7475
ObjectPrototypeHasOwnProperty,
7576
StringPrototypeCharAt,
7677
Symbol,
@@ -144,15 +145,15 @@ class SourceMap {
144145
this.#payload = cloneSourceMapV3(payload);
145146
this.#parseMappingPayload();
146147
if (ArrayIsArray(lineLengths) && lineLengths.length) {
147-
this.#lineLengths = lineLengths;
148+
this.#lineLengths = ObjectFreeze(ArrayPrototypeSlice(lineLengths));
148149
}
149150
}
150151

151152
/**
152153
* @returns {object} raw source map v3 payload.
153154
*/
154155
get payload() {
155-
return cloneSourceMapV3(this.#payload);
156+
return this.#payload;
156157
}
157158

158159
get [kMappings]() {
@@ -163,10 +164,7 @@ class SourceMap {
163164
* @returns {number[] | undefined} line lengths of generated source code
164165
*/
165166
get lineLengths() {
166-
if (this.#lineLengths) {
167-
return ArrayPrototypeSlice(this.#lineLengths);
168-
}
169-
return undefined;
167+
return this.#lineLengths;
170168
}
171169

172170
#parseMappingPayload = () => {
@@ -366,10 +364,10 @@ function cloneSourceMapV3(payload) {
366364
for (const key in payload) {
367365
if (ObjectPrototypeHasOwnProperty(payload, key) &&
368366
ArrayIsArray(payload[key])) {
369-
payload[key] = ArrayPrototypeSlice(payload[key]);
367+
payload[key] = ObjectFreeze(ArrayPrototypeSlice(payload[key]));
370368
}
371369
}
372-
return payload;
370+
return ObjectFreeze(payload);
373371
}
374372

375373
/**

test/parallel/test-source-map-api.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ const { readFileSync } = require('fs');
140140
assert.notStrictEqual(payload, sourceMap.payload);
141141
assert.strictEqual(payload.sources[0], sourceMap.payload.sources[0]);
142142
assert.notStrictEqual(payload.sources, sourceMap.payload.sources);
143+
// The payload and its arrays should be frozen to avoid unnecessary cloning:
144+
assert(Object.isFrozen(sourceMap.payload));
145+
assert(Object.isFrozen(sourceMap.payload.sources));
146+
// The same frozen object is returned on each call:
147+
assert.strictEqual(sourceMap.payload, sourceMap.payload);
148+
// lineLengths should be frozen and return the same reference each call:
149+
assert(Object.isFrozen(sourceMap.lineLengths));
150+
assert.strictEqual(sourceMap.lineLengths, sourceMap.lineLengths);
143151
}
144152

145153
// findEntry() and findOrigin() must return empty object instead of
@@ -178,6 +186,11 @@ const { readFileSync } = require('fs');
178186
assert.notStrictEqual(payload, sourceMap.payload);
179187
assert.strictEqual(payload.sources[0], sourceMap.payload.sources[0]);
180188
assert.notStrictEqual(payload.sources, sourceMap.payload.sources);
189+
// The payload and its arrays should be frozen to avoid unnecessary cloning:
190+
assert(Object.isFrozen(sourceMap.payload));
191+
assert(Object.isFrozen(sourceMap.payload.sources));
192+
// The same frozen object is returned on each call:
193+
assert.strictEqual(sourceMap.payload, sourceMap.payload);
181194
}
182195

183196
// Test various known decodings to ensure decodeVLQ works correctly.

0 commit comments

Comments
 (0)