Skip to content

Commit

Permalink
fix: seek subdomain correctly (#888)
Browse files Browse the repository at this point in the history
We relied on a regex for detecting subdomain in order to choose what cookie domain to set for PostHog storage.

This worked fine for example.com or www.example.com but it didn't work for example.co.uk or example.com.au because it would grab the first two parts and so try to set on the public suffix co.uk or com.au

A lot of suggested solutions on line involve keeping a list of public suffixes, or making DNS lookups to validate values which wouldn't fit here.

But... browsers reject cookies set on public suffixes - despite not offering an API to directly check if something is a public suffix.

So, we can split the current hostname up and then progressively try to set a cookie until the browser accepts it.

For a.long.domain.someone.is.using.com.au we would check:

.au rejected by browser
.com.au rejected by browser
.using.com.au accepted by browser
We call this method a lot and the browser can't maintain in-memory variables and navigate between sub-domains, so I also cache the result

I manually tested the code in the developer console of firefox, chrome, and safari
  • Loading branch information
pauldambra authored Nov 15, 2023
1 parent 76f058b commit 8b289a3
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 56 deletions.
45 changes: 0 additions & 45 deletions src/__tests__/storage.js

This file was deleted.

84 changes: 84 additions & 0 deletions src/__tests__/storage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { window } from '../../src/utils/globals'
import { resetSessionStorageSupported, seekFirstNonPublicSubDomain, sessionStore } from '../storage'

describe('sessionStore', () => {
describe('seekFirstNonPublicSubDomain', () => {
const mockDocumentDotCookie = {
value_: '',

get cookie() {
return this.value_
},

set cookie(value) {
//needs to refuse known public suffixes, like a browser would
// value arrives like dmn_chk_1699961248575=1;domain=.uk
const domain = value.split('domain=')
if (['.uk', '.com', '.au', '.com.au', '.co.uk'].includes(domain[1])) return
this.value_ += value + ';'
},
}
test.each([
{
candidate: 'www.google.co.uk',
expected: 'google.co.uk',
},
{
candidate: 'www.google.com',
expected: 'google.com',
},
{
candidate: 'www.google.com.au',
expected: 'google.com.au',
},
{
candidate: 'localhost',
expected: '',
},
])(`%s subdomain check`, ({ candidate, expected }) => {
expect(seekFirstNonPublicSubDomain(candidate, mockDocumentDotCookie)).toEqual(expected)
})
})

it('stores objects as strings', () => {
sessionStore.set('foo', { bar: 'baz' })
expect(sessionStore.get('foo')).toEqual('{"bar":"baz"}')
})
it('stores and retrieves an object untouched', () => {
const obj = { bar: 'baz' }
sessionStore.set('foo', obj)
expect(sessionStore.parse('foo')).toEqual(obj)
})
it('stores and retrieves a string untouched', () => {
const str = 'hey hey'
sessionStore.set('foo', str)
expect(sessionStore.parse('foo')).toEqual(str)
})
it('returns null if the key does not exist', () => {
expect(sessionStore.parse('baz')).toEqual(null)
})
it('remove deletes an item from storage', () => {
const str = 'hey hey'
sessionStore.set('foo', str)
expect(sessionStore.parse('foo')).toEqual(str)
sessionStore.remove('foo')
expect(sessionStore.parse('foo')).toEqual(null)
})

describe('sessionStore.is_supported', () => {
beforeEach(() => {
// Reset the sessionStorageSupported before each test. Otherwise, we'd just be testing the cached value.
// eslint-disable-next-line no-unused-vars
resetSessionStorageSupported()
})
it('returns false if sessionStorage is undefined', () => {
const sessionStorage = (window as any).sessionStorage
delete (window as any).sessionStorage
expect(sessionStore.is_supported()).toEqual(false)
;(window as any).sessionStorage = sessionStorage
})
it('returns true by default', () => {
expect(sessionStore.is_supported()).toEqual(true)
})
})
})
72 changes: 63 additions & 9 deletions src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,68 @@ import { DISTINCT_ID, SESSION_ID, SESSION_RECORDING_IS_SAMPLED } from './constan
import { _isNull, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'

const Y1970 = 'Thu, 01 Jan 1970 00:00:00 GMT'

/**
* Browsers don't offer a way to check if something is a public suffix
* e.g. `.com.au`, `.io`, `.org.uk`
*
* But they do reject cookies set on public suffixes
* Setting a cookie on `.co.uk` would mean it was sent for every `.co.uk` site visited
*
* So, we can use this to check if a domain is a public suffix
* by trying to set a cookie on a subdomain of the provided hostname
* until the browser accepts it
*
* inspired by https://github.com/AngusFu/browser-root-domain
*/
export function seekFirstNonPublicSubDomain(hostname: string, cookieJar = document): string {
if (['localhost', '127.0.0.1'].includes(hostname)) return ''

const list = hostname.split('.')
let len = list.length
const key = 'dmn_chk_' + +new Date()
const R = new RegExp('(^|;)\\s*' + key + '=1')

while (len--) {
const candidate = list.slice(len).join('.')
const candidateCookieValue = key + '=1;domain=.' + candidate

// try to set cookie
cookieJar.cookie = candidateCookieValue

if (R.test(cookieJar.cookie)) {
// the cookie was accepted by the browser, remove the test cookie
cookieJar.cookie = candidateCookieValue + ';expires=' + Y1970
return candidate
}
}
return ''
}

const DOMAIN_MATCH_REGEX = /[a-z0-9][a-z0-9-]+\.[a-z]{2,}$/i
const originalCookieDomainFn = (hostname: string): string => {
const matches = hostname.match(DOMAIN_MATCH_REGEX)
return matches ? matches[0] : ''
}

export function chooseCookieDomain(hostname: string, cross_subdomain: boolean | undefined): string {
if (cross_subdomain) {
// NOTE: Could we use this for cross domain tracking?
let matchedSubDomain = seekFirstNonPublicSubDomain(hostname)

if (!matchedSubDomain) {
const originalMatch = originalCookieDomainFn(hostname)
if (originalMatch !== matchedSubDomain) {
logger.info('Warning: cookie subdomain discovery mismatch', originalMatch, matchedSubDomain)
}
matchedSubDomain = originalMatch
}

return matchedSubDomain ? '; domain=.' + matchedSubDomain : ''
}
return ''
}

// Methods partially borrowed from quirksmode.org/js/cookies.html
export const cookieStore: PersistentStore = {
Expand Down Expand Up @@ -44,17 +105,10 @@ export const cookieStore: PersistentStore = {

set: function (name, value, days, cross_subdomain, is_secure) {
try {
let cdomain = '',
expires = '',
let expires = '',
secure = ''

if (cross_subdomain) {
// NOTE: Could we use this for cross domain tracking?
const matches = document.location.hostname.match(DOMAIN_MATCH_REGEX),
domain = matches ? matches[0] : ''

cdomain = domain ? '; domain=.' + domain : ''
}
const cdomain = chooseCookieDomain(document.location.hostname, cross_subdomain)

if (days) {
const date = new Date()
Expand Down
2 changes: 2 additions & 0 deletions testcafe/e2e.spec.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import { t } from 'testcafe'
import { retryUntilResults, queryAPI, initPosthog, captureLogger, staticFilesMock } from './helpers'

// eslint-disable-next-line no-undef
fixture('posthog.js capture')
.page('http://localhost:8000/playground/cypress-full/index.html')
.requestHooks(captureLogger, staticFilesMock)
.afterEach(async () => {
const browserLogs = await t.getBrowserConsoleMessages()
Object.keys(browserLogs).forEach((level) => {
browserLogs[level].forEach((line) => {
// eslint-disable-next-line no-console
console.log(`Browser ${level}:`, line)
})
})
Expand Down
12 changes: 10 additions & 2 deletions testcafe/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import fetch from 'node-fetch'

// NOTE: These tests are run against a dedicated test project in PostHog cloud
// but can be overridden to call a local API when running locally
// eslint-disable-next-line no-undef
const currentEnv = process.env
const {
POSTHOG_PROJECT_KEY,
POSTHOG_API_KEY,
POSTHOG_API_HOST = 'https://app.posthog.com',
POSTHOG_API_PROJECT = '11213',
} = process.env
} = currentEnv

const HEADERS = { Authorization: `Bearer ${POSTHOG_API_KEY}` }

Expand All @@ -26,18 +28,20 @@ export const captureLogger = RequestLogger(/ip=1/, {
export const staticFilesMock = RequestMock()
.onRequestTo(/array.full.js/)
.respond((req, res) => {
// eslint-disable-next-line no-undef
const arrayjs = fs.readFileSync(path.resolve(__dirname, '../dist/array.full.js'))
res.setBody(arrayjs)
})
.onRequestTo(/playground/)
.respond((req, res) => {
// eslint-disable-next-line no-undef
const html = fs.readFileSync(path.resolve(__dirname, '../playground/cypress-full/index.html'))
res.setBody(html)
})

export const initPosthog = (config) => {
return ClientFunction((configParams = {}) => {
var testSessionId = Math.round(Math.random() * 10000000000).toString()
const testSessionId = Math.round(Math.random() * 10000000000).toString()
configParams.debug = true
window.posthog.init(configParams.api_key, configParams)
window.posthog.register({
Expand Down Expand Up @@ -70,6 +74,7 @@ export async function retryUntilResults(operation, target_results, limit = 6, de
if (results.length >= target_results) {
resolve(results)
} else {
// eslint-disable-next-line no-console
console.log(`Expected ${target_results} results, got ${results.length} (attempt ${count})`)
attempt(count + 1, resolve, reject)
}
Expand All @@ -78,6 +83,8 @@ export async function retryUntilResults(operation, target_results, limit = 6, de
}, delay)
}

// new Promise isn't supported in IE11, but we don't care in these tests
// eslint-disable-next-line compat/compat
return new Promise((...args) => attempt(0, ...args))
}

Expand All @@ -90,6 +97,7 @@ export async function queryAPI(testSessionId) {
const data = await response.text()

if (!response.ok) {
// eslint-disable-next-line no-console
console.error('Bad Response', response.status, data)
throw new Error('Bad Response')
}
Expand Down

0 comments on commit 8b289a3

Please sign in to comment.