Cscott has uploaded a new change for review.
https://gerrit.wikimedia.org/r/207816
Change subject: Ensure that embedded Maps and Sets are properly deep-frozen.
......................................................................
Ensure that embedded Maps and Sets are properly deep-frozen.
Object.freeze() doesn't actually have any effect on Map and Set, since
their mutable data is held in a hidden internal field. Create new
ReadOnlyMap and ReadOnlySet implementations which use a private
scoped variable to ensure that the mutable Map/Set backing it is
safely protected.
Tweak deepFreeze to recurse correctly through included maps and sets,
freezing the keys/values as appropriate.
Since the frozen map/set is a different object than the original map/set,
change deepFreeze to return the frozen object.
Change-Id: Ia16738e88aa3ee9c46fd71da50ce66b8917a07d3
---
M lib/ext.core.Sanitizer.js
M lib/jsutils.js
M lib/mediawiki.ParsoidConfig.js
M lib/mediawiki.WikiConfig.js
M lib/mediawiki.wikitext.constants.js
M lib/wts.SerializerState.js
6 files changed, 105 insertions(+), 18 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid
refs/changes/16/207816/1
diff --git a/lib/ext.core.Sanitizer.js b/lib/ext.core.Sanitizer.js
index 62b52ec..c1df8b2 100644
--- a/lib/ext.core.Sanitizer.js
+++ b/lib/ext.core.Sanitizer.js
@@ -576,7 +576,7 @@
SanitizerConstants.setDerivedConstants();
// Freeze it blocking all accidental changes
-JSUtils.deepFreeze(SanitizerConstants);
+SanitizerConstants = JSUtils.deepFreeze(SanitizerConstants);
/**
* @class
diff --git a/lib/jsutils.js b/lib/jsutils.js
index 816646c..742cf85 100644
--- a/lib/jsutils.js
+++ b/lib/jsutils.js
@@ -5,6 +5,34 @@
'use strict';
require('./core-upgrade');
+var ReadOnlyMap = function ReadOnlyMap() {
+ /* the actual data will be stored in a private scope variable */
+};
+ReadOnlyMap.prototype.clear =
+ReadOnlyMap.prototype['delete'] =
+ReadOnlyMap.prototype.set = function() {
+ throw new TypeError("Mutation attempted on read-only map.");
+};
+if (typeof Symbol !== 'undefined' && typeof Symbol.iterator !== 'undefined') {
+ Object.defineProperty(ReadOnlyMap.prototype, Symbol.iterator, {
+ get: function() { return this.entries; }
+ });
+}
+
+var ReadOnlySet = function ReadOnlySet() {
+ /* the actual data will be stored in a private scope variable */
+};
+ReadOnlySet.prototype.add =
+ReadOnlySet.prototype.clear =
+ReadOnlyMap.prototype['delete'] = function() {
+ throw new TypeError("Mutation attempted on read-only set.");
+};
+if (typeof Symbol !== 'undefined' && typeof Symbol.iterator !== 'undefined') {
+ Object.defineProperty(ReadOnlySet.prototype, Symbol.iterator, {
+ get: function() { return this.values; }
+ });
+}
+
var JSUtils = {
// in ES7 it should be `new Map(Object.entries(obj))`
@@ -14,29 +42,90 @@
}));
},
+ // ES6 maps/sets are still writable even when frozen, because they
+ // store data inside the object linked from an internal slot.
+ // This returns an actual read-only Map.
+ readOnlyMap: function(it, freezeEntries) {
+ // Allow `it` to be an iterable, as well as a map.
+ if (!(it instanceof Map)) { it = new Map(it); }
+ /* Copy `it`, to prevent later mutation, and (optionally)
+ * freeze keys and values along the way. */
+ var freeze = function(v) {
+ return freezeEntries ? JSUtils.deepFreeze(v) : v;
+ };
+ var m = new Map();
+ it.forEach(function(val, key) { m.set(freeze(key),
freeze(val)); });
+ // Now make our read-only map object.
+ var ro = new ReadOnlyMap();
+ Object.defineProperties(ro, {
+ constructor: { value: Map /* a little white lie */ },
+ entries: { value: function() { return m.entries(); } },
+ forEach: { value: function(f, t) { return m.forEach(f,
t); } },
+ get: { value: function(k) { return m.get(k); } },
+ has: { value: function(k) { return m.has(k); } },
+ keys: { value: function() { return m.keys(); } },
+ size: { get: function() { return m.size; } },
+ values: { value: function() { return m.values(); } },
+ toString: { value: function() { return m.toString(); }
},
+ });
+ Object.freeze(ro);
+ return ro;
+ },
+ // This returns an actual read-only Set.
+ readOnlySet: function(it, freezeEntries) {
+ // Allow `it` to be an iterable, as well as a set.
+ if (!(it instanceof Set)) { it = new Set(it); }
+ /* Copy `it`, to prevent later mutation, and (optionally)
+ * freeze keys and values along the way. */
+ var freeze = function(v) {
+ return freezeEntries ? JSUtils.deepFreeze(v) : v;
+ };
+ var s = new Set();
+ it.forEach(function(val) { s.add(freeze(val)); });
+ // Now make our read-only set object.
+ var ro = new ReadOnlySet();
+ Object.defineProperties(ro, {
+ constructor: { value: Set /* a little white lie */ },
+ entries: { value: function() { return s.entries(); } },
+ forEach: { value: function(f, t) { return s.forEach(f,
t); } },
+ has: { value: function(k) { return s.has(k); } },
+ keys: { value: function() { return s.keys(); } },
+ size: { get: function() { return s.size; } },
+ values: { value: function() { return s.values(); } },
+ toString: { value: function() { return s.toString(); }
},
+ });
+ Object.freeze(ro);
+ return ro;
+ },
+
// Deep-freeze an object
// See
https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Object/freeze
deepFreeze: function(o) {
if ( o === undefined ) {
- return;
+ return o;
} else if ( !(o instanceof Object) ) {
- return;
+ return o;
} else if ( Object.isFrozen(o) ) {
- return;
+ return o;
+ } else if (o instanceof Map) {
+ return JSUtils.readOnlyMap(o, true);
+ } else if (o instanceof Set) {
+ return JSUtils.readOnlySet(o, true);
}
- Object.freeze(o); // First freeze the object.
for (var propKey in o) {
- var prop = o[propKey];
- if (!o.hasOwnProperty(propKey) || !(prop instanceof
Object) || Object.isFrozen(prop)) {
- // If the object is on the prototype, not an
object, or is already frozen,
+ var desc = Object.getOwnPropertyDescriptor(o, propKey);
+ if (!desc || !(desc.value instanceof Object) ||
Object.isFrozen(desc.value)) {
+ // If the object is on the prototype, is a
getter, not an object, or is already frozen,
// skip it. Note that this might leave an
unfrozen reference somewhere in the
// object if there is an already frozen object
containing an unfrozen object.
continue;
}
-
- this.deepFreeze(prop); // Recursively call deepFreeze.
+ // Recursively call deepFreeze.
+ o[propKey] = JSUtils.deepFreeze(desc.value);
}
+ Object.freeze(o); // Now freeze the object.
+ return o;
},
// Convert a counter to a Base64 encoded string.
diff --git a/lib/mediawiki.ParsoidConfig.js b/lib/mediawiki.ParsoidConfig.js
index f1fa168..baa6d5b 100644
--- a/lib/mediawiki.ParsoidConfig.js
+++ b/lib/mediawiki.ParsoidConfig.js
@@ -59,7 +59,7 @@
}
// ParsoidConfig is used across requests. Freeze it to avoid mutation.
- JSUtils.deepFreeze(this);
+ return JSUtils.deepFreeze(this);
}
/**
diff --git a/lib/mediawiki.WikiConfig.js b/lib/mediawiki.WikiConfig.js
index 749c2ca..9a7bf67 100644
--- a/lib/mediawiki.WikiConfig.js
+++ b/lib/mediawiki.WikiConfig.js
@@ -10,7 +10,7 @@
var request = require('request');
// Make sure our base config is never modified
-JSUtils.deepFreeze(baseConfig);
+baseConfig = JSUtils.deepFreeze(baseConfig);
/**
diff --git a/lib/mediawiki.wikitext.constants.js
b/lib/mediawiki.wikitext.constants.js
index ef08550..c0c8e0d 100644
--- a/lib/mediawiki.wikitext.constants.js
+++ b/lib/mediawiki.wikitext.constants.js
@@ -301,7 +301,7 @@
});
// Freeze constants to prevent accidental changes
-JSUtils.deepFreeze(WikitextConstants);
+WikitextConstants = JSUtils.deepFreeze(WikitextConstants);
if (typeof module === "object") {
module.exports.WikitextConstants = WikitextConstants;
diff --git a/lib/wts.SerializerState.js b/lib/wts.SerializerState.js
index a16c9b7..482ebfd 100644
--- a/lib/wts.SerializerState.js
+++ b/lib/wts.SerializerState.js
@@ -70,7 +70,8 @@
* Stack used to enforce single-line context
* ********************************************************************* */
-var initialState = {
+// Freeze to ensure the initialState is never modified
+var initialState = JSUtils.deepFreeze({
rtTesting: true,
sep: {},
onSOL: true,
@@ -84,10 +85,7 @@
wteHandlerStack: [],
// XXX: replace with output buffering per line
currLine: null
-};
-
-// Make sure the initialState is never modified
-JSUtils.deepFreeze(initialState);
+});
// Stack and helpers to enforce single-line context while serializing
function SingleLineContext() {
--
To view, visit https://gerrit.wikimedia.org/r/207816
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia16738e88aa3ee9c46fd71da50ce66b8917a07d3
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Cscott <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
