Skip to content

Commit

Permalink
Hide or don't minify extra internal properties on binding context
Browse files Browse the repository at this point in the history
It is possible for properties such as `_subscribable` to be created on a
binding context, which, after minification, could be renamed to any
arbitrary character. In most cases this is not an issue, except when a
global variable with the same name exists, however rare the case may be.
This causes the global variable to be shadowed in all bindings, which is
especially problematic in the case of, say, `$`, which breaks jquery.

To fix this, either hide such properties with a Symbol, or simply don't
minify them so that the name is constant and predictable.

Fixes #2294
  • Loading branch information
bennieswart committed Sep 23, 2017
1 parent e57500a commit f5baa12
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
13 changes: 13 additions & 0 deletions spec/bindingAttributeBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,19 @@ describe('Binding attribute syntax', function() {
expect(testNode).toContainText("my value");
});

it('Binding context should hide or not minify extra internal properties', function () {
testNode.innerHTML = "<div data-bind='with: $data'><div></div></div>";
ko.applyBindings({}, testNode);

var allowedProperties = ['$parents', '$root', 'ko', '$rawData', '$data', '$parentContext', '$parent'];
if (ko.utils.createSymbolOrString('') === '') {
allowedProperties.push('_subscribable');
}
ko.utils.objectForEach(ko.contextFor(testNode.childNodes[0].childNodes[0]), function (prop) {
expect(allowedProperties).toContain(prop);
});
});

it('Should be able to retrieve the binding context associated with any node', function() {
testNode.innerHTML = "<div><div data-bind='text: name'></div></div>";
ko.applyBindings({ name: 'Bert' }, testNode.childNodes[0]);
Expand Down
25 changes: 14 additions & 11 deletions src/binding/bindingAttributeSyntax.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
(function () {
// Hide or don't minify context properties, see https://github.com/knockout/knockout/issues/2294
var contextSubscribable = ko.utils.createSymbolOrString('_subscribable');

ko.bindingHandlers = {};

// The following element types will not be recursed into during binding.
Expand Down Expand Up @@ -38,14 +41,14 @@
if (parentContext) {
// When a "parent" context is given, register a dependency on the parent context. Thus whenever the
// parent context is updated, this context will also be updated.
if (parentContext._subscribable)
parentContext._subscribable();
if (parentContext[contextSubscribable])
parentContext[contextSubscribable]();

// Copy $root and any custom properties from the parent context
ko.utils.extend(self, parentContext);

// Because the above copy overwrites our own properties, we need to reset them.
self._subscribable = subscribable;
self[contextSubscribable] = subscribable;
} else {
self['$parents'] = [];
self['$root'] = dataItem;
Expand Down Expand Up @@ -97,7 +100,7 @@
// computed will be inactive, and we can safely throw it away. If it's active, the computed is stored in
// the context object.
if (subscribable.isActive()) {
self._subscribable = subscribable;
self[contextSubscribable] = subscribable;

// Always notify because even if the model ($data) hasn't changed, other context properties might have changed
subscribable['equalityComparer'] = null;
Expand All @@ -115,7 +118,7 @@
ko.utils.arrayRemoveItem(nodes, node);
if (!nodes.length) {
subscribable.dispose();
self._subscribable = subscribable = undefined;
self[contextSubscribable] = subscribable = undefined;
}
});
};
Expand Down Expand Up @@ -144,8 +147,8 @@
// Similarly to "child" contexts, provide a function here to make sure that the correct values are set
// when an observable view model is updated.
ko.bindingContext.prototype['extend'] = function(properties) {
// If the parent context references an observable view model, "_subscribable" will always be the
// latest view model object. If not, "_subscribable" isn't set, and we can use the static "$data" value.
// If the parent context references an observable view model, "contextSubscribable" will always be the
// latest view model object. If not, "contextSubscribable" isn't set, and we can use the static "$data" value.
return new ko.bindingContext(inheritParentVm, this, null, function(self, parentContext) {
ko.utils.extend(self, typeof(properties) == "function" ? properties() : properties);
});
Expand Down Expand Up @@ -294,8 +297,8 @@
}

ko.utils.domData.set(node, boundElementDomDataKey, {context: bindingContext});
if (bindingContext._subscribable)
bindingContext._subscribable._addNode(node);
if (bindingContext[contextSubscribable])
bindingContext[contextSubscribable]._addNode(node);
}

// Use bindings if given, otherwise fall back on asking the bindings provider to give us some bindings
Expand All @@ -312,8 +315,8 @@
function() {
bindings = sourceBindings ? sourceBindings(bindingContext, node) : getBindings.call(provider, node, bindingContext);
// Register a dependency on the binding context to support observable view models.
if (bindings && bindingContext._subscribable)
bindingContext._subscribable();
if (bindings && bindingContext[contextSubscribable])
bindingContext[contextSubscribable]();
return bindings;
},
null, { disposeWhenNodeIsRemoved: node }
Expand Down
1 change: 1 addition & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,7 @@ ko.exportSymbol('utils.arrayMap', ko.utils.arrayMap);
ko.exportSymbol('utils.arrayPushAll', ko.utils.arrayPushAll);
ko.exportSymbol('utils.arrayRemoveItem', ko.utils.arrayRemoveItem);
ko.exportSymbol('utils.cloneNodes', ko.utils.cloneNodes);
ko.exportSymbol('utils.createSymbolOrString', ko.utils.createSymbolOrString);
ko.exportSymbol('utils.extend', ko.utils.extend);
ko.exportSymbol('utils.fieldsIncludedWithJsonPost', ko.utils.fieldsIncludedWithJsonPost);
ko.exportSymbol('utils.getFormFields', ko.utils.getFormFields);
Expand Down

0 comments on commit f5baa12

Please sign in to comment.