Merge pull request #4607 from nextcloud-libraries/feat/nc-icon-svg-wr…

…apper-remove-svg-id

src/components/NcIconSvgWrapper/NcIconSvgWrapper.vue

@@ -91,34 +91,66 @@ export default {
 <template>
 	<span class="icon-vue"
 		role="img"
-		:aria-hidden="!name"
-		:aria-label="name"
+		:aria-hidden="!name ? true : undefined"
+		:aria-label="name || undefined"
 		v-html="cleanSvg" /> <!-- eslint-disable-line vue/no-v-html -->
 </template>
 
 <script>
+import Vue from 'vue'
 import DOMPurify from 'dompurify'
 
 export default {
 	name: 'NcIconSvgWrapper',
 
 	props: {
+		/**
+		 * Raw SVG string to render
+		 */
 		svg: {
 			type: String,
 			default: '',
 		},
+
+		/**
+		 * Label of the icon, used in aria-label
+		 */
 		name: {
 			type: String,
 			default: '',
 		},
+
+		/**
+		 * By default MDI icons have an ID on the `<svg>` element. It leads to dupliated IDs on a web-page.
+		 * This component removes the ID on the received SVG.
+		 * Use this prop to disable this behavior and to not remove the ID.
+		 */
+		keepId: {
+			type: Boolean,
+			default: false,
+		},
 	},
 
 	computed: {
 		cleanSvg() {
 			if (!this.svg) {
 				return
 			}
-			return DOMPurify.sanitize(this.svg)
+
+			const svg = DOMPurify.sanitize(this.svg)
+
+			const svgDocument = new DOMParser().parseFromString(svg, 'image/svg+xml')
+
+			if (svgDocument.querySelector('parsererror')) {
+				Vue.util.warn('SVG is not valid')
+				return ''
+			}
+
+			if (!this.keepId && svgDocument.documentElement.id) {
+				svgDocument.documentElement.removeAttribute('id')
+			}
+
+			return svgDocument.documentElement.outerHTML
 		},
 	},
 }

tests/unit/components/NcIconSvgWrapper/NcIconSvgWrapper.spec.js

@@ -0,0 +1,61 @@
+import { shallowMount } from '@vue/test-utils'
+import NcIconSvgWrapper from '../../../../src/components/NcIconSvgWrapper/index.js'
+
+// @mdi/check.svg
+const SVG_ICON = '<svg xmlns="http://www.w3.org/2000/svg" id="mdi-check" viewBox="0 0 24 24"><path d="M21,7L9,19L3.5,13.5L4.91,12.09L9,16.17L19.59,5.59L21,7Z" /></svg>'
+
+const SVG_ICON_SNAPSHOT = `<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+  <path d="M21,7L9,19L3.5,13.5L4.91,12.09L9,16.17L19.59,5.59L21,7Z"></path>
+</svg>`
+
+/**
+ * @param {NcIconSvgWrapper.props} propsData - NcIconSvgWrapper.props
+ */
+function mountNcIconSvgWrapper(propsData) {
+	return shallowMount(NcIconSvgWrapper, { propsData })
+}
+
+describe('NcIconSvgWrapper', () => {
+	it('should render SVG from svg prop in a span', () => {
+		const wrapper = mountNcIconSvgWrapper({ svg: SVG_ICON })
+		const svg = wrapper.find('svg')
+		expect(svg.exists()).toBeTruthy()
+		expect(svg.html()).toBe(SVG_ICON_SNAPSHOT)
+	})
+
+	it('should render SVG in a span with aria-hidden when no name is provided', () => {
+		const wrapper = mountNcIconSvgWrapper({ svg: SVG_ICON })
+		expect(wrapper.attributes('aria-hidden')).toBe('true')
+		expect(wrapper.attributes('aria-label')).not.toBeDefined()
+	})
+
+	it('should render SVG in a span with aria-label when name is provided', () => {
+		const wrapper = mountNcIconSvgWrapper({ svg: SVG_ICON, name: 'Check' })
+		expect(wrapper.attributes('aria-hidden')).not.toBeDefined()
+		expect(wrapper.attributes('aria-label')).toBe('Check')
+	})
+
+	it('should remove ID from rendered SVG', () => {
+		const wrapper = mountNcIconSvgWrapper({ svg: SVG_ICON })
+		const svg = wrapper.get('svg')
+		expect(svg.attributes('id')).not.toBeDefined()
+	})
+
+	it('should keep ID from rendered SVG when keepId is provided', () => {
+		const wrapper = mountNcIconSvgWrapper({ svg: SVG_ICON, keepId: true })
+		const svg = wrapper.get('svg')
+		expect(svg.attributes('id')).toBe('mdi-check')
+	})
+
+	it('should sanitize SVG', () => {
+		const svgWithXSS = `<svg xmlns="http://www.w3.org/2000/svg" id="mdi-check" viewBox="0 0 24 24">
+            <path d="M21,7L9,19L3.5,13.5L4.91,12.09L9,16.17L19.59,5.59L21,7Z" />
+            <script type="text/javascript">
+                alert("This is an example of a stored XSS attack in an SVG image")
+            </script>
+        </svg>`
+		const wrapper = mountNcIconSvgWrapper({ svg: svgWithXSS })
+		const svg = wrapper.get('svg')
+		expect(svg.find('script').exists()).toBeFalsy()
+	})
+})