diff --git a/examples/monaco/index.html b/examples/monaco/index.html index cfd3cd0c..151ca7b3 100644 --- a/examples/monaco/index.html +++ b/examples/monaco/index.html @@ -14,10 +14,7 @@ } - - - diff --git a/package.json b/package.json index 369316de..21dcfdc3 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,8 @@ "postpublish": "tag-dist-files --overwrite-existing-tag" }, "files": [ - "y.*" + "y.*", + "src/*" ], "standard": { "ignore": [ diff --git a/src/Type/y-xml/YXmlElement.js b/src/Type/y-xml/YXmlElement.js index 3af417be..b2205d34 100644 --- a/src/Type/y-xml/YXmlElement.js +++ b/src/Type/y-xml/YXmlElement.js @@ -34,16 +34,15 @@ export default class YXmlElement extends YXmlFragment { } else { // tag is already set in constructor // set attributes - let attrNames = [] + let attributes = new Map() for (let i = 0; i < dom.attributes.length; i++) { - attrNames.push(dom.attributes[i].name) - } - attrNames = this._domFilter(dom, attrNames) - for (let i = 0; i < attrNames.length; i++) { - let attrName = attrNames[i] - let attrValue = dom.getAttribute(attrName) - this.setAttribute(attrName, attrValue) + let attr = dom.attributes[i] + attributes.set(attr.name, attr.value) } + attributes = this._domFilter(dom, attributes) + attributes.forEach((value, name) => { + this.setAttribute(name, value) + }) this.insertDomElements(0, Array.prototype.slice.call(dom.childNodes), _document) this._bindToDom(dom, _document) return dom @@ -104,7 +103,9 @@ export default class YXmlElement extends YXmlFragment { getAttributes () { const obj = {} for (let [key, value] of this._map) { - obj[key] = value._content[0] + if (!value._deleted) { + obj[key] = value._content[0] + } } return obj } diff --git a/src/Type/y-xml/YXmlFragment.js b/src/Type/y-xml/YXmlFragment.js index eadc1f47..792075f4 100644 --- a/src/Type/y-xml/YXmlFragment.js +++ b/src/Type/y-xml/YXmlFragment.js @@ -15,7 +15,7 @@ function domToYXml (parent, doms, _document) { if (d._yxml != null && d._yxml !== false) { d._yxml._unbindFromDom() } - if (parent._domFilter(d, []) !== null) { + if (parent._domFilter(d.nodeName, new Map()) !== null) { let type if (d.nodeType === d.TEXT_NODE) { type = new YXmlText(d) @@ -203,6 +203,52 @@ export default class YXmlFragment extends YArray { } this._y.on('beforeTransaction', beforeTransactionSelectionFixer) this._y.on('afterTransaction', afterTransactionSelectionFixer) + const applyFilter = (type) => { + if (type._deleted) { + return + } + // check if type is a child of this + let isChild = false + let p = type + while (p !== this._y) { + if (p === this) { + isChild = true + break + } + p = p._parent + } + if (!isChild) { + return + } + // filter attributes + let attributes = new Map() + if (type.getAttributes !== undefined) { + let attrs = type.getAttributes() + for (let key in attrs) { + attributes.set(key, attrs[key]) + } + } + let result = this._domFilter(type.nodeName, new Map(attributes)) + if (result === null) { + type._delete(this._y) + } else { + attributes.forEach((value, key) => { + if (!result.has(key)) { + type.removeAttribute(key) + } + }) + } + } + this._y.on('beforeObserverCalls', function (y, transaction) { + // apply dom filter to new and changed types + transaction.changedTypes.forEach(function (subs, type) { + if (subs.size > 1 || !subs.has(null)) { + // only apply changes on attributes + applyFilter(type) + } + }) + transaction.newTypes.forEach(applyFilter) + }) // Apply Y.Xml events to dom this.observeDeep(events => { reflectChangesOnDom.call(this, events, _document) @@ -240,10 +286,15 @@ export default class YXmlFragment extends YArray { } break case 'attributes': + if (yxml.constructor === YXmlFragment) { + break + } let name = mutation.attributeName + let val = dom.getAttribute(name) // check if filter accepts attribute - if (this._domFilter(dom, [name]).length > 0 && yxml.constructor !== YXmlFragment) { - var val = dom.getAttribute(name) + let attributes = new Map() + attributes.set(name, val) + if (this._domFilter(dom.nodeName, attributes).size > 0 && yxml.constructor !== YXmlFragment) { if (yxml.getAttribute(name) !== val) { if (val == null) { yxml.removeAttribute(name) diff --git a/src/Y.js b/src/Y.js index 67fc7d60..ff7884dc 100644 --- a/src/Y.js +++ b/src/Y.js @@ -54,6 +54,7 @@ export default class Y extends NamedEventHandler { console.error(e) } if (initialCall) { + this.emit('beforeObserverCalls', this, this._transaction, remote) const transaction = this._transaction this._transaction = null // emit change events on changed types @@ -94,17 +95,10 @@ export default class Y extends NamedEventHandler { define (name, TypeConstructor) { let id = new RootID(name, TypeConstructor) let type = this.os.get(id) - if (type === null) { - type = new TypeConstructor() - type._id = id - type._parent = this - type._integrate(this) - if (this.share[name] !== undefined) { - throw new Error('Type is already defined with a different constructor!') - } - } if (this.share[name] === undefined) { this.share[name] = type + } else if (this.share[name] !== type) { + throw new Error('Type is already defined with a different constructor') } return type } diff --git a/test/y-xml.tests.js b/test/y-xml.tests.js index ccf2ba56..4f05d072 100644 --- a/test/y-xml.tests.js +++ b/test/y-xml.tests.js @@ -230,8 +230,8 @@ test('filter node', async function xml14 (t) { var { users, xml0, xml1 } = await initArrays(t, { users: 3 }) let dom0 = xml0.getDom() let dom1 = xml1.getDom() - let domFilter = (node, attrs) => { - if (node.nodeName === 'H1') { + let domFilter = (nodeName, attrs) => { + if (nodeName === 'H1') { return null } else { return attrs @@ -251,8 +251,9 @@ test('filter attribute', async function xml15 (t) { var { users, xml0, xml1 } = await initArrays(t, { users: 3 }) let dom0 = xml0.getDom() let dom1 = xml1.getDom() - let domFilter = (node, attrs) => { - return attrs.filter(name => name !== 'hidden') + let domFilter = (nodeName, attrs) => { + attrs.delete('hidden') + return attrs } xml0.setDomFilter(domFilter) xml1.setDomFilter(domFilter) @@ -303,6 +304,51 @@ test('treeWalker', async function xml17 (t) { await compareUsers(t, users) }) +/** + * The expected behavior is that changes on your own dom (e.g. malicious attributes) persist. + * Yjs should just ignore them, never propagate those attributes. + * Incoming changes that contain malicious attributes should be deleted. + */ +test('Filtering remote changes', async function xmlFilteringRemote (t) { + var { users, xml0, xml1 } = await initArrays(t, { users: 3 }) + xml0.setDomFilter(function (nodeName, attributes) { + attributes.delete('malicious') + if (nodeName === 'HIDEME') { + return null + } else if (attributes.has('isHidden')) { + return null + } else { + return attributes + } + }) + // make sure that dom filters are active + // TODO: do not rely on .getDom for domFilters + xml0.getDom() + xml1.getDom() + let paragraph = new Y.XmlElement('p') + let hideMe = new Y.XmlElement('hideMe') + let span = new Y.XmlElement('span') + span.setAttribute('malicious', 'alert("give me money")') + let tag = new Y.XmlElement('tag') + tag.setAttribute('isHidden', 'true') + paragraph.insert(0, [hideMe, span, tag]) + xml0.insert(0, [paragraph]) + let tag2 = new Y.XmlElement('tag') + tag2.setAttribute('isHidden', 'true') + paragraph.insert(0, [tag2]) + await flushAll(t, users) + // check dom + paragraph.getDom().setAttribute('malicious', 'true') + span.getDom().setAttribute('malicious', 'true') + console.log(xml0.toString()) + // check incoming attributes + xml1.get(0).get(0).setAttribute('malicious', 'true') + xml1.insert(0, [new Y.XmlElement('hideMe')]) + await flushAll(t, users) + + await compareUsers(t, users) +}) + // TODO: move elements var xmlTransactions = [ function attributeChange (t, user, chance) { diff --git a/tests-lib/helper.js b/tests-lib/helper.js index 03c7722b..317ab5d9 100644 --- a/tests-lib/helper.js +++ b/tests-lib/helper.js @@ -153,12 +153,12 @@ export async function initArrays (t, opts) { result['array' + i] = y.define('array', Y.Array) result['map' + i] = y.define('map', Y.Map) result['xml' + i] = y.define('xml', Y.XmlElement) - y.get('xml').setDomFilter(function (d, attrs) { - if (d.nodeName === 'HIDDEN') { + y.get('xml').setDomFilter(function (nodeName, attrs) { + if (nodeName === 'HIDDEN') { return null - } else { - return attrs.filter(a => a !== 'hidden') } + attrs.delete('hidden') + return attrs }) y.on('afterTransaction', function () { for (let missing of y._missingStructs.values()) {