Skip to content
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

Normalize URL in network signals #1113

Merged
merged 4 commits into from
Jul 18, 2024
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
5 changes: 5 additions & 0 deletions .changeset/chatty-flowers-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@segment/analytics-signals': patch
---

Update network signals to include a full url
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
import { SignalEmitter } from '../../emitter'
import { Response } from 'node-fetch'
import { sleep } from '@segment/analytics-core'
import { setLocation } from '../../../test-helpers/set-location'

describe(containsJSONContent, () => {
it('should return true if headers contain application/json', () => {
Expand All @@ -31,18 +32,8 @@ describe(containsJSONContent, () => {
})

describe(matchHostname, () => {
const setHostname = (hostname: string) => {
Object.defineProperty(window, 'location', {
value: {
...window.location,
hostname: hostname,
},
writable: true,
})
}

beforeEach(() => {
setHostname('example.com')
setLocation({ hostname: 'example.com' })
})
it('should only match first party domains', () => {
expect(matchHostname('https://www.example.com')).toBe(true)
Expand All @@ -54,7 +45,7 @@ describe(matchHostname, () => {
})

it('should work with subdomains', () => {
setHostname('api.example.com')
setLocation({ hostname: 'api.example.com' })
expect(matchHostname('https://api.example.com/foo')).toBe(true)
expect(matchHostname('https://foo.com/foo')).toBe(false)
expect(matchHostname('https://example.com/foo')).toBe(false)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { setLocation } from '../../../test-helpers/set-location'
import { normalizeUrl } from '../normalize-url'

describe('normalizeUrl', () => {
beforeEach(() => {
setLocation({ origin: 'https://www.currentsite.com', protocol: 'https:' })
})

it('should return the same URL if it starts with http', () => {
const url = 'https://www.example.com'
expect(normalizeUrl(url)).toBe('https://www.example.com')
})

it('should work with subdomains', () => {
const url = 'https://api.example.com'
expect(normalizeUrl(url)).toBe('https://api.example.com')
})

it('should prepend hostname to path starting with /', () => {
const url = '/foo/bar'
expect(normalizeUrl(url)).toBe('https://www.currentsite.com/foo/bar')
})

it('should prepend hostname and / to path not starting with /', () => {
const url = 'foo/bar'
expect(normalizeUrl(url)).toBe('https://www.currentsite.com/foo/bar')
})

it('should prepend hostname and / to a single word path', () => {
const url = 'foo'
expect(normalizeUrl(url)).toBe('https://www.currentsite.com/foo')
})

it('should use the current protocol of the page if none is provided', () => {
const url = 'example.com/bar'
setLocation({ protocol: 'http:' })
expect(normalizeUrl(url)).toBe('http://example.com/bar')
})

it('should work if no /', () => {
const url = 'example.com'
setLocation({ protocol: 'http:' })
expect(normalizeUrl(url)).toBe('http://example.com')
})

it('protocols should work with subdomains', () => {
const url = 'api.example.com/foo'
setLocation({ protocol: 'http:' })
expect(normalizeUrl(url)).toBe('http://api.example.com/foo')
})
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { logger } from '../../lib/logger'
import { createNetworkSignal } from '../../types'
import { SignalEmitter } from '../emitter'
import { normalizeUrl } from './normalize-url'
import { SignalGenerator } from './types'
let origFetch: typeof window.fetch

Expand Down Expand Up @@ -68,7 +69,7 @@ export class NetworkGenerator implements SignalGenerator {
emitter.emit(
createNetworkSignal({
action: 'Request',
url: sUrl,
url: normalizeUrl(sUrl),
method: rq.method || '',
data: JSON.parse(rq.body.toString()),
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Normalize URL provided by the fetch API into a valid URL string that can be used with new URL.
*/
export const normalizeUrl = (urlOrPath: string): string => {
if (urlOrPath.startsWith('http')) {
return urlOrPath
}
if (urlOrPath.startsWith('/')) {
return window.location.origin + urlOrPath
}
if (urlOrPath.includes('.')) {
return `${location.protocol}//${urlOrPath}`
}
return window.location.origin + '/' + urlOrPath
}
9 changes: 9 additions & 0 deletions packages/signals/signals/src/test-helpers/set-location.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const setLocation = (location: Partial<Location> = {}) => {
Object.defineProperty(window, 'location', {
value: {
...window.location,
...location,
},
writable: true,
})
}
Loading