Add limit checks on custom signals keys and values

Author: ashish-kothari

packages/remote-config/src/api.ts

@@ -23,8 +23,8 @@ import {
   Value
 } from './public_types';
 import { RemoteConfigAbortSignal } from './client/remote_config_fetch_client';
-import { RC_COMPONENT_NAME } from './constants';
-import { ErrorCode, hasErrorCode } from './errors';
+import { RC_COMPONENT_NAME, RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH, RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH } from './constants';
+import { ERROR_FACTORY, ErrorCode, hasErrorCode } from './errors';
 import { RemoteConfig as RemoteConfigImpl } from './remote_config';
 import { Value as ValueImpl } from './value';
 import { LogLevel as FirebaseLogLevel } from '@firebase/logger';
@@ -276,5 +276,22 @@ export async function setCustomSignals(
   customSignals: CustomSignals
 ): Promise<void> {
   const rc = getModularInstance(remoteConfig) as RemoteConfigImpl;
+  // eslint-disable-next-line guard-for-in
+  for (const key in customSignals) {
+    if (key.length > RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH) {
+      throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_KEY_LENGTH, {
+        key,
+        maxLength: RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH
+      });
+    }
+    const value = customSignals[key];
+    if (typeof value === 'string' && value.length > RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH) {
+      throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_VALUE_LENGTH, {
+        key,
+        maxLength: RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH
+      });
+    }
+  }
+
   return rc._storageCache.setCustomSignals(customSignals);
 }

packages/remote-config/src/constants.ts

@@ -16,3 +16,6 @@
  */
 
 export const RC_COMPONENT_NAME = 'remote-config';
+export const RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS = 100;
+export const RC_CUSTOM_SIGNAL_KEY_MAX_LENGTH = 250;
+export const RC_CUSTOM_SIGNAL_VALUE_MAX_LENGTH = 500;

packages/remote-config/src/errors.ts

@@ -31,7 +31,10 @@ export const enum ErrorCode {
   FETCH_THROTTLE = 'fetch-throttle',
   FETCH_PARSE = 'fetch-client-parse',
   FETCH_STATUS = 'fetch-status',
-  INDEXED_DB_UNAVAILABLE = 'indexed-db-unavailable'
+  INDEXED_DB_UNAVAILABLE = 'indexed-db-unavailable',
+  CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS = 'custom-signal-max-allowed-signals',
+  CUSTOM_SIGNAL_KEY_LENGTH = 'custom-signal-key-length',
+  CUSTOM_SIGNAL_VALUE_LENGTH = 'custom-signal-value-length'
 }
 
 const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
@@ -67,7 +70,13 @@ const ERROR_DESCRIPTION_MAP: { readonly [key in ErrorCode]: string } = {
   [ErrorCode.FETCH_STATUS]:
     'Fetch server returned an HTTP error status. HTTP status: {$httpStatus}.',
   [ErrorCode.INDEXED_DB_UNAVAILABLE]:
-    'Indexed DB is not supported by current browser'
+    'Indexed DB is not supported by current browser',
+  [ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS]:
+    'Setting more than {$maxSignals} custom signals is not supported.',
+  [ErrorCode.CUSTOM_SIGNAL_KEY_LENGTH]:
+    'Custom signal key {$key} is too long, max allowed length is {$maxLength}.',
+  [ErrorCode.CUSTOM_SIGNAL_VALUE_LENGTH]:
+    'Value supplied for custom signal {$key} is too long, max allowed length is {$maxLength}.'
 };
 
 // Note this is effectively a type system binding a code to params. This approach overlaps with the
@@ -86,6 +95,9 @@ interface ErrorParams {
   [ErrorCode.FETCH_THROTTLE]: { throttleEndTimeMillis: number };
   [ErrorCode.FETCH_PARSE]: { originalErrorMessage: string };
   [ErrorCode.FETCH_STATUS]: { httpStatus: number };
+  [ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS]: { maxSignals: number };
+  [ErrorCode.CUSTOM_SIGNAL_KEY_LENGTH]: { key: string, maxLength: number };
+  [ErrorCode.CUSTOM_SIGNAL_VALUE_LENGTH]: { key: string, maxLength: number };
 }
 
 export const ERROR_FACTORY = new ErrorFactory<ErrorCode, ErrorParams>(

packages/remote-config/src/storage/storage.ts

@@ -21,6 +21,7 @@ import {
   FirebaseRemoteConfigObject
 } from '../client/remote_config_fetch_client';
 import { ERROR_FACTORY, ErrorCode } from '../errors';
+import { RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS } from '../constants';
 import { FirebaseError } from '@firebase/util';
 
 /**
@@ -201,6 +202,14 @@ export class Storage {
     const signalsToUpdate = Object.fromEntries(
       Object.entries(combinedSignals).filter(([_, v]) => v !== null)
     );
+
+    // Throw an error if the number of custom signals to be stored exceeds the limit
+    if (Object.keys(signalsToUpdate).length > RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS) {
+      throw ERROR_FACTORY.create(ErrorCode.CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS, {
+        maxSignals: RC_CUSTOM_SIGNAL_MAX_ALLOWED_SIGNALS
+      });
+    }
+
     return this.setWithTransaction<CustomSignals>(
       'custom_signals',
       signalsToUpdate,

packages/remote-config/test/remote_config.test.ts

@@ -107,6 +107,24 @@ describe('RemoteConfig', () => {
         key: 'value'
       });
     });
+    
+    it('throws an error when supplied with a custom signal key greater than 250 characters', async () => {
+      const longKey = 'a'.repeat(251);
+      const customSignals = { [longKey]: 'value' };
+
+      await expect(setCustomSignals(rc, customSignals)).to.eventually.be.rejectedWith(
+        `Remote Config: Custom signal key ${longKey} is too long, max allowed length is 250.`
+      );
+    });
+    
+    it('throws an error when supplied with a custom signal value greater than 500 characters', async () => {
+      const longValue = 'a'.repeat(501);
+      const customSignals = { 'key': longValue };
+
+      await expect(setCustomSignals(rc, customSignals)).to.eventually.be.rejectedWith(
+        'Remote Config: Value supplied for custom signal key is too long, max allowed length is 500.'
+      );
+    });
   });
 
   // Adapts getUserLanguage tests from packages/auth/test/utils_test.js for TypeScript.

packages/remote-config/test/storage/storage.test.ts

@@ -153,4 +153,16 @@ describe('Storage', () => {
 
     expect(storedCustomSignals).to.deep.eq(updatedSignals);
   });
+
+  it('throws an error when supplied with excess custom signals', async () => {
+    const customSignals: { [key: string]: string } = {};
+    for (let i = 0; i < 101; i++) {
+      customSignals[`key${i}`] = `value${i}`;
+    }    
+
+    await expect(storage.setCustomSignals(customSignals)).to.eventually.be.rejectedWith(
+      'Remote Config: Setting more than 100 custom signals is not supported.'
+    );
+
+  });
 });