diff --git a/package-lock.json b/package-lock.json index 3f55635f72..604a7fb1b1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5651,7 +5651,8 @@ "long": { "version": "5.2.3", "resolved": "https://registry.npmjs.org/long/-/long-5.2.3.tgz", - "integrity": "sha512-lcHwpNoggQTObv5apGNCTdJrO69eHOZMi4BNC+rTLER8iHAqGrUVeLh/irVIM7zTw2bOXA8T6uNPeujwOLg/2Q==" + "integrity": "sha512-lcHwpNoggQTObv5apGNCTdJrO69eHOZMi4BNC+rTLER8iHAqGrUVeLh/irVIM7zTw2bOXA8T6uNPeujwOLg/2Q==", + "optional": true }, "loupe": { "version": "2.3.7", diff --git a/package.json b/package.json index 6a68c36302..a31cc77ae3 100644 --- a/package.json +++ b/package.json @@ -203,7 +203,6 @@ "farmhash-modern": "^1.1.0", "jsonwebtoken": "^9.0.0", "jwks-rsa": "^3.1.0", - "long": "^5.2.3", "node-forge": "^1.3.1", "uuid": "^10.0.0" }, diff --git a/src/remote-config/condition-evaluator-internal.ts b/src/remote-config/condition-evaluator-internal.ts index 46803f8402..2be55ec08d 100644 --- a/src/remote-config/condition-evaluator-internal.ts +++ b/src/remote-config/condition-evaluator-internal.ts @@ -26,7 +26,6 @@ import { PercentConditionOperator } from './remote-config-api'; import * as farmhash from 'farmhash-modern'; -import long = require('long'); /** * Encapsulates condition evaluation logic to simplify organization and @@ -147,26 +146,18 @@ export class ConditionEvaluator { const seedPrefix = seed && seed.length > 0 ? `${seed}.` : ''; const stringToHash = `${seedPrefix}${context.randomizationId}`; + const hash64 = ConditionEvaluator.hashSeededRandomizationId(stringToHash) - // Using a 64-bit long for consistency with the Remote Config fetch endpoint. - let hash64 = long.fromString(farmhash.fingerprint64(stringToHash).toString()); + const instanceMicroPercentile = hash64 % BigInt(100 * 1_000_000); - // Negate the hash if its value is less than 0. We handle this manually because the - // Long library doesn't provided an absolute value method. - if (hash64.lt(0)) { - hash64 = hash64.negate(); - } - - const instanceMicroPercentile = hash64.mod(100 * 1_000_000); - switch (percentOperator) { case PercentConditionOperator.LESS_OR_EQUAL: - return instanceMicroPercentile.lte(normalizedMicroPercent); + return instanceMicroPercentile <= normalizedMicroPercent; case PercentConditionOperator.GREATER_THAN: - return instanceMicroPercentile.gt(normalizedMicroPercent); + return instanceMicroPercentile > normalizedMicroPercent; case PercentConditionOperator.BETWEEN: - return instanceMicroPercentile.gt(normalizedMicroPercentLowerBound) - && instanceMicroPercentile.lte(normalizedMicroPercentUpperBound); + return instanceMicroPercentile > normalizedMicroPercentLowerBound + && instanceMicroPercentile <= normalizedMicroPercentUpperBound; case PercentConditionOperator.UNKNOWN: default: break; @@ -175,4 +166,20 @@ export class ConditionEvaluator { // TODO: add logging once we have a wrapped logger. return false; } + + // Visible for testing + 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; + } } diff --git a/test/unit/remote-config/condition-evaluator.spec.ts b/test/unit/remote-config/condition-evaluator.spec.ts index 5bb0f89133..17e524f6d8 100644 --- a/test/unit/remote-config/condition-evaluator.spec.ts +++ b/test/unit/remote-config/condition-evaluator.spec.ts @@ -858,4 +858,77 @@ 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')) + }); + }); });