fix: properly handle special characters in user-provided IDs (closes #4927, #5561) (#5564)

dietergeerts · jacobmllr95 · web-flow · commit 1fabd68bb44b · 2020-07-21T20:00:49.000+02:00
* fix(b-form-group): make it work for ids with special characters like "/" Special characters are allowed in HTML5 (https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id) but need to be escaped when used in a selector for usage in e.g. "querySelector" Refs #5561 * Use own `cssEscape()` util + use/test everywhere needed Co-authored-by: Jacob Müller <jacob.mueller.elz@gmail.com>
diff --git a/src/components/form-group/form-group.js b/src/components/form-group/form-group.js
@@ -1,3 +1,4 @@
+import cssEscape from '../../utils/css-escape'
 import memoize from '../../utils/memoize'
 import { arrayIncludes } from '../../utils/array'
 import { getBreakpointsUpCached } from '../../utils/config'
@@ -379,7 +380,8 @@ export const BFormGroup = {
       // Optionally accepts a string of IDs to remove as the second parameter.
       // Preserves any aria-describedby value(s) user may have on input.
       if (this.labelFor && isBrowser) {
-        const input = select(`#${this.labelFor}`, this.$refs.content)
+        // We need to escape `labelFor` since it can be user-provided
+        const input = select(`#${cssEscape(this.labelFor)}`, this.$refs.content)
         if (input) {
           const adb = 'aria-describedby'
           let ids = (getAttr(input, adb) || '').split(/\s+/)
diff --git a/src/components/form-group/form-group.spec.js b/src/components/form-group/form-group.spec.js
@@ -176,6 +176,32 @@ describe('form-group', () => {
     wrapper.destroy()
   })
 
+  it('sets "aria-describedby" even when special characters are used in IDs', async () => {
+    const wrapper = mount(BFormGroup, {
+      propsData: {
+        id: '/group-id',
+        label: 'test',
+        labelFor: '/input-id',
+        description: 'foo' // Description is needed to set "aria-describedby"
+      },
+      slots: {
+        default: '<input id="/input-id" type="text">'
+      }
+    })
+
+    expect(wrapper.vm).toBeDefined()
+
+    // Auto ID is created after mounted
+    await waitNT(wrapper.vm)
+
+    const $input = wrapper.find('input')
+    expect($input.exists()).toBe(true)
+    expect($input.attributes('aria-describedby')).toBeDefined()
+    expect($input.attributes('aria-describedby')).toEqual('/group-id__BV_description_')
+
+    wrapper.destroy()
+  })
+
   it('horizontal layout without prop label-for set has expected structure', async () => {
     const wrapper = mount(BFormGroup, {
       propsData: {
diff --git a/src/components/form-tags/form-tags.js b/src/components/form-tags/form-tags.js
@@ -2,6 +2,7 @@
 // Based loosely on https://adamwathan.me/renderless-components-in-vuejs/
 import Vue from '../../utils/vue'
 import KeyCodes from '../../utils/key-codes'
+import cssEscape from '../../utils/css-escape'
 import identity from '../../utils/identity'
 import looseEqual from '../../utils/loose-equal'
 import { arrayIncludes, concat } from '../../utils/array'
@@ -523,7 +524,8 @@ export const BFormTags = /*#__PURE__*/ Vue.extend({
     },
     getInput() {
       // Returns the input element reference (or null if not found)
-      return select(`#${this.computedInputId}`, this.$el)
+      // We need to escape `computedInputId` since it can be user-provided
+      return select(`#${cssEscape(this.computedInputId)}`, this.$el)
     },
     // Default User Interface render
     defaultRender({
diff --git a/src/components/form-tags/form-tags.spec.js b/src/components/form-tags/form-tags.spec.js
@@ -157,6 +157,40 @@ describe('form-tags', () => {
     wrapper.destroy()
   })
 
+  it('applies "input-id" to the input', async () => {
+    const wrapper = mount(BFormTags, {
+      propsData: {
+        inputId: '1-tag-input',
+        value: ['apple', 'orange']
+      }
+    })
+
+    expect(wrapper.element.tagName).toBe('DIV')
+    expect(wrapper.vm.tags).toEqual(['apple', 'orange'])
+    expect(wrapper.vm.newTag).toEqual('')
+
+    const $input = wrapper.find('input')
+    expect($input.exists()).toBe(true)
+    expect($input.element.value).toBe('')
+    expect($input.element.type).toBe('text')
+    expect($input.element.id).toEqual('1-tag-input')
+
+    $input.element.value = 'pear'
+    await $input.trigger('input')
+    expect(wrapper.vm.newTag).toEqual('pear')
+    expect(wrapper.vm.tags).toEqual(['apple', 'orange'])
+    await $input.trigger('change')
+    expect(wrapper.vm.newTag).toEqual('pear')
+    expect(wrapper.vm.tags).toEqual(['apple', 'orange'])
+    await wrapper.setProps({ addOnChange: true })
+    await $input.trigger('change')
+    expect(wrapper.vm.newTag).toEqual('')
+    expect(wrapper.vm.tags).toEqual(['apple', 'orange', 'pear'])
+    await wrapper.setProps({ addOnChange: false })
+
+    wrapper.destroy()
+  })
+
   it('removes tags when user clicks remove on tag', async () => {
     const wrapper = mount(BFormTags, {
       propsData: {
diff --git a/src/utils/css-escape.js b/src/utils/css-escape.js
@@ -0,0 +1,75 @@
+import { toString } from './string'
+
+const escapeChar = value => '\\' + value
+
+// The `cssEscape()` util is based on this `CSS.escape()` polyfill:
+// https://github.com/mathiasbynens/CSS.escape
+const cssEscape = value => {
+  value = toString(value)
+
+  const length = value.length
+  const firstCharCode = value.charCodeAt(0)
+
+  return value.split('').reduce((result, char, index) => {
+    const charCode = value.charCodeAt(index)
+
+    // If the character is NULL (U+0000), use (U+FFFD) as replacement
+    if (charCode === 0x0000) {
+      return result + '\uFFFD'
+    }
+
+    // If the character ...
+    if (
+      // ... is U+007F OR
+      charCode === 0x007f ||
+      // ... is in the range [\1-\1F] (U+0001 to U+001F) OR ...
+      (charCode >= 0x0001 && charCode <= 0x001f) ||
+      // ... is the first character and is in the range [0-9] (U+0030 to U+0039) OR ...
+      (index === 0 && charCode >= 0x0030 && charCode <= 0x0039) ||
+      // ... is the second character and is in the range [0-9] (U+0030 to U+0039)
+      // and the first character is a `-` (U+002D) ...
+      (index === 1 && charCode >= 0x0030 && charCode <= 0x0039 && firstCharCode === 0x002d)
+    ) {
+      // ... https://drafts.csswg.org/cssom/#escape-a-character-as-code-point
+      return result + escapeChar(`${charCode.toString(16)} `)
+    }
+
+    // If the character ...
+    if (
+      // ... is the first character AND ...
+      index === 0 &&
+      // ... is a `-` (U+002D) AND ...
+      charCode === 0x002d &&
+      // ... there is no second character ...
+      length === 1
+    ) {
+      // ... use the escaped character
+      return result + escapeChar(char)
+    }
+
+    // If the character ...
+    if (
+      // ... is greater than or equal to U+0080 OR ...
+      charCode >= 0x0080 ||
+      // ... is `-` (U+002D) OR ...
+      charCode === 0x002d ||
+      // ... is `_` (U+005F) OR ...
+      charCode === 0x005f ||
+      // ... is in the range [0-9] (U+0030 to U+0039) OR ...
+      (charCode >= 0x0030 && charCode <= 0x0039) ||
+      // ... is in the range [A-Z] (U+0041 to U+005A) OR ...
+      (charCode >= 0x0041 && charCode <= 0x005a) ||
+      // ... is in the range [a-z] (U+0061 to U+007A) ...
+      (charCode >= 0x0061 && charCode <= 0x007a)
+    ) {
+      // ... use the character itself
+      return result + char
+    }
+
+    // Otherwise use the escaped character
+    // See: https://drafts.csswg.org/cssom/#escape-a-character
+    return result + escapeChar(char)
+  }, '')
+}
+
+export default cssEscape
diff --git a/src/utils/css-escape.spec.js b/src/utils/css-escape.spec.js
@@ -0,0 +1,82 @@
+import cssEscape from './css-escape'
+
+describe('utils/cssEscape', () => {
+  it('works', () => {
+    expect(cssEscape('\0')).toBe('\uFFFD')
+    expect(cssEscape('a\0')).toBe('a\uFFFD')
+    expect(cssEscape('\0b')).toBe('\uFFFDb')
+    expect(cssEscape('a\0b')).toBe('a\uFFFDb')
+
+    expect(cssEscape('\uFFFD')).toBe('\uFFFD')
+    expect(cssEscape('a\uFFFD')).toBe('a\uFFFD')
+    expect(cssEscape('\uFFFDb')).toBe('\uFFFDb')
+    expect(cssEscape('a\uFFFDb')).toBe('a\uFFFDb')
+
+    expect(cssEscape(undefined)).toBe('')
+    expect(cssEscape(null)).toBe('')
+    expect(cssEscape(true)).toBe('true')
+    expect(cssEscape(false)).toBe('false')
+    expect(cssEscape('')).toBe('')
+
+    expect(cssEscape('\x01\x02\x1E\x1F')).toBe('\\1 \\2 \\1e \\1f ')
+
+    expect(cssEscape('0a')).toBe('\\30 a')
+    expect(cssEscape('1a')).toBe('\\31 a')
+    expect(cssEscape('2a')).toBe('\\32 a')
+    expect(cssEscape('3a')).toBe('\\33 a')
+    expect(cssEscape('4a')).toBe('\\34 a')
+    expect(cssEscape('5a')).toBe('\\35 a')
+    expect(cssEscape('6a')).toBe('\\36 a')
+    expect(cssEscape('7a')).toBe('\\37 a')
+    expect(cssEscape('8a')).toBe('\\38 a')
+    expect(cssEscape('9a')).toBe('\\39 a')
+
+    expect(cssEscape('a0b')).toBe('a0b')
+    expect(cssEscape('a1b')).toBe('a1b')
+    expect(cssEscape('a2b')).toBe('a2b')
+    expect(cssEscape('a3b')).toBe('a3b')
+    expect(cssEscape('a4b')).toBe('a4b')
+    expect(cssEscape('a5b')).toBe('a5b')
+    expect(cssEscape('a6b')).toBe('a6b')
+    expect(cssEscape('a7b')).toBe('a7b')
+    expect(cssEscape('a8b')).toBe('a8b')
+    expect(cssEscape('a9b')).toBe('a9b')
+
+    expect(cssEscape('-0a')).toBe('-\\30 a')
+    expect(cssEscape('-1a')).toBe('-\\31 a')
+    expect(cssEscape('-2a')).toBe('-\\32 a')
+    expect(cssEscape('-3a')).toBe('-\\33 a')
+    expect(cssEscape('-4a')).toBe('-\\34 a')
+    expect(cssEscape('-5a')).toBe('-\\35 a')
+    expect(cssEscape('-6a')).toBe('-\\36 a')
+    expect(cssEscape('-7a')).toBe('-\\37 a')
+    expect(cssEscape('-8a')).toBe('-\\38 a')
+    expect(cssEscape('-9a')).toBe('-\\39 a')
+
+    expect(cssEscape('-')).toBe('\\-')
+    expect(cssEscape('-a')).toBe('-a')
+    expect(cssEscape('--')).toBe('--')
+    expect(cssEscape('--a')).toBe('--a')
+
+    expect(cssEscape('\x80\x2D\x5F\xA9')).toBe('\x80\x2D\x5F\xA9')
+    expect(
+      cssEscape(
+        '\x7F\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F'
+      )
+    ).toBe(
+      '\\7f \x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F'
+    )
+    expect(cssEscape('\xA0\xA1\xA2')).toBe('\xA0\xA1\xA2')
+    expect(cssEscape('a0123456789b')).toBe('a0123456789b')
+    expect(cssEscape('abcdefghijklmnopqrstuvwxyz')).toBe('abcdefghijklmnopqrstuvwxyz')
+    expect(cssEscape('ABCDEFGHIJKLMNOPQRSTUVWXYZ')).toBe('ABCDEFGHIJKLMNOPQRSTUVWXYZ')
+
+    expect(cssEscape('\x20\x21\x78\x79')).toBe('\\ \\!xy')
+
+    // Astral symbol (U+1D306 TETRAGRAM FOR CENTRE)
+    expect(cssEscape('\uD834\uDF06')).toBe('\uD834\uDF06')
+    // Lone surrogates
+    expect(cssEscape('\uDF06')).toBe('\uDF06')
+    expect(cssEscape('\uD834')).toBe('\uD834')
+  })
+})
