Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hashing algorithm changed without mentioning and caused Multiple Exposures #75

Closed
levochkaa opened this issue Sep 9, 2024 · 6 comments

Comments

@levochkaa
Copy link
Contributor

levochkaa commented Sep 9, 2024

We have updated GrowthBook SDK from 1.0.48 to 1.0.61, because of this fix #74
But got another problem:
qWstmCxY

After some investigation, we've found, that:

I've made a quick test to check if the hashing really changed:

import Foundation

enum Utils {
    static func oldHash(seed: String, value: String, version: Float) -> Float? {
        switch version {
        case 2:
            // New unbiased hashing algorithm
            let combinedValue = seed + value
            let hashedValue = digest(combinedValue + "")
            return Float(hashedValue % 10000) / 10000
            
        case 1:
            let combinedValue = value + seed
            let hashedValue = digest(combinedValue + "")
            return Float(hashedValue % 1000) / 1000
            
        default:
            // Unknown hash version
            return nil
        }
    }
    
    static func newHash(seed: String, value: String, version: Float) -> Float? {
        switch version {
        case 2:
            // New unbiased hashing algorithm
            let combinedValue = seed + value
            let hashedCombinedValue = digest(combinedValue).description + ""
            let hashedValue = digest(hashedCombinedValue) % 10000
            return Float(hashedValue) / 10000
            
        case 1:
            let combinedValue = value + seed
            let hashedValue = digest(combinedValue)
            return Float(hashedValue % 1000) / 1000
            
        default:
            // Unknown hash version
            return nil
        }
    }
}

enum Common {
    static let offsetBasis32: UInt32 = 2_166_136_261
    static let prime32: UInt32 = 16_777_619

    static func fnv1a<T: FixedWidthInteger>(_ array: [UInt8], offsetBasis: T, prime: T) -> T {
        var hash: T = offsetBasis
        
        for elm in array {
            hash = hash ^ T(elm)
            hash = hash &* prime
        }
        
        return hash
    }
}

func digest(_ string: String) -> UInt32 {
    Common.fnv1a(Array(string.utf8), offsetBasis: Common.offsetBasis32, prime: Common.prime32)
}

let seed = "seed"
let value = "value"

print(
    "version 1 -",
    "old: \(Utils.oldHash(seed: seed, value: value, version: 1) ?? 0)",
    "new: \(Utils.newHash(seed: seed, value: value, version: 1) ?? 0)"
)

print(
    "version 2 -",
    "old: \(Utils.oldHash(seed: seed, value: value, version: 2) ?? 0)",
    "new: \(Utils.newHash(seed: seed, value: value, version: 2) ?? 0)"
)

The output is:

version 1 - old: 0.579 new: 0.579
version 2 - old: 0.1733 new: 0.9213

Yes, hashing changed.

@vazarkevych
Copy link
Collaborator

Hi, @levochkaa. Thank you for providing the detailed information. We will review it and get back to you.

@vazarkevych
Copy link
Collaborator

We just checked the hash in different versions of the latest Growthbook SDKs and received the same result as in Swift SDK. So it seems to be working as expected. Did you try another latest SDK, and did you get a different result?

@levochkaa
Copy link
Contributor Author

Did you try another latest SDK

The hashing didn't change after 1.0.49 release, as I see in git commits history, so I am comparing 1.0.48 (oldHash) vs 1.0.49 (newHash)

We just checked the hash in different versions of the latest Growthbook SDKs and received the same result as in Swift SDK.

Can you share what versions you were comparing, how, and what are the results?

@vazarkevych
Copy link
Collaborator

vazarkevych commented Sep 12, 2024

Hi, @levochkaa. Sure, here is example of running that using the Java SDK.
JavaSDK

@levochkaa
Copy link
Contributor Author

levochkaa commented Sep 13, 2024

We are going to test some features through JS SDK, so I wanted to make sure, that the changed hash is correct now.
https://github.com/growthbook/growthbook/blob/main/packages/sdk-js/src/util.ts

function hashFnv32a(str: string): number {
  let hval = 0x811c9dc5;
  const l = str.length;

  for (let i = 0; i < l; i++) {
    hval ^= str.charCodeAt(i);
    hval +=
      (hval << 1) + (hval << 4) + (hval << 7) + (hval << 8) + (hval << 24);
  }
  return hval >>> 0;
}

export function hash(
  seed: string,
  value: string,
  version: number
): number | null {
  // New unbiased hashing algorithm
  if (version === 2) {
    return (hashFnv32a(hashFnv32a(seed + value) + "") % 10000) / 10000;
  }
  // Original biased hashing algorithm (keep for backwards compatibility)
  if (version === 1) {
    return (hashFnv32a(value + seed) % 1000) / 1000;
  }

  // Unknown hash version
  return null;
}

const seed = "seed"
const value = "value"

console.log(`version 1 - ${hash(seed, value, 1)}`)
console.log(`version 2 - ${hash(seed, value, 2)}`)

The output is:

version 1 - 0.579
version 2 - 0.9213

Which matches the new hash in 1.0.49 in iOS SDK, so, thanks for the fix, but this really should be mentioned somewhere (in readme, release notes).
And now it is mentioned on Issues tab, for folks that going to google for this error.

@levochkaa
Copy link
Contributor Author

levochkaa commented Sep 13, 2024

Hi, @vazarkevych, didn't see your reply about Java SDK.

I think the issue is closed, consider mentioning the change in release notes, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants