fix: Fix distributed tracing handling of empty string fetch parameter (…

…#640)

.github/actions/build-ab/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "@newrelic/browser-agent.actions.find-pull-request",
+  "name": "@newrelic/browser-agent.actions.build-ab",
   "description": "",
   "private": true,
   "type": "module",

package.json

@@ -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",

src/common/deny-list/deny-list.js

@@ -65,7 +65,7 @@ export function setDenyList (denyListConfig) {
       host = url
       pathname = ''
     }
-    let [hostname, port] = host.split(':')
+    let [hostname] = host.split(':')
 
     denyList.push({ hostname, pathname })
   }

src/common/deny-list/deny-list.test.js

@@ -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()
 })

src/common/ids/__mocks__/unique-id.js

@@ -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)
+}

src/common/url/__mocks__/parse-url.js

@@ -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 {}
+  }
+}

src/features/ajax/instrument/distributed-tracing.js

@@ -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) {