change(rc): Update Remote Config condition evaluation hashing (#2760)

* move from farmhash to sha256 * lint * add more special char test cases * another special char case * further refine test cases * clean up tests * Trigger CIs * fix newline --------- Co-authored-by: Kevin Elko <kjelko@google.com>

Author: kjelko

src/remote-config/condition-evaluator-internal.ts

@@ -27,7 +27,8 @@ import {
  CustomSignalCondition,
  CustomSignalOperator,
 } from './remote-config-api';
-import * as farmhash from 'farmhash-modern';
+import { createHash } from 'crypto';
+
 
 /**
  * Encapsulates condition evaluation logic to simplify organization and
@@ -151,9 +152,8 @@ export class ConditionEvaluator {
  const seedPrefix = seed && seed.length > 0 ? `${seed}.` : '';
  const stringToHash = `${seedPrefix}${context.randomizationId}`;
 
- const hash64 = ConditionEvaluator.hashSeededRandomizationId(stringToHash)
-
- const instanceMicroPercentile = hash64 % BigInt(100 * 1_000_000);
+ const hash = ConditionEvaluator.hashSeededRandomizationId(stringToHash)
+ const instanceMicroPercentile = hash % BigInt(100 * 1_000_000);
 
  switch (percentOperator) {
  case PercentConditionOperator.LESS_OR_EQUAL:
@@ -173,18 +173,8 @@ export class ConditionEvaluator {
  }
 
  static hashSeededRandomizationId(seededRandomizationId: string): bigint {
- // For consistency with the Remote Config fetch endpoint's percent condition behavior
- // we use Farmhash's fingerprint64 algorithm and interpret the resulting unsigned value
- // as a signed value.
- let hash64 = BigInt.asIntN(64, farmhash.fingerprint64(seededRandomizationId));
-
- // Manually negate the hash if its value is less than 0, since Math.abs doesn't
- // support BigInt.
- if (hash64 < 0) {
- hash64 = -hash64;
- }
-
- return hash64;
+ const hex = createHash('sha256').update(seededRandomizationId).digest('hex');
+ return BigInt(`0x${hex}`);
  }
 
  private evaluateCustomSignalCondition(

test/unit/remote-config/condition-evaluator.spec.ts

@@ -28,7 +28,7 @@ import {
 } from '../../../src/remote-config/remote-config-api';
 import { v4 as uuidv4 } from 'uuid';
 import { clone } from 'lodash';
-import * as farmhash from 'farmhash-modern';
+import * as crypto from 'crypto';
 
 const expect = chai.expect;
 
@@ -251,9 +251,8 @@ describe('ConditionEvaluator', () => {
 
  it('should use zero for undefined microPercent', () => {
  // Stubs ID hasher to return a number larger than zero.
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(1n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('1');
  stubs.push(stub);
 
  const condition = {
@@ -284,9 +283,8 @@ describe('ConditionEvaluator', () => {
 
  it('should use zeros for undefined microPercentRange', () => {
  // Stubs ID hasher to return a number in range.
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(1n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('1');
  stubs.push(stub);
 
  const condition = {
@@ -317,9 +315,8 @@ describe('ConditionEvaluator', () => {
 
  it('should use zero for undefined microPercentUpperBound', () => {
  // Stubs ID hasher to return a number outside range.
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(1n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('1');
  stubs.push(stub);
 
  const condition = {
@@ -353,9 +350,8 @@ describe('ConditionEvaluator', () => {
 
  it('should use zero for undefined microPercentLowerBound', () => {
  // Stubs ID hasher to return a number in range.
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(1n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('1');
  stubs.push(stub);
 
  const condition = {
@@ -388,9 +384,9 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 9 as less or equal to 10', () => {
- const stub = sinon
-  .stub(farmhash, 'fingerprint64')
-  .returns(9n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('9');
+ stubs.push(stub);
 
  stubs.push(stub);
  const condition = {
@@ -419,9 +415,9 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 10 as less or equal to 10', () => {
- const stub = sinon
-  .stub(farmhash, 'fingerprint64')
-  .returns(10n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((10).toString(16));
+ stubs.push(stub);
 
  stubs.push(stub);
  const condition = {
@@ -450,9 +446,9 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 11 as not less or equal to 10', () => {
- const stub = sinon
-  .stub(farmhash, 'fingerprint64')
-  .returns(11n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((11).toString(16));
+ stubs.push(stub);
 
  stubs.push(stub);
  const condition = {
@@ -481,9 +477,9 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should negate -11 to 11 and evaluate as not less or equal to 10', () => {
- const stub = sinon
-  .stub(farmhash, 'fingerprint64')
-  .returns(-11n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((11).toString(16));
+ stubs.push(stub);
 
  stubs.push(stub);
  const condition = {
@@ -540,9 +536,9 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 11M as greater than 10M', () => {
- const stub = sinon
-  .stub(farmhash, 'fingerprint64')
-  .returns(11n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((11).toString(16));
+ stubs.push(stub);
 
  stubs.push(stub);
  const condition = {
@@ -571,9 +567,8 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 9 as not greater than 10', () => {
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(9n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns('9');
  stubs.push(stub);
 
  const condition = {
@@ -661,9 +656,8 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 10 as between 9 and 11', () => {
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(10n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((10).toString(16));
  stubs.push(stub);
 
  const condition = {
@@ -726,9 +720,8 @@ describe('ConditionEvaluator', () => {
  });
 
  it('should evaluate 12 as not between 9 and 11', () => {
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- .returns(12n);
+ const stub = sinon.stub(crypto.Hash.prototype, 'digest');
+ stub.withArgs('hex').returns((12).toString(16));
  stubs.push(stub);
 
  const condition = {
@@ -759,6 +752,51 @@ describe('ConditionEvaluator', () => {
  expect(actual).to.be.false;
  });
 
+ describe('known percent condition values', () => {
+ // This test is useful for ensuring consistency across all places we
+ // evaluate percent conditions. It creates a set of 10 conditions targeting 50% 
+ // with randomizationIds 0-9 and a constant `seed` value.
+ const conditionEvaluator = new ConditionEvaluator();
+
+ const percentCondition = {
+ percentOperator: PercentConditionOperator.BETWEEN,
+ microPercentRange: {
+ microPercentLowerBound: 0,
+ microPercentUpperBound: 50_000_000 // 50%
+ },
+ };
+
+ const testCases = [
+ { seed: '1', randomizationId: 'one', result: false },
+ { seed: '2', randomizationId: 'two', result: false },
+ { seed: '3', randomizationId: 'three', result: true },
+ { seed: '4', randomizationId: 'four', result: false },
+ { seed: '5', randomizationId: 'five', result: true },
+ { seed: '', randomizationId: '😊', result: true },
+ { seed: '', randomizationId: '😀', result: false },
+ { seed: 'hêl£o', randomizationId: 'wørlÐ', result: false },
+ { seed: 'řemøťe', randomizationId: 'çōnfįġ', result: true },
+ { seed: 'long', randomizationId: '.'.repeat(100), result: true },
+ { seed: 'very-long', randomizationId: '.'.repeat(1000), result: false },
+ ];
+
+ testCases.map(({ randomizationId, seed, result }) => {
+
+ const idSummary = randomizationId.length > 25 
+ ? `a ${randomizationId.length} character randomizationID` 
+ : `"${randomizationId}"`;
+
+ it(`should evaluate ${idSummary} with seed "${seed}" to ${result}`, () => {
+ const context = { randomizationId };
+ const evalResult = conditionEvaluator.evaluateConditions([{
+ name: 'is_enabled',
+ condition: { percent: { ...percentCondition, seed } }
+ }], context);
+ expect(evalResult.get('is_enabled')).to.equal(result);
+ });
+ });
+ });
+
  // The following tests are probabilistic. They use tolerances based on
  // standard deviations to balance accuracy and flakiness. Random IDs will
  // hash to the target range + 3 standard deviations 99.7% of the time,
@@ -958,15 +996,15 @@ describe('ConditionEvaluator', () => {
  describe('STRING_CONTAINS', () => {
  const testCases: CustomSignalTestCase[] = [
  { targets: ['foo', 'biz'], actual: 'foobar', outcome: true },
- { targets: ['foo', 'biz'],actual: 'bar',outcome: false },
+ { targets: ['foo', 'biz'], actual: 'bar', outcome: false },
  ];
 
  testCases.forEach(runCustomSignalTestCase(CustomSignalOperator.STRING_CONTAINS));
  });
 
  describe('STRING_DOES_NOT_CONTAIN', () => {
  const testCases: CustomSignalTestCase[] = [
- { targets: ['foo', 'biz'],actual: 'bar',outcome: true },
+ { targets: ['foo', 'biz'], actual: 'bar', outcome: true },
  { targets: ['foo', 'biz'], actual: 'foobar', outcome: false },
  ];
 
@@ -1135,77 +1173,4 @@ describe('ConditionEvaluator', () => {
  });
  });
  });
-
- describe('hashSeededRandomizationId', () => {
- // The Farmhash algorithm produces a 64 bit unsigned integer,
- // which we convert to a signed integer for legacy compatibility.
- // This has caused confusion in the past, so we explicitly
- // test here.
- it('should leave numbers <= 2^63-1 (max signed long) as is', function () {
- if (nodeVersion.startsWith('14')) {
- this.skip();
- }
-
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- // 2^63-1 = 9223372036854775807.
- .returns(BigInt('9223372036854775807'));
- stubs.push(stub);
-
- const actual = ConditionEvaluator.hashSeededRandomizationId('anything');
-
- expect(actual).to.equal(BigInt('9223372036854775807'))
- });
-
- it('should convert 2^63 to negative (min signed long) and then find the absolute value', function () {
- if (nodeVersion.startsWith('14')) {
- this.skip();
- }
-
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- // 2^63 = 9223372036854775808.
- .returns(BigInt('9223372036854775808'));
- stubs.push(stub);
-
- const actual = ConditionEvaluator.hashSeededRandomizationId('anything');
-
- // 2^63 is the negation of 2^63-1
- expect(actual).to.equal(BigInt('9223372036854775808'))
- });
-
- it('should convert 2^63+1 to negative and then find the absolute value', function () {
- if (nodeVersion.startsWith('14')) {
- this.skip();
- }
-
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- // 2^63+1 9223372036854775809.
- .returns(BigInt('9223372036854775809'));
- stubs.push(stub);
-
- const actual = ConditionEvaluator.hashSeededRandomizationId('anything');
-
- // 2^63+1 is larger than 2^63, so the absolute value is smaller
- expect(actual).to.equal(BigInt('9223372036854775807'))
- });
-
- it('should handle the value that initially caused confusion', function () {
- if (nodeVersion.startsWith('14')) {
- this.skip();
- }
-
- const stub = sinon
- .stub(farmhash, 'fingerprint64')
- // We were initially confused about the nature of this value ...
- .returns(BigInt('16081085603393958147'));
- stubs.push(stub);
-
- const actual = ConditionEvaluator.hashSeededRandomizationId('anything');
-
- // ... Now we know it's the unsigned equivalent of this absolute value.
- expect(actual).to.equal(BigInt('2365658470315593469'))
- });
- });
 });