Skip to content

Commit 546284d

Browse files
committed
chore: keep security suggestions regarding regex
1 parent fb4fd85 commit 546284d

4 files changed

Lines changed: 77 additions & 28 deletions

File tree

packages/analytics-js-integrations/__tests__/integrations/GoogleAds/browser.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ describe('GoogleAds init tests', () => {
9292

9393
test('uses custom sdkBaseUrl when provided', () => {
9494
googleAds = new GoogleAds(
95-
{ conversionID: mockConversionId, sdkBaseUrl: 'https://custom-gtm.example.com' },
95+
{ conversionID: mockConversionId, sdkBaseUrl: 'https://custom-gtm.example.com/gtag/js' },
9696
{},
9797
destinationInfo,
9898
);
@@ -106,7 +106,7 @@ describe('GoogleAds init tests', () => {
106106

107107
test('normalizes trailing slash in sdkBaseUrl', () => {
108108
googleAds = new GoogleAds(
109-
{ conversionID: mockConversionId, sdkBaseUrl: 'https://custom-gtm.example.com///' },
109+
{ conversionID: mockConversionId, sdkBaseUrl: 'https://custom-gtm.example.com/gtag/js///' },
110110
{},
111111
destinationInfo,
112112
);

packages/analytics-js-integrations/__tests__/utils/utils.test.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,3 +373,66 @@ describe('getValueOrDefault Tests', () => {
373373
expect(utils.getValueOrDefault({}, false)).toBe(false);
374374
});
375375
});
376+
377+
describe('isValidURL Tests', () => {
378+
test('returns false for non-string inputs', () => {
379+
expect(utils.isValidURL(undefined)).toBe(false);
380+
expect(utils.isValidURL(null)).toBe(false);
381+
expect(utils.isValidURL(123)).toBe(false);
382+
expect(utils.isValidURL({})).toBe(false);
383+
expect(utils.isValidURL([])).toBe(false);
384+
});
385+
386+
test('returns false for empty or blank strings', () => {
387+
expect(utils.isValidURL('')).toBe(false);
388+
expect(utils.isValidURL(' ')).toBe(false);
389+
});
390+
391+
test('returns true for valid http and https URLs', () => {
392+
expect(utils.isValidURL('http://example.com')).toBe(true);
393+
expect(utils.isValidURL('https://example.com')).toBe(true);
394+
expect(utils.isValidURL('https://www.googletagmanager.com/gtag/js')).toBe(true);
395+
expect(utils.isValidURL('https://custom-gtm.example.com/gtag/js?id=AW-123')).toBe(true);
396+
expect(utils.isValidURL('http://localhost:8080')).toBe(true);
397+
expect(utils.isValidURL('https://192.168.0.1:3000/path')).toBe(true);
398+
});
399+
400+
test('returns false for URLs with unsupported or missing protocol', () => {
401+
expect(utils.isValidURL('ftp://example.com')).toBe(false);
402+
expect(utils.isValidURL('ws://example.com')).toBe(false);
403+
expect(utils.isValidURL('example.com')).toBe(false);
404+
expect(utils.isValidURL('www.example.com')).toBe(false);
405+
expect(utils.isValidURL('//example.com')).toBe(false);
406+
});
407+
408+
test('returns false for malformed URLs', () => {
409+
expect(utils.isValidURL('not-a-valid-url')).toBe(false);
410+
expect(utils.isValidURL('http://')).toBe(false);
411+
expect(utils.isValidURL('https://')).toBe(false);
412+
});
413+
414+
describe('fallback path when globalThis.URL is unavailable', () => {
415+
let originalURL;
416+
417+
beforeEach(() => {
418+
originalURL = globalThis.URL;
419+
// Force the regex fallback branch
420+
globalThis.URL = undefined;
421+
});
422+
423+
afterEach(() => {
424+
globalThis.URL = originalURL;
425+
});
426+
427+
test('returns true for valid http/https URLs via regex fallback', () => {
428+
expect(utils.isValidURL('http://example.com')).toBe(true);
429+
expect(utils.isValidURL('https://www.googletagmanager.com/gtag/js?id=AW-123')).toBe(true);
430+
});
431+
432+
test('returns false for invalid URLs via regex fallback', () => {
433+
expect(utils.isValidURL('not-a-valid-url')).toBe(false);
434+
expect(utils.isValidURL('ftp://example.com')).toBe(false);
435+
expect(utils.isValidURL('http://')).toBe(false);
436+
});
437+
});
438+
});

packages/analytics-js-integrations/src/integrations/GoogleAds/browser.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class GoogleAds {
5353
if (sanitizedSdkBaseUrl && !isValidURL(sanitizedSdkBaseUrl)) {
5454
throw new Error(`Invalid sdkBaseUrl: "${sanitizedSdkBaseUrl}"`);
5555
}
56-
this.sdkBaseUrl = sanitizedSdkBaseUrl || 'https://www.googletagmanager.com';
56+
this.sdkBaseUrl = sanitizedSdkBaseUrl || 'https://www.googletagmanager.com/gtag/js';
5757
this.name = NAME;
5858
({
5959
shouldApplyDeviceModeTransformation: this.shouldApplyDeviceModeTransformation,
@@ -63,7 +63,7 @@ class GoogleAds {
6363
}
6464

6565
init() {
66-
const sourceUrl = `${this.sdkBaseUrl}/gtag/js?id=${this.conversionId}`;
66+
const sourceUrl = `${this.sdkBaseUrl}?id=${this.conversionId}`;
6767
loadNativeSdk(sourceUrl);
6868

6969
// Additional Settings

packages/analytics-js-integrations/src/utils/utils.js

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,9 @@ function removeTrailingSlashes(inURL) {
3535
return inURL?.endsWith('/') ? inURL.replace(/\/+$/, '') : inURL;
3636
}
3737

38-
// NOTE: URL_PATTERN and isValidURL below are duplicated from
39-
// `@rudderstack/analytics-js-common` (src/constants/urls.ts and src/utilities/url.ts).
40-
// We intentionally avoid depending on analytics-js-common here so that
41-
// non-functional changes (e.g. type-only edits) in common do not force a
42-
// rebuild + release of analytics-js-integrations. Tracked under SDK-4878 —
43-
// once a frozen-API shared utilities package exists, switch this module to
44-
// consume it and delete the duplicate. Keep the two implementations in sync
45-
// until then.
46-
const URL_PATTERN = new RegExp(
47-
'^(https?:\\/\\/)' + // protocol
48-
'(' +
49-
'((([a-zA-Z\\d]([a-zA-Z\\d-]*[a-zA-Z\\d])*)\\.)+[a-zA-Z]{2,}|' + // domain name
50-
'localhost|' + // localhost
51-
'((25[0-5]|2[0-4][0-9]|[0-1]?[0-9]?[0-9]?)\\.){3}' + // OR IP (v4) address first 3 octets
52-
'(25[0-5]|2[0-4][0-9]|[0-1]?[0-9]?[0-9]?))' + // last octet of IP address
53-
')' +
54-
'(\\:\\d+)?' + // port
55-
'(\\/[-a-zA-Z\\d%_.~+]*)*' + // path
56-
'(\\?[;&a-zA-Z\\d%_.~+=-]*)?' + // query string
57-
'(\\#[-a-zA-Z\\d_]*)?$', // fragment locator
58-
);
38+
// Keep fallback validation simple and linear-time for environments without URL support.
39+
// This checks basic URL shape only; full validation should use the built-in URL parser.
40+
const URL_PATTERN = /^https?:\/\/[^\s#$./?].\S*$/i;
5941

6042
/**
6143
* Checks if provided url is valid or not
@@ -68,12 +50,16 @@ function isValidURL(url) {
6850
}
6951

7052
try {
71-
// If URL is supported by the browser, we can use it to validate the URL
72-
// Otherwise, we can at least check if the URL matches the pattern
53+
// Prefer built-in URL parsing where available (linear and standards-based).
7354
if (typeof globalThis.URL === 'function') {
7455
// eslint-disable-next-line no-new
75-
new URL(url);
56+
const parsedURL = new URL(url);
57+
return (
58+
(parsedURL.protocol === 'http:' || parsedURL.protocol === 'https:') &&
59+
parsedURL.hostname.length > 0
60+
);
7661
}
62+
// Fallback for environments without URL support.
7763
return URL_PATTERN.test(url);
7864
} catch (e) {
7965
return false;

0 commit comments

Comments
 (0)