Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/components/form-group/form-group.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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+/)
Expand Down
26 changes: 26 additions & 0 deletions src/components/form-group/form-group.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
4 changes: 3 additions & 1 deletion src/components/form-tags/form-tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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({
Expand Down
34 changes: 34 additions & 0 deletions src/components/form-tags/form-tags.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
75 changes: 75 additions & 0 deletions src/utils/css-escape.js
Original file line number Diff line number Diff line change
@@ -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
82 changes: 82 additions & 0 deletions src/utils/css-escape.spec.js
Original file line number Diff line number Diff line change
@@ -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')
})
})