Merge pull request #3608 from fauzora/fix/tracker-configuration

fix(hash): improve URL normalization and handling in tracking functions

src/lib/__tests__/detect.test.ts

@@ -2,6 +2,7 @@ import * as detect from '../detect';
 import { expect } from '@jest/globals';
 
 const IP = '127.0.0.1';
+const BAD_IP = '127.127.127.127';
 
 test('getIpAddress: Custom header', () => {
   process.env.CLIENT_IP_HEADER = 'x-custom-ip-header';
@@ -17,6 +18,12 @@ test('getIpAddress: Standard header', () => {
   expect(detect.getIpAddress(new Headers({ 'x-forwarded-for': IP }))).toEqual(IP);
 });
 
+test('getIpAddress: CloudFlare header is lower priority than standard header', () => {
+  expect(
+    detect.getIpAddress(new Headers({ 'cf-connecting-ip': BAD_IP, 'x-forwarded-for': IP })),
+  ).toEqual(IP);
+});
+
 test('getIpAddress: No header', () => {
   expect(detect.getIpAddress(new Headers())).toEqual(null);
 });

src/lib/constants.ts

@@ -333,17 +333,21 @@ export const BROWSERS = {
   yandexbrowser: 'Yandex',
 };
 
+// The order here is important and influences how IPs are detected by lib/detect.ts
+// Please do not change the order unless you know exactly what you're doing - read https://developers.cloudflare.com/fundamentals/reference/http-headers/
 export const IP_ADDRESS_HEADERS = [
-  'cf-connecting-ip',
+  'x-client-ip',
+  'x-forwarded-for',
+  'cf-connecting-ip', // This should be *after* x-forwarded-for, so that x-forwarded-for is respected if present
   'do-connecting-ip',
   'fastly-client-ip',
-  'forwarded',
   'true-client-ip',
-  'x-appengine-user-ip',
-  'x-client-ip',
+  'x-real-ip',
   'x-cluster-client-ip',
   'x-forwarded',
-  'x-forwarded-for',
+  'forwarded',
+  'x-appengine-user-ip',
+  'x-nf-client-connection-ip',
   'x-real-ip',
 ];
 

src/tracker/index.js

@@ -38,6 +38,18 @@
 
   /* Helper functions */
 
+  const normalize = raw => {
+    if (!raw) return raw;
+    try {
+      const u = new URL(raw, location.href);
+      if (excludeSearch) u.search = '';
+      if (excludeHash) u.hash = '';
+      return u.toString();
+    } catch (e) {
+      return raw;
+    }
+  };
+
   const getPayload = () => ({
     website,
     screen,
@@ -61,11 +73,7 @@
     if (!url) return;
 
     currentRef = currentUrl;
-    currentUrl = new URL(url, location.href);
-
-    if (excludeSearch) currentUrl.search = '';
-    if (excludeHash) currentUrl.hash = '';
-    currentUrl = currentUrl.toString();
+    currentUrl = normalize(new URL(url, location.href).toString());
 
     if (currentUrl !== currentRef) {
       setTimeout(track, delayDuration);
@@ -210,8 +218,9 @@
     };
   }
 
-  let currentUrl = href;
-  let currentRef = referrer.startsWith(origin) ? '' : referrer;
+  let currentUrl = normalize(href);
+  let currentRef = normalize(referrer.startsWith(origin) ? '' : referrer);
+
   let initialized = false;
   let disabled = false;
   let cache;