From cdb46752a9a84f4f2219cab74ba669ada53081f9 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 30 Apr 2024 20:02:13 +0200 Subject: [PATCH 1/5] fix: ensure tilelayer attribution with smart text is displayed as HMTL --- umap/static/umap/js/umap.core.js | 2 +- umap/tests/integration/test_tilelayer.py | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/umap/static/umap/js/umap.core.js b/umap/static/umap/js/umap.core.js index aae48995..06313859 100644 --- a/umap/static/umap/js/umap.core.js +++ b/umap/static/umap/js/umap.core.js @@ -73,7 +73,7 @@ L.DomUtil.add = (tagName, className, container, content) => { if (content.nodeType && content.nodeType === 1) { el.appendChild(content) } else { - el.textContent = content + el.innerHTML = content } } return el diff --git a/umap/tests/integration/test_tilelayer.py b/umap/tests/integration/test_tilelayer.py index c6897935..2a2f35ae 100644 --- a/umap/tests/integration/test_tilelayer.py +++ b/umap/tests/integration/test_tilelayer.py @@ -101,9 +101,9 @@ def test_map_should_display_custom_tilelayer(map, live_server, tilelayers, page) url_pattern = re.compile( r"https://[abc]{1}.basemaps.cartocdn.com/rastertiles/voyager/\d+/\d+/\d+.png" ) - map.settings["properties"]["tilelayer"]["url_template"] = ( - "https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}{r}.png" - ) + map.settings["properties"]["tilelayer"][ + "url_template" + ] = "https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}{r}.png" map.settings["properties"]["tilelayersControl"] = True map.save() page.goto(f"{live_server.url}{map.get_absolute_url()}") @@ -112,3 +112,13 @@ def test_map_should_display_custom_tilelayer(map, live_server, tilelayers, page) iconTiles = page.locator(".leaflet-iconLayers .leaflet-iconLayers-layer") # The second of the list should be the current expect(iconTiles.nth(1)).to_have_css("background-image", url_pattern) + + +def test_can_have_smart_text_in_attribution(tilelayer, map, live_server, page): + map.settings["properties"]["tilelayer"][ + "attribution" + ] = "© [[http://www.openstreetmap.org/copyright|OpenStreetMap]] contributors" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + expect(page.get_by_text("© OpenStreetMap contributors")).to_be_visible() + expect(page.get_by_role("link", name="OpenStreetMap")).to_be_visible() From dccb93c8a8002dca2ab5e8ecaefc5c8b3628d7c1 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Tue, 30 Apr 2024 23:19:19 +0200 Subject: [PATCH 2/5] fix: make sure we escape all innerHTML calls --- umap/static/umap/js/umap.controls.js | 76 +++++++++++++++---------- umap/static/umap/js/umap.core.js | 7 ++- umap/static/umap/js/umap.forms.js | 36 ++++++------ umap/static/umap/js/umap.permissions.js | 11 +++- umap/static/umap/js/umap.popup.js | 2 +- umap/static/umap/js/umap.share.js | 2 +- umap/static/umap/js/umap.ui.js | 8 +-- 7 files changed, 85 insertions(+), 57 deletions(-) diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index a77d7ae6..3d6da688 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -669,20 +669,29 @@ const ControlsMixin = { L.DomUtil.createTitle(container, this.options.name, 'icon-caption') this.permissions.addOwnerLink('h5', container) if (this.options.description) { - const description = L.DomUtil.create('div', 'umap-map-description', container) - description.innerHTML = U.Utils.toHTML(this.options.description) + const description = L.DomUtil.element( + 'div', + { + className: 'umap-map-description', + safeHTML: U.Utils.toHTML(this.options.description), + }, + container + ) } const datalayerContainer = L.DomUtil.create('div', 'datalayer-container', container) this.eachVisibleDataLayer((datalayer) => { if (!datalayer.options.inCaption) return const p = L.DomUtil.create('p', 'datalayer-legend', datalayerContainer), legend = L.DomUtil.create('span', '', p), - headline = L.DomUtil.create('strong', '', p), - description = L.DomUtil.create('span', '', p) + headline = L.DomUtil.create('strong', '', p) datalayer.onceLoaded(function () { datalayer.renderLegend(legend) if (datalayer.options.description) { - description.innerHTML = U.Utils.toHTML(datalayer.options.description) + L.DomUtil.element( + 'span', + { safeHTML: U.Utils.toHTML(datalayer.options.description) }, + p + ) } }) datalayer.renderToolbox(headline) @@ -692,11 +701,12 @@ const ControlsMixin = { credits = L.DomUtil.createFieldset(creditsContainer, L._('Credits')) title = L.DomUtil.add('h5', '', credits, L._('User content credits')) if (this.options.shortCredit || this.options.longCredit) { - L.DomUtil.add( + L.DomUtil.element( 'p', - '', - credits, - U.Utils.toHTML(this.options.longCredit || this.options.shortCredit) + { + safeHTML: U.Utils.toHTML(this.options.longCredit || this.options.shortCredit), + }, + credits ) } if (this.options.licence) { @@ -718,21 +728,26 @@ const ControlsMixin = { L.DomUtil.create('hr', '', credits) title = L.DomUtil.create('h5', '', credits) title.textContent = L._('Map background credits') - const tilelayerCredit = L.DomUtil.create('p', '', credits), - name = L.DomUtil.create('strong', '', tilelayerCredit), - attribution = L.DomUtil.create('span', '', tilelayerCredit) - name.textContent = `${this.selected_tilelayer.options.name} ` - attribution.innerHTML = this.selected_tilelayer.getAttribution() + const tilelayerCredit = L.DomUtil.create('p', '', credits) + L.DomUtil.element( + 'strong', + { textContent: `${this.selected_tilelayer.options.name} ` }, + tilelayerCredit + ) + L.DomUtil.element( + 'span', + { safeHTML: this.selected_tilelayer.getAttribution() }, + tilelayerCredit + ) L.DomUtil.create('hr', '', credits) - const umapCredit = L.DomUtil.create('p', '', credits), - urls = { - leaflet: 'http://leafletjs.com', - django: 'https://www.djangoproject.com', - umap: 'http://wiki.openstreetmap.org/wiki/UMap', - changelog: 'https://umap-project.readthedocs.io/en/master/changelog/', - version: this.options.umap_version, - } - umapCredit.innerHTML = L._( + const urls = { + leaflet: 'http://leafletjs.com', + django: 'https://www.djangoproject.com', + umap: 'http://wiki.openstreetmap.org/wiki/UMap', + changelog: 'https://umap-project.readthedocs.io/en/master/changelog/', + version: this.options.umap_version, + } + const creditHTML = L._( ` Powered by Leaflet and Django, @@ -741,6 +756,7 @@ const ControlsMixin = { `, urls ) + L.DomUtil.element('p', { innerHTML: creditHTML }, credits) this.panel.open({ content: container }) }, @@ -1052,16 +1068,16 @@ U.AttributionControl = L.Control.Attribution.extend({ // Use our own container, so we can hide/show on small screens const credits = this._container.innerHTML this._container.innerHTML = '' - const container = L.DomUtil.add( - 'div', - 'attribution-container', - this._container, - credits - ) + const container = L.DomUtil.create('div', 'attribution-container', this._container) + container.innerHTML = credits const shortCredit = this._map.getOption('shortCredit'), captionMenus = this._map.getOption('captionMenus') if (shortCredit) { - L.DomUtil.add('span', '', container, ` — ${U.Utils.toHTML(shortCredit)}`) + L.DomUtil.element( + 'span', + { safeHTML: ` — ${U.Utils.toHTML(shortCredit)}` }, + container + ) } if (captionMenus) { const link = L.DomUtil.add('a', '', container, ` — ${L._('About')}`) diff --git a/umap/static/umap/js/umap.core.js b/umap/static/umap/js/umap.core.js index 06313859..24ebe4a2 100644 --- a/umap/static/umap/js/umap.core.js +++ b/umap/static/umap/js/umap.core.js @@ -73,7 +73,7 @@ L.DomUtil.add = (tagName, className, container, content) => { if (content.nodeType && content.nodeType === 1) { el.appendChild(content) } else { - el.innerHTML = content + el.textContent = content } } return el @@ -165,6 +165,11 @@ L.DomUtil.classIf = (el, className, bool) => { L.DomUtil.element = (what, attrs, parent) => { const el = document.createElement(what) + if (attrs.innerHTML) { + attrs.innerHTML = U.Utils.escapeHTML(attrs.innerHTML) + } else if (attrs.safeHTML) { + attrs.innerHTML = attrs.safeHTML + } for (const attr in attrs) { el[attr] = attrs[attr] } diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index 8f9619e2..22ea7d7f 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -757,12 +757,12 @@ L.FormBuilder.FacetSearchChoices = L.FormBuilder.Element.extend({ }, buildLabel: function () { - this.label = L.DomUtil.element('legend', {textContent: this.options.label}) + this.label = L.DomUtil.element('legend', { textContent: this.options.label }) }, buildLi: function (value) { const property_li = L.DomUtil.create('li', '', this.ul) - const label = L.DomUtil.add('label', '', property_li) + const label = L.DomUtil.create('label', '', property_li) const input = L.DomUtil.create('input', '', label) L.DomUtil.add('span', '', label, value) @@ -800,14 +800,14 @@ L.FormBuilder.MinMaxBase = L.FormBuilder.Element.extend({ build: function () { this.container = L.DomUtil.create('fieldset', 'umap-facet', this.parentNode) this.container.appendChild(this.label) - const {min, max, type} = this.options.criteria + const { min, max, type } = this.options.criteria this.type = type this.inputType = this.getInputType(this.type) const [minLabel, maxLabel] = this.getLabels() this.minLabel = L.DomUtil.create('label', '', this.container) - this.minLabel.innerHTML = minLabel + this.minLabel.textContent = minLabel this.minInput = L.DomUtil.create('input', '', this.minLabel) this.minInput.type = this.inputType @@ -817,9 +817,8 @@ L.FormBuilder.MinMaxBase = L.FormBuilder.Element.extend({ this.minInput.dataset.value = min } - this.maxLabel = L.DomUtil.create('label', '', this.container) - this.maxLabel.innerHTML = maxLabel + this.maxLabel.textContent = maxLabel this.maxInput = L.DomUtil.create('input', '', this.maxLabel) this.maxInput.type = this.inputType @@ -834,7 +833,7 @@ L.FormBuilder.MinMaxBase = L.FormBuilder.Element.extend({ }, buildLabel: function () { - this.label = L.DomUtil.element('legend', {textContent: this.options.label}) + this.label = L.DomUtil.element('legend', { textContent: this.options.label }) }, toJS: function () { @@ -974,22 +973,25 @@ L.FormBuilder.Range = L.FormBuilder.FloatInput.extend({ }, buildHelpText: function () { - const datalist = L.DomUtil.create( - 'datalist', - 'umap-field-datalist', - this.getHelpTextParent() - ) - datalist.id = `range-${this.options.label || this.name}` - this.input.setAttribute('list', datalist.id) let options = '' - const step = this.options.step || 1, - digits = step < 1 ? 1 : 0 + const step = this.options.step || 1 + const digits = step < 1 ? 1 : 0 + const id = `range-${this.options.label || this.name}` for (let i = this.options.min; i <= this.options.max; i += this.options.step) { options += `` } - datalist.innerHTML = options + const datalist = L.DomUtil.element( + 'datalist', + { + className: 'umap-field-datalist', + safeHTML: options, + id: id, + }, + this.getHelpTextParent() + ) + this.input.setAttribute('list', id) L.FormBuilder.Input.prototype.buildHelpText.call(this) }, }) diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js index dcdd2bfd..7b33ff73 100644 --- a/umap/static/umap/js/umap.permissions.js +++ b/umap/static/umap/js/umap.permissions.js @@ -62,9 +62,14 @@ U.MapPermissions = L.Class.extend({ L.DomUtil.createTitle(container, L._('Update permissions'), 'icon-key') if (this.isAnonymousMap()) { if (this.options.anonymous_edit_url) { - const helpText = `${L._('Secret edit link:')}
${this.options.anonymous_edit_url - }` - L.DomUtil.add('p', 'help-text', container, helpText) + const helpText = `${L._('Secret edit link:')}
${ + this.options.anonymous_edit_url + }` + L.DomUtil.element( + 'p', + { className: 'help-text', innerHTML: helpText }, + container + ) fields.push([ 'options.edit_status', { diff --git a/umap/static/umap/js/umap.popup.js b/umap/static/umap/js/umap.popup.js index 2421c954..6e9a96d8 100644 --- a/umap/static/umap/js/umap.popup.js +++ b/umap/static/umap/js/umap.popup.js @@ -190,7 +190,7 @@ U.PopupTemplate.Table = U.PopupTemplate.BaseWithTitle.extend({ addRow: function (container, key, value) { const tr = L.DomUtil.create('tr', '', container) L.DomUtil.add('th', '', tr, key) - L.DomUtil.add('td', '', tr, this.formatRow(key, value)) + L.DomUtil.element('td', { innerHTML: this.formatRow(key, value) }, tr) }, renderBody: function () { diff --git a/umap/static/umap/js/umap.share.js b/umap/static/umap/js/umap.share.js index 1c8650ec..459183ac 100644 --- a/umap/static/umap/js/umap.share.js +++ b/umap/static/umap/js/umap.share.js @@ -143,7 +143,7 @@ U.Share = L.Class.extend({ } const iframeExporter = new U.IframeExporter(this.map) const buildIframeCode = () => { - iframe.innerHTML = iframeExporter.build() + iframe.textContent = iframeExporter.build() exportUrl.value = window.location.protocol + iframeExporter.buildUrl() } buildIframeCode() diff --git a/umap/static/umap/js/umap.ui.js b/umap/static/umap/js/umap.ui.js index 4f238244..f7cb4a4f 100644 --- a/umap/static/umap/js/umap.ui.js +++ b/umap/static/umap/js/umap.ui.js @@ -51,13 +51,13 @@ U.UI = L.Evented.extend({ close, this ) - L.DomUtil.add('i', 'umap-close-icon', closeButton) + L.DomUtil.create('i', 'umap-close-icon', closeButton) const label = L.DomUtil.create('span', '', closeButton) label.title = label.textContent = L._('Close') - L.DomUtil.add('div', '', this._alert, e.content) + L.DomUtil.element('div', {innerHTML: e.content}, this._alert) if (e.actions) { let action, el, input - const form = L.DomUtil.add('div', 'umap-alert-actions', this._alert) + const form = L.DomUtil.create('div', 'umap-alert-actions', this._alert) for (let i = 0; i < e.actions.length; i++) { action = e.actions[i] if (action.input) { @@ -97,7 +97,7 @@ U.UI = L.Evented.extend({ this.anchorTooltipAbsolute() } L.DomUtil.addClass(this.parent, 'umap-tooltip') - this._tooltip.innerHTML = opts.content + this._tooltip.innerHTML = U.Utils.escapeHTML(opts.content) } this.TOOLTIP_ID = window.setTimeout(L.bind(showIt, this), opts.delay || 0) const id = this.TOOLTIP_ID From c3516516cdc9e858156069a349f202497f94e30a Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 1 May 2024 15:22:08 +0200 Subject: [PATCH 3/5] chore: change DomUtil.element signature --- umap/static/umap/js/umap.autocomplete.js | 65 ++++++++------ umap/static/umap/js/umap.controls.js | 68 +++++++-------- umap/static/umap/js/umap.core.js | 26 +++--- umap/static/umap/js/umap.forms.js | 26 +++--- umap/static/umap/js/umap.importer.js | 106 +++++++++++++---------- umap/static/umap/js/umap.permissions.js | 11 +-- umap/static/umap/js/umap.popup.js | 25 ++++-- umap/static/umap/js/umap.ui.js | 13 +-- 8 files changed, 190 insertions(+), 150 deletions(-) diff --git a/umap/static/umap/js/umap.autocomplete.js b/umap/static/umap/js/umap.autocomplete.js index 3da1bdeb..a1aa930a 100644 --- a/umap/static/umap/js/umap.autocomplete.js +++ b/umap/static/umap/js/umap.autocomplete.js @@ -35,27 +35,25 @@ U.AutoComplete = L.Class.extend({ }, createInput: function () { - this.input = L.DomUtil.element( - 'input', - { - type: 'text', - placeholder: this.options.placeholder, - autocomplete: 'off', - className: this.options.className, - }, - this.el - ) + this.input = L.DomUtil.element({ + tagName: 'input', + type: 'text', + parent: this.el, + placeholder: this.options.placeholder, + autocomplete: 'off', + className: this.options.className, + }) L.DomEvent.on(this.input, 'keydown', this.onKeyDown, this) L.DomEvent.on(this.input, 'keyup', this.onKeyUp, this) L.DomEvent.on(this.input, 'blur', this.onBlur, this) }, createContainer: function () { - this.container = L.DomUtil.element( - 'ul', - { className: 'umap-autocomplete' }, - document.body - ) + this.container = L.DomUtil.element({ + tagName: 'ul', + parent: document.body, + className: 'umap-autocomplete', + }) }, resizeContainer: function () { @@ -174,8 +172,11 @@ U.AutoComplete = L.Class.extend({ }, createResult: function (item) { - const el = L.DomUtil.element('li', {}, this.container) - el.textContent = item.label + const el = L.DomUtil.element({ + tagName: 'li', + parent: this.container, + textContent: item.label, + }) const result = { item: item, el: el, @@ -276,15 +277,22 @@ U.AutoComplete.Ajax.SelectMultiple = U.AutoComplete.Ajax.extend({ initSelectedContainer: function () { return L.DomUtil.after( this.input, - L.DomUtil.element('ul', { className: 'umap-multiresult' }) + L.DomUtil.element({ tagName: 'ul', className: 'umap-multiresult' }) ) }, displaySelected: function (result) { - const result_el = L.DomUtil.element('li', {}, this.selected_container) + const result_el = L.DomUtil.element({ + tagName: 'li', + parent: this.selected_container, + }) result_el.textContent = result.item.label - const close = L.DomUtil.element('span', { className: 'close' }, result_el) - close.textContent = '×' + const close = L.DomUtil.element({ + tagName: 'span', + parent: result_el, + className: 'close', + textContent: '×', + }) L.DomEvent.on( close, 'click', @@ -302,15 +310,22 @@ U.AutoComplete.Ajax.Select = U.AutoComplete.Ajax.extend({ initSelectedContainer: function () { return L.DomUtil.after( this.input, - L.DomUtil.element('div', { className: 'umap-singleresult' }) + L.DomUtil.element({ tagName: 'div', className: 'umap-singleresult' }) ) }, displaySelected: function (result) { - const result_el = L.DomUtil.element('div', {}, this.selected_container) + const result_el = L.DomUtil.element({ + tagName: 'div', + parent: this.selected_container, + }) result_el.textContent = result.item.label - const close = L.DomUtil.element('span', { className: 'close' }, result_el) - close.textContent = '×' + const close = L.DomUtil.element({ + tagName: 'span', + parent: result_el, + className: 'close', + textContent: '×', + }) this.input.style.display = 'none' L.DomEvent.on( close, diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 3d6da688..cb30a2ab 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -669,14 +669,12 @@ const ControlsMixin = { L.DomUtil.createTitle(container, this.options.name, 'icon-caption') this.permissions.addOwnerLink('h5', container) if (this.options.description) { - const description = L.DomUtil.element( - 'div', - { - className: 'umap-map-description', - safeHTML: U.Utils.toHTML(this.options.description), - }, - container - ) + const description = L.DomUtil.element({ + tagName: 'div', + className: 'umap-map-description', + safeHTML: U.Utils.toHTML(this.options.description), + parent: container, + }) } const datalayerContainer = L.DomUtil.create('div', 'datalayer-container', container) this.eachVisibleDataLayer((datalayer) => { @@ -687,11 +685,11 @@ const ControlsMixin = { datalayer.onceLoaded(function () { datalayer.renderLegend(legend) if (datalayer.options.description) { - L.DomUtil.element( - 'span', - { safeHTML: U.Utils.toHTML(datalayer.options.description) }, - p - ) + L.DomUtil.element({ + tagName: 'span', + parent: p, + safeHTML: U.Utils.toHTML(datalayer.options.description), + }) } }) datalayer.renderToolbox(headline) @@ -701,13 +699,11 @@ const ControlsMixin = { credits = L.DomUtil.createFieldset(creditsContainer, L._('Credits')) title = L.DomUtil.add('h5', '', credits, L._('User content credits')) if (this.options.shortCredit || this.options.longCredit) { - L.DomUtil.element( - 'p', - { - safeHTML: U.Utils.toHTML(this.options.longCredit || this.options.shortCredit), - }, - credits - ) + L.DomUtil.element({ + tagName: 'p', + parent: credits, + safeHTML: U.Utils.toHTML(this.options.longCredit || this.options.shortCredit), + }) } if (this.options.licence) { const licence = L.DomUtil.add( @@ -729,16 +725,16 @@ const ControlsMixin = { title = L.DomUtil.create('h5', '', credits) title.textContent = L._('Map background credits') const tilelayerCredit = L.DomUtil.create('p', '', credits) - L.DomUtil.element( - 'strong', - { textContent: `${this.selected_tilelayer.options.name} ` }, - tilelayerCredit - ) - L.DomUtil.element( - 'span', - { safeHTML: this.selected_tilelayer.getAttribution() }, - tilelayerCredit - ) + L.DomUtil.element({ + tagName: 'strong', + parent: tilelayerCredit, + textContent: `${this.selected_tilelayer.options.name} `, + }) + L.DomUtil.element({ + tagName: 'span', + parent: tilelayerCredit, + safeHTML: this.selected_tilelayer.getAttribution(), + }) L.DomUtil.create('hr', '', credits) const urls = { leaflet: 'http://leafletjs.com', @@ -756,7 +752,7 @@ const ControlsMixin = { `, urls ) - L.DomUtil.element('p', { innerHTML: creditHTML }, credits) + L.DomUtil.element({ tagName: 'p', innerHTML: creditHTML, parent: credits }) this.panel.open({ content: container }) }, @@ -1073,11 +1069,11 @@ U.AttributionControl = L.Control.Attribution.extend({ const shortCredit = this._map.getOption('shortCredit'), captionMenus = this._map.getOption('captionMenus') if (shortCredit) { - L.DomUtil.element( - 'span', - { safeHTML: ` — ${U.Utils.toHTML(shortCredit)}` }, - container - ) + L.DomUtil.element({ + tagName: 'span', + parent: container, + safeHTML: ` — ${U.Utils.toHTML(shortCredit)}`, + }) } if (captionMenus) { const link = L.DomUtil.add('a', '', container, ` — ${L._('About')}`) diff --git a/umap/static/umap/js/umap.core.js b/umap/static/umap/js/umap.core.js index 24ebe4a2..ecf6c077 100644 --- a/umap/static/umap/js/umap.core.js +++ b/umap/static/umap/js/umap.core.js @@ -118,19 +118,21 @@ L.DomUtil.createLink = (className, container, content, url, target, title) => { } L.DomUtil.createIcon = (parent, className, title, size = 16) => { - return L.DomUtil.element( - 'i', - { className: `icon icon-${size} ${className}`, title: title || '' }, - parent - ) + return L.DomUtil.element({ + tagName: 'i', + parent: parent, + className: `icon icon-${size} ${className}`, + title: title || '', + }) } L.DomUtil.createButtonIcon = (parent, className, title, size = 16) => { - return L.DomUtil.element( - 'button', - { className: `icon icon-${size} ${className}`, title: title || '' }, - parent - ) + return L.DomUtil.element({ + tagName: 'button', + parent: parent, + className: `icon icon-${size} ${className}`, + title: title || '', + }) } L.DomUtil.createTitle = (parent, text, className, tag = 'h3') => { @@ -163,8 +165,8 @@ L.DomUtil.classIf = (el, className, bool) => { else L.DomUtil.removeClass(el, className) } -L.DomUtil.element = (what, attrs, parent) => { - const el = document.createElement(what) +L.DomUtil.element = ({ tagName, parent, ...attrs }) => { + const el = document.createElement(tagName) if (attrs.innerHTML) { attrs.innerHTML = U.Utils.escapeHTML(attrs.innerHTML) } else if (attrs.safeHTML) { diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index 22ea7d7f..4039f2d9 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -757,7 +757,10 @@ L.FormBuilder.FacetSearchChoices = L.FormBuilder.Element.extend({ }, buildLabel: function () { - this.label = L.DomUtil.element('legend', { textContent: this.options.label }) + this.label = L.DomUtil.element({ + tagName: 'legend', + textContent: this.options.label, + }) }, buildLi: function (value) { @@ -833,7 +836,10 @@ L.FormBuilder.MinMaxBase = L.FormBuilder.Element.extend({ }, buildLabel: function () { - this.label = L.DomUtil.element('legend', { textContent: this.options.label }) + this.label = L.DomUtil.element({ + tagName: 'legend', + textContent: this.options.label, + }) }, toJS: function () { @@ -982,15 +988,13 @@ L.FormBuilder.Range = L.FormBuilder.FloatInput.extend({ digits )}">` } - const datalist = L.DomUtil.element( - 'datalist', - { - className: 'umap-field-datalist', - safeHTML: options, - id: id, - }, - this.getHelpTextParent() - ) + const datalist = L.DomUtil.element({ + tagName: 'datalist', + parent: this.getHelpTextParent(), + className: 'umap-field-datalist', + safeHTML: options, + id: id, + }) this.input.setAttribute('list', id) L.FormBuilder.Input.prototype.buildHelpText.call(this) }, diff --git a/umap/static/umap/js/umap.importer.js b/umap/static/umap/js/umap.importer.js index f6b32a2e..037c8452 100644 --- a/umap/static/umap/js/umap.importer.js +++ b/umap/static/umap/js/umap.importer.js @@ -15,21 +15,24 @@ U.Importer = L.Class.extend({ this.presetBox = L.DomUtil.create('div', 'formbox', this.container) this.presetSelect = L.DomUtil.create('select', '', this.presetBox) this.fileBox = L.DomUtil.create('div', 'formbox', this.container) - this.fileInput = L.DomUtil.element( - 'input', - { type: 'file', multiple: 'multiple', autofocus: true }, - this.fileBox - ) - this.urlInput = L.DomUtil.element( - 'input', - { type: 'text', placeholder: L._('Provide an URL here') }, - this.container - ) - this.rawInput = L.DomUtil.element( - 'textarea', - { placeholder: L._('Paste your data here') }, - this.container - ) + this.fileInput = L.DomUtil.element({ + tagName: 'input', + type: 'file', + parent: this.fileBox, + multiple: 'multiple', + autofocus: true, + }) + this.urlInput = L.DomUtil.element({ + tagName: 'input', + type: 'text', + parent: this.container, + placeholder: L._('Provide an URL here'), + }) + this.rawInput = L.DomUtil.element({ + tagName: 'textarea', + parent: this.container, + placeholder: L._('Paste your data here'), + }) this.typeLabel = L.DomUtil.add( 'label', '', @@ -42,33 +45,43 @@ U.Importer = L.Class.extend({ this.container, L._('Choose the layer to import in') ) - this.clearLabel = L.DomUtil.element( - 'label', - { textContent: L._('Replace layer content'), for: 'datalayer-clear-check' }, - this.container - ) - this.submitInput = L.DomUtil.element( - 'input', - { type: 'button', value: L._('Import'), className: 'button' }, - this.container - ) + this.clearLabel = L.DomUtil.element({ + tagName: 'label', + parent: this.container, + textContent: L._('Replace layer content'), + for: 'datalayer-clear-check', + }) + this.submitInput = L.DomUtil.element({ + tagName: 'input', + type: 'button', + parent: this.container, + value: L._('Import'), + className: 'button', + }) this.map.help.button(this.typeLabel, 'importFormats') - this.typeInput = L.DomUtil.element('select', { name: 'format' }, this.typeLabel) - this.layerInput = L.DomUtil.element( - 'select', - { name: 'datalayer' }, - this.layerLabel - ) - this.clearFlag = L.DomUtil.element( - 'input', - { type: 'checkbox', name: 'clear', id: 'datalayer-clear-check' }, - this.clearLabel - ) - L.DomUtil.element( - 'option', - { value: '', textContent: L._('Choose the data format') }, - this.typeInput - ) + this.typeInput = L.DomUtil.element({ + tagName: 'select', + name: 'format', + parent: this.typeLabel, + }) + this.layerInput = L.DomUtil.element({ + tagName: 'select', + name: 'datalayer', + parent: this.layerLabel, + }) + this.clearFlag = L.DomUtil.element({ + tagName: 'input', + type: 'checkbox', + name: 'clear', + id: 'datalayer-clear-check', + parent: this.clearLabel, + }) + L.DomUtil.element({ + tagName: 'option', + value: '', + textContent: L._('Choose the data format'), + parent: this.typeInput, + }) for (let i = 0; i < this.TYPES.length; i++) { option = L.DomUtil.create('option', '', this.typeInput) option.value = option.textContent = this.TYPES[i] @@ -119,11 +132,12 @@ U.Importer = L.Class.extend({ option.value = id } }) - L.DomUtil.element( - 'option', - { value: '', textContent: L._('Import in a new layer') }, - this.layerInput - ) + L.DomUtil.element({ + tagName: 'option', + value: '', + textContent: L._('Import in a new layer'), + parent: this.layerInput, + }) }) }, diff --git a/umap/static/umap/js/umap.permissions.js b/umap/static/umap/js/umap.permissions.js index 7b33ff73..663d9528 100644 --- a/umap/static/umap/js/umap.permissions.js +++ b/umap/static/umap/js/umap.permissions.js @@ -65,11 +65,12 @@ U.MapPermissions = L.Class.extend({ const helpText = `${L._('Secret edit link:')}
${ this.options.anonymous_edit_url }` - L.DomUtil.element( - 'p', - { className: 'help-text', innerHTML: helpText }, - container - ) + L.DomUtil.element({ + tagName: 'p', + className: 'help-text', + innerHTML: helpText, + parent: container, + }) fields.push([ 'options.edit_status', { diff --git a/umap/static/umap/js/umap.popup.js b/umap/static/umap/js/umap.popup.js index 6e9a96d8..b81576a3 100644 --- a/umap/static/umap/js/umap.popup.js +++ b/umap/static/umap/js/umap.popup.js @@ -190,7 +190,11 @@ U.PopupTemplate.Table = U.PopupTemplate.BaseWithTitle.extend({ addRow: function (container, key, value) { const tr = L.DomUtil.create('tr', '', container) L.DomUtil.add('th', '', tr, key) - L.DomUtil.element('td', { innerHTML: this.formatRow(key, value) }, tr) + L.DomUtil.element({ + tagName: 'td', + parent: tr, + innerHTML: this.formatRow(key, value), + }) }, renderBody: function () { @@ -281,11 +285,12 @@ U.PopupTemplate.OSM = U.PopupTemplate.Default.extend({ } } if (props.website) { - L.DomUtil.element( - 'a', - { href: props.website, textContent: props.website }, - container - ) + L.DomUtil.element({ + tagName: 'a', + parent: container, + href: props.website, + textContent: props.website, + }) } const phone = props.phone || props['contact:phone'] if (phone) { @@ -293,7 +298,7 @@ U.PopupTemplate.OSM = U.PopupTemplate.Default.extend({ 'div', '', container, - L.DomUtil.element('a', { href: `tel:${phone}`, textContent: phone }) + L.DomUtil.element({ tagName: 'a', href: `tel:${phone}`, textContent: phone }) ) } if (props.mobile) { @@ -301,7 +306,8 @@ U.PopupTemplate.OSM = U.PopupTemplate.Default.extend({ 'div', '', container, - L.DomUtil.element('a', { + L.DomUtil.element({ + tagName: 'a', href: `tel:${props.mobile}`, textContent: props.mobile, }) @@ -322,7 +328,8 @@ U.PopupTemplate.OSM = U.PopupTemplate.Default.extend({ 'div', 'osm-link', container, - L.DomUtil.element('a', { + L.DomUtil.element({ + tagName: 'a', href: `https://www.openstreetmap.org/${id}`, textContent: L._('See on OpenStreetMap'), }) diff --git a/umap/static/umap/js/umap.ui.js b/umap/static/umap/js/umap.ui.js index f7cb4a4f..8cadc752 100644 --- a/umap/static/umap/js/umap.ui.js +++ b/umap/static/umap/js/umap.ui.js @@ -54,18 +54,19 @@ U.UI = L.Evented.extend({ L.DomUtil.create('i', 'umap-close-icon', closeButton) const label = L.DomUtil.create('span', '', closeButton) label.title = label.textContent = L._('Close') - L.DomUtil.element('div', {innerHTML: e.content}, this._alert) + L.DomUtil.element({ tagName: 'div', innerHTML: e.content, parent: this._alert }) if (e.actions) { let action, el, input const form = L.DomUtil.create('div', 'umap-alert-actions', this._alert) for (let i = 0; i < e.actions.length; i++) { action = e.actions[i] if (action.input) { - input = L.DomUtil.element( - 'input', - { className: 'umap-alert-input', placeholder: action.input }, - form - ) + input = L.DomUtil.element({ + tagName: 'input', + parent: form, + className: 'umap-alert-input', + placeholder: action.input, + }) } el = L.DomUtil.createButton( 'umap-action', From 200ee6ea108c4acd3e55c15055f77d8c35a358a4 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 1 May 2024 15:32:41 +0200 Subject: [PATCH 4/5] chore: fix formatting --- umap/tests/integration/test_tilelayer.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/umap/tests/integration/test_tilelayer.py b/umap/tests/integration/test_tilelayer.py index 2a2f35ae..e055d356 100644 --- a/umap/tests/integration/test_tilelayer.py +++ b/umap/tests/integration/test_tilelayer.py @@ -101,9 +101,9 @@ def test_map_should_display_custom_tilelayer(map, live_server, tilelayers, page) url_pattern = re.compile( r"https://[abc]{1}.basemaps.cartocdn.com/rastertiles/voyager/\d+/\d+/\d+.png" ) - map.settings["properties"]["tilelayer"][ - "url_template" - ] = "https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}{r}.png" + map.settings["properties"]["tilelayer"]["url_template"] = ( + "https://{s}.basemaps.cartocdn.com/rastertiles/voyager/{z}/{x}/{y}{r}.png" + ) map.settings["properties"]["tilelayersControl"] = True map.save() page.goto(f"{live_server.url}{map.get_absolute_url()}") @@ -115,9 +115,9 @@ def test_map_should_display_custom_tilelayer(map, live_server, tilelayers, page) def test_can_have_smart_text_in_attribution(tilelayer, map, live_server, page): - map.settings["properties"]["tilelayer"][ - "attribution" - ] = "© [[http://www.openstreetmap.org/copyright|OpenStreetMap]] contributors" + map.settings["properties"]["tilelayer"]["attribution"] = ( + "© [[http://www.openstreetmap.org/copyright|OpenStreetMap]] contributors" + ) map.save() page.goto(f"{live_server.url}{map.get_absolute_url()}") expect(page.get_by_text("© OpenStreetMap contributors")).to_be_visible() From fedb083612a9505a0c18a42855764bdc0babe9e4 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Wed, 1 May 2024 16:22:49 +0200 Subject: [PATCH 5/5] chore: be more explicit on what HTML we allow when escaping --- umap/static/umap/js/modules/utils.js | 4 ++-- umap/tests/integration/test_basics.py | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/umap/static/umap/js/modules/utils.js b/umap/static/umap/js/modules/utils.js index 5b18286e..4c6bfee3 100644 --- a/umap/static/umap/js/modules/utils.js +++ b/umap/static/umap/js/modules/utils.js @@ -70,7 +70,6 @@ export default function getPurify() { export function escapeHTML(s) { s = s ? s.toString() : '' s = getPurify().sanitize(s, { - USE_PROFILES: { html: true }, ADD_TAGS: ['iframe'], ALLOWED_TAGS: [ 'h3', @@ -86,9 +85,10 @@ export function escapeHTML(s) { 'iframe', 'img', 'br', + 'span', ], ADD_ATTR: ['target', 'allow', 'allowfullscreen', 'frameborder', 'scrolling'], - ALLOWED_ATTR: ['href', 'src', 'width', 'height'], + ALLOWED_ATTR: ['href', 'src', 'width', 'height', 'style'], // Added: `geo:` URL scheme as defined in RFC5870: // https://www.rfc-editor.org/rfc/rfc5870.html // The base RegExp comes from: diff --git a/umap/tests/integration/test_basics.py b/umap/tests/integration/test_basics.py index 206cc785..0bf0e60b 100644 --- a/umap/tests/integration/test_basics.py +++ b/umap/tests/integration/test_basics.py @@ -45,3 +45,24 @@ def test_create_map_with_cursor(page, live_server, tilelayer): "z-index: 200;" ), ) + + +def test_cannot_put_script_tag_in_datalayer_name_or_description( + openmap, live_server, page, tilelayer +): + page.goto(f"{live_server.url}{openmap.get_absolute_url()}") + page.get_by_role("button", name="Edit").click() + page.get_by_role("link", name="Manage layers").click() + page.get_by_role("button", name="Add a layer").click() + page.locator('input[name="name"]').click() + page.locator('input[name="name"]').fill('') + page.locator(".umap-field-description textarea").click() + page.locator(".umap-field-description textarea").fill( + '

before after

' + ) + page.get_by_role("button", name="Save").click() + page.get_by_role("button", name="About").click() + # Title should contain raw HTML (we are using textContent) + expect(page.get_by_text('')).to_be_visible() + # Description should contain escaped HTML + expect(page.get_by_text("before after")).to_be_visible()