Skip to content

fix: Fix distributed tracing handling of empty string fetch parameter #640

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 5 commits into from
Aug 15, 2023
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
2 changes: 1 addition & 1 deletion .github/actions/build-ab/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "@newrelic/browser-agent.actions.find-pull-request",
"name": "@newrelic/browser-agent.actions.build-ab",
"description": "",
"private": true,
"type": "module",
Expand Down
21,014 changes: 21,012 additions & 2 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@
"url": "https://github.com/newrelic/newrelic-browser-agent.git"
},
"dependencies": {
"caniuse-lite": "^1.0.30001520",
"core-js": "^3.26.0",
"fflate": "^0.7.4",
"rrweb": "^2.0.0-alpha.8",
Expand Down
2 changes: 1 addition & 1 deletion src/common/deny-list/deny-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function setDenyList (denyListConfig) {
host = url
pathname = ''
}
let [hostname, port] = host.split(':')
let [hostname] = host.split(':')

denyList.push({ hostname, pathname })
}
Expand Down
133 changes: 103 additions & 30 deletions src/common/deny-list/deny-list.test.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,104 @@
describe('Setting deny list', () => {
let DlMod
beforeEach(() => {
jest.isolateModules(() => import('./deny-list.js').then(m => DlMod = m)) // give every test its own denyList (sandbox)
})

it('respects path', () => {
DlMod.setDenyList(['bam.nr-data.net/somepath'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '/somepath' })).toBeFalsy() // shouldCollectEvent returns 'false' when there IS a match...
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '/someotherpath' })).toBeTruthy()
})

it('ignores port', () => {
DlMod.setDenyList(['bam.nr-data.net:1234'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', port: '4321' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'http', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
})

it('ignores protocol', () => {
DlMod.setDenyList(['http://bam.nr-data.net'])

expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', protocol: 'https' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()
expect(DlMod.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '', protocol: 'http' })).toBeTruthy()
})
jest.enableAutomock()
jest.unmock('./deny-list')

let denyListModule

beforeEach(async () => {
denyListModule = await import('./deny-list')
})

afterEach(() => {
jest.resetModules()
})

test('domain-only blocks all subdomains and all paths', () => {
denyListModule.setDenyList(['foo.com'])

expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a/b' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'www.foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.b.foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.b.foo.com', pathname: '/c/d' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'oo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.com', pathname: '/' })).toBeTruthy()
})

test('subdomain blocks further subdomains, but not parent domain', () => {
denyListModule.setDenyList(['bar.foo.com'])

expect(denyListModule.shouldCollectEvent({ hostname: 'bar.foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.foo.com', pathname: '/a' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.foo.com', pathname: '/a/b' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'a.bar.foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.bar.foo.com', pathname: '/a' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.bar.foo.com', pathname: '/a/b' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a/b' })).toBeTruthy()

expect(denyListModule.shouldCollectEvent({ hostname: 'oo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.com', pathname: '/' })).toBeTruthy()
})

test('* blocks all domains', () => {
denyListModule.setDenyList(['*'])

expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'www.foo.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.com', pathname: '/' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'www.bar.com', pathname: '/' })).toBeFalsy()
})

test('respects path', () => {
denyListModule.setDenyList(['bam.nr-data.net/somepath'])

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '/somepath' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '/someotherpath' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '/some/otherpath' })).toBeTruthy()
})

test('ignores port', () => {
denyListModule.setDenyList(['bam.nr-data.net:1234'])

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', port: '4321' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'http', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '' })).toBeTruthy()
})

test('ignores protocol', () => {
denyListModule.setDenyList(['http://bam.nr-data.net'])

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', pathname: '', protocol: 'https' })).toBeFalsy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.net', port: '7890', protocol: 'https', host: 'bam.nr-data.net:7890', pathname: '/somepath' })).toBeFalsy()

expect(denyListModule.shouldCollectEvent({ hostname: 'bam.nr-data.com', pathname: '', protocol: 'http' })).toBeTruthy()
})

test.each([
null,
undefined,
'!@$%^*',
'https://example.com/http://foo.bar/'
])('ignores invalid deny list value %s', (denyListValue) => {
denyListModule.setDenyList([denyListValue])

expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'foo.com', pathname: '/a/b' })).toBeTruthy()

expect(denyListModule.shouldCollectEvent({ hostname: 'www.foo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.b.foo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'a.b.foo.com', pathname: '/c/d' })).toBeTruthy()

expect(denyListModule.shouldCollectEvent({ hostname: 'oo.com', pathname: '/' })).toBeTruthy()
expect(denyListModule.shouldCollectEvent({ hostname: 'bar.com', pathname: '/' })).toBeTruthy()
})
17 changes: 17 additions & 0 deletions src/common/ids/__mocks__/unique-id.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { faker } from '@faker-js/faker'

export function generateUuid () {
return faker.datatype.uuid()
}

export function generateRandomHexString (length) {
return faker.datatype.hexadecimal({ length, prefix: '' })
}

export function generateSpanId () {
return generateRandomHexString(16)
}

export function generateTraceId () {
return generateRandomHexString(32)
}
15 changes: 15 additions & 0 deletions src/common/url/__mocks__/parse-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export function parseUrl (url) {
try {
const result = new URL(url)

return {
hostname: result.hostname,
port: result.port || result.protocol.startsWith('https') ? '443' : '80',
pathname: result.pathname,
protocol: result.protocol.replace(':', ''),
sameOrigin: false
}
} catch (err) {
return {}
}
}
File renamed without changes.
4 changes: 0 additions & 4 deletions src/features/ajax/instrument/distributed-tracing.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@ import { stringify } from '../../../common/util/stringify'
export class DT {
constructor (agentIdentifier) {
this.agentIdentifier = agentIdentifier
// Binds this class instance context to the following fn used in an external module (exported);
// Alternatively, can make them class field arrow functions, but requires experimental features/plugin for eslint.
this.generateTracePayload = this.generateTracePayload.bind(this)
this.shouldGenerateTrace = this.shouldGenerateTrace.bind(this)
}

generateTracePayload (parsedOrigin) {
Expand Down
Loading