Skip to content

Commit

Permalink
fix: sanitize class string more (#925)
Browse files Browse the repository at this point in the history
* fix: sanitize class string more

* fix

* format comment

* extend tests
  • Loading branch information
pauldambra authored Dec 7, 2023
1 parent 673e7ab commit 7ee1b74
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/__tests__/autocapture-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ describe(`Autocapture utility functions`, () => {
})
it('should cope with a class list with unexpected new lines', () => {
const el = document!.createElement('div')
el.className = ' class1\r\n \r\n \n class2 '
el.className = ' class1\r\n \r\n \n \t\f class2 \t'
const classNames = getClassNames(el)
expect(classNames).toEqual(['class1', 'class2'])
})
Expand Down
32 changes: 30 additions & 2 deletions src/__tests__/autocapture.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1085,19 +1085,47 @@ describe('Autocapture system', () => {
const e = {
target: elTarget,
type: 'click',
}
} as unknown as MouseEvent

const newLib = {
...lib,
elementsChainAsString: true,
}
} as PostHog

autocapture._captureEvent(e, newLib)
const props1 = getCapturedProps(newLib.capture)

expect(props1['$elements_chain']).toBeDefined()
expect(props1['$elements']).toBeUndefined()
})

it('returns elementsChain correctly with newlines in css', () => {
const elTarget = document.createElement('a')
elTarget.setAttribute('href', 'http://test.com')
elTarget.setAttribute(
'class',
'\ftest-class\n test-class2\ttest-class3 test-class4 \r\n test-class5'
)
const elParent = document.createElement('span')
elParent.appendChild(elTarget)

const e = {
target: elTarget,
type: 'click',
} as unknown as MouseEvent

const newLib = {
...lib,
elementsChainAsString: true,
} as PostHog

autocapture._captureEvent(e, newLib)
const props1 = getCapturedProps(newLib.capture)

expect(props1['$elements_chain']).toBe(
'a.test-class,test-class2,test-class3,test-class4,test-class5:nth-child="1"nth-of-type="1"href="http://test.com"attr__href="http://test.com"attr__class="test-class,test-class2,test-class3,test-class4,test-class5";span:nth-child="1"nth-of-type="1"'
)
})
})

describe('_addDomEventHandlers', () => {
Expand Down
8 changes: 6 additions & 2 deletions src/autocapture-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { _isArray, _isNull, _isString, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'
import { window } from './utils/globals'

export function splitClassString(s: string): string[] {
return s ? _trim(s).split(/\s+/) : []
}

/*
* Get the className of an element, accounting for edge cases where element.className is an object
*
Expand All @@ -26,7 +30,7 @@ export function getClassNames(el: Element): string[] {
className = ''
}

return className.length ? _trim(className).split(/\s+/) : []
return splitClassString(className)
}

/*
Expand Down Expand Up @@ -444,6 +448,6 @@ function extractAttrClass(el: Properties): PHElement['attr_class'] {
} else if (_isArray(attr_class)) {
return attr_class
} else {
return attr_class.split(' ')
return splitClassString(attr_class)
}
}
10 changes: 9 additions & 1 deletion src/autocapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
isDocumentFragment,
getDirectAndNestedSpanText,
getElementsChainString,
splitClassString,
} from './autocapture-utils'
import RageClick from './extensions/rageclick'
import { AutocaptureConfig, AutoCaptureCustomProperty, DecideResponse, Properties } from './types'
Expand Down Expand Up @@ -104,7 +105,14 @@ const autocapture = {
if (elementAttributeIgnorelist?.includes(attr.name)) return

if (!maskInputs && shouldCaptureValue(attr.value) && !isAngularStyleAttr(attr.name)) {
props['attr__' + attr.name] = limitText(1024, attr.value)
let value = attr.value
if (attr.name === 'class') {
// html attributes can _technically_ contain linebreaks,
// but we're very intolerant of them in the class string,
// so we strip them.
value = splitClassString(value).join()
}
props['attr__' + attr.name] = limitText(1024, value)
}
})

Expand Down

0 comments on commit 7ee1b74

Please sign in to comment.