Merge pull request #369 from splitio/javascript_client_issue_847

Sanitize `SplitSDKMachineName` header value to avoid HTTP/S request exceptions.

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,7 @@
+2.0.2 (December 3, 2024)
+ - Updated the factory `init` and `destroy` methods to support re-initialization after destruction. This update ensures compatibility of the React SDK with React Strict Mode, where the factory's `init` and `destroy` effects are executed an extra time to validate proper resource cleanup.
+ - Bugfixing - Sanitize the `SplitSDKMachineName` header value to avoid exceptions on HTTP/S requests when it contains non ISO-8859-1 characters (Related to issue https://github.com/splitio/javascript-client/issues/847).
+
 2.0.1 (November 25, 2024)
  - Bugfixing - Fixed an issue with the SDK_UPDATE event on server-side, where it was not being emitted if there was an empty segment and the SDK received a feature flag update notification.
 

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "2.0.1",
+  "version": "2.0.2",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",

package.json

@@ -37,7 +37,9 @@ function segmentsEventEmitterFactory(EventEmitter: new () => SplitIO.IEventEmitt
 export function readinessManagerFactory(
   EventEmitter: new () => SplitIO.IEventEmitter,
   settings: ISettings,
-  splits: ISplitsEventEmitter = splitsEventEmitterFactory(EventEmitter)): IReadinessManager {
+  splits: ISplitsEventEmitter = splitsEventEmitterFactory(EventEmitter),
+  isShared?: boolean
+): IReadinessManager {
 
   const readyTimeout = settings.startup.readyTimeout;
 
@@ -66,23 +68,26 @@ export function readinessManagerFactory(
     gate.emit(SDK_READY_TIMED_OUT, 'Split SDK emitted SDK_READY_TIMED_OUT event.');
   }
 
-  let readyTimeoutId: ReturnType<typeof setTimeout>;
-  if (readyTimeout > 0) {
-    if (splits.hasInit) readyTimeoutId = setTimeout(timeout, readyTimeout);
-    else splits.initCallbacks.push(() => { readyTimeoutId = setTimeout(timeout, readyTimeout); });
-  }
 
   // emit SDK_READY and SDK_UPDATE
   let isReady = false;
   splits.on(SDK_SPLITS_ARRIVED, checkIsReadyOrUpdate);
   segments.on(SDK_SEGMENTS_ARRIVED, checkIsReadyOrUpdate);
 
   let isDestroyed = false;
+  let readyTimeoutId: ReturnType<typeof setTimeout>;
+  function __init() {
+    isDestroyed = false;
+    if (readyTimeout > 0 && !isReady) readyTimeoutId = setTimeout(timeout, readyTimeout);
+  }
+
+  splits.initCallbacks.push(__init);
+  if (splits.hasInit) __init();
 
   function checkIsReadyFromCache() {
     isReadyFromCache = true;
     // Don't emit SDK_READY_FROM_CACHE if SDK_READY has been emitted
-    if (!isReady) {
+    if (!isReady && !isDestroyed) {
       try {
         syncLastUpdate();
         gate.emit(SDK_READY_FROM_CACHE);
@@ -94,6 +99,7 @@ export function readinessManagerFactory(
   }
 
   function checkIsReadyOrUpdate(diff: any) {
+    if (isDestroyed) return;
     if (isReady) {
       try {
         syncLastUpdate();
@@ -117,16 +123,13 @@ export function readinessManagerFactory(
     }
   }
 
-  let refCount = 1;
-
   return {
     splits,
     segments,
     gate,
 
     shared() {
-      refCount++;
-      return readinessManagerFactory(EventEmitter, settings, splits);
+      return readinessManagerFactory(EventEmitter, settings, splits, true);
     },
 
     // @TODO review/remove next methods when non-recoverable errors are reworked
@@ -145,13 +148,9 @@ export function readinessManagerFactory(
     destroy() {
       isDestroyed = true;
       syncLastUpdate();
-
-      segments.removeAllListeners();
-      gate.removeAllListeners();
       clearTimeout(readyTimeoutId);
 
-      if (refCount > 0) refCount--;
-      if (refCount === 0) splits.removeAllListeners();
+      if (!isShared) splits.hasInit = false;
     },
 
     isReady() { return isReady; },

src/readiness/readinessManager.ts

@@ -126,6 +126,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ISDK | SplitIO.IA
     settings,
 
     destroy() {
+      hasInit = false;
       return Promise.all(Object.keys(clients).map(key => clients[key].destroy())).then(() => { });
     }
   }, extraProps && extraProps(ctx), lazyInit ? { init } : init());

src/sdkFactory/index.ts

@@ -1,5 +1,5 @@
 import { ISettings } from '../../types';
-import { decorateHeaders } from '../decorateHeaders';
+import { decorateHeaders, removeNonISO88591 } from '../decorateHeaders';
 
 const HEADERS = {
   Authorization: 'Bearer SDK-KEY',
@@ -48,3 +48,10 @@ describe('decorateHeaders', () => {
     expect(settings.log.error).toHaveBeenCalledWith('Problem adding custom headers to request decorator: Error: Unexpected error');
   });
 });
+
+test('removeNonISO88591', () => {
+  expect(removeNonISO88591('')).toBe('');
+  expect(removeNonISO88591('This is a test')).toBe('This is a test');
+  expect(removeNonISO88591('This is a test ó \u0FFF 你')).toBe('This is a test ó  ');
+  expect(removeNonISO88591('Emiliano’s-MacBook-Pro')).toBe('Emilianos-MacBook-Pro');
+});

src/services/__tests__/decorateHeaders.spec.ts

@@ -30,3 +30,8 @@ export function decorateHeaders(settings: ISettings, headers: Record<string, str
   }
   return headers;
 }
+
+export function removeNonISO88591(input: string) {
+  // eslint-disable-next-line no-control-regex
+  return input.replace(/[^\x00-\xFF]/g, '');
+}

src/services/decorateHeaders.ts

@@ -3,7 +3,7 @@ import { objectAssign } from '../utils/lang/objectAssign';
 import { ERROR_HTTP, ERROR_CLIENT_CANNOT_GET_READY } from '../logger/constants';
 import { ISettings } from '../types';
 import { IPlatform } from '../sdkFactory/types';
-import { decorateHeaders } from './decorateHeaders';
+import { decorateHeaders, removeNonISO88591 } from './decorateHeaders';
 
 const messageNoFetch = 'Global fetch API is not available.';
 
@@ -30,7 +30,7 @@ export function splitHttpClientFactory(settings: ISettings, { getOptions, getFet
   };
 
   if (ip) commonHeaders['SplitSDKMachineIP'] = ip;
-  if (hostname) commonHeaders['SplitSDKMachineName'] = hostname;
+  if (hostname) commonHeaders['SplitSDKMachineName'] = removeNonISO88591(hostname);
 
   return function httpClient(url: string, reqOpts: IRequestOptions = {}, latencyTracker: (error?: NetworkError) => void = () => { }, logErrorsAsInfo: boolean = false): Promise<IResponse> {
 

src/services/splitHttpClient.ts

@@ -7,8 +7,6 @@ import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../KeyBuilderCS';
 import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable';
 import { SplitsCacheInLocal } from './SplitsCacheInLocal';
 import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal';
-import { MySegmentsCacheInMemory } from '../inMemory/MySegmentsCacheInMemory';
-import { SplitsCacheInMemory } from '../inMemory/SplitsCacheInMemory';
 import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser';
 import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS';
 import { LOG_PREFIX } from './constants';
@@ -36,7 +34,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
       return InMemoryStorageCSFactory(params);
     }
 
-    const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode, __splitFiltersValidation } } } = params;
+    const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode } } } = params;
     const matchingKey = getMatching(settings.core.key);
     const keys = new KeyBuilderCS(prefix, matchingKey);
     const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS;
@@ -55,15 +53,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
       telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined,
       uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined,
 
-      destroy() {
-        this.splits = new SplitsCacheInMemory(__splitFiltersValidation);
-        this.segments = new MySegmentsCacheInMemory();
-        this.largeSegments = new MySegmentsCacheInMemory();
-        this.impressions.clear();
-        this.impressionCounts && this.impressionCounts.clear();
-        this.events.clear();
-        this.uniqueKeys?.clear();
-      },
+      destroy() { },
 
       // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key).
       shared(matchingKey: string) {
@@ -77,11 +67,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
           events: this.events,
           telemetry: this.telemetry,
 
-          destroy() {
-            this.splits = new SplitsCacheInMemory(__splitFiltersValidation);
-            this.segments = new MySegmentsCacheInMemory();
-            this.largeSegments = new MySegmentsCacheInMemory();
-          }
+          destroy() { }
         };
       },
     };

src/storages/inLocalStorage/index.ts

@@ -28,15 +28,7 @@ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageS
     telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined,
     uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemory() : undefined,
 
-    // When using MEMORY we should clean all the caches to leave them empty
-    destroy() {
-      this.splits.clear();
-      this.segments.clear();
-      this.impressions.clear();
-      this.impressionCounts && this.impressionCounts.clear();
-      this.events.clear();
-      this.uniqueKeys && this.uniqueKeys.clear();
-    }
+    destroy() { }
   };
 
   // @TODO revisit storage logic in localhost mode