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.

Sign up for GitHub

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

Jump to bottom

updated userid module to stop caching the entire consent object #5641

Merged
merged 5 commits into from
Aug 20, 2020
Merged

updated userid module to stop caching the entire consent object #5641

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50adafd
updated userid module to stop caching the entire consent object but r…
smenzer Aug 20, 2020
1cd1301
IE doesn't support Math.imul, so providing a polyfill for it when nec…
smenzer Aug 20, 2020
9b3f002
use `===` to compare consent values; convert hashes to a string when …
smenzer Aug 20, 2020
3d09923
add test for string response
smenzer Aug 20, 2020
44b5dee
don't use polyfills!
smenzer Aug 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions modules/userId/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ function getStoredValue(storage, key = undefined) {
* @param consentData
* @returns {{apiVersion: number, gdprApplies: boolean, consentString: string}}
*/
function makeStoredConsentDataObject(consentData) {
function makeStoredConsentDataHash(consentData) {
const storedConsentData = {
consentString: '',
gdprApplies: false,
Expand All @@ -244,8 +244,7 @@ function makeStoredConsentDataObject(consentData) {
storedConsentData.gdprApplies = consentData.gdprApplies;
storedConsentData.apiVersion = consentData.apiVersion;
}

return storedConsentData;
return utils.simpleHash(JSON.stringify(storedConsentData));
}

/**
Expand All @@ -255,7 +254,7 @@ function makeStoredConsentDataObject(consentData) {
export function setStoredConsentData(consentData) {
try {
const expiresStr = (new Date(Date.now() + (CONSENT_DATA_COOKIE_STORAGE_CONFIG.expires * (60 * 60 * 24 * 1000)))).toUTCString();
coreStorage.setCookie(CONSENT_DATA_COOKIE_STORAGE_CONFIG.name, JSON.stringify(makeStoredConsentDataObject(consentData)), expiresStr, 'Lax');
coreStorage.setCookie(CONSENT_DATA_COOKIE_STORAGE_CONFIG.name, makeStoredConsentDataHash(consentData), expiresStr, 'Lax');
} catch (error) {
utils.logError(error);
}
Expand All @@ -266,13 +265,11 @@ export function setStoredConsentData(consentData) {
* @returns {string}
*/
function getStoredConsentData() {
let storedValue;
try {
storedValue = JSON.parse(coreStorage.getCookie(CONSENT_DATA_COOKIE_STORAGE_CONFIG.name));
return coreStorage.getCookie(CONSENT_DATA_COOKIE_STORAGE_CONFIG.name);
} catch (e) {
utils.logError(e);
}
return storedValue;
}

/**
Expand All @@ -287,7 +284,7 @@ function storedConsentDataMatchesConsentData(storedConsentData, consentData) {
return (
typeof storedConsentData === 'undefined' ||
storedConsentData === null ||
utils.deepEqual(storedConsentData, makeStoredConsentDataObject(consentData))
storedConsentData === makeStoredConsentDataHash(consentData)
);
}

Expand Down
38 changes: 38 additions & 0 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1215,3 +1215,41 @@ export function mergeDeep(target, ...sources) {

return mergeDeep(target, ...sources);
}

/**
* returns a hash of a string using a fast algorithm
* source: https://stackoverflow.com/a/52171480/845390
* @param str
* @param seed (optional)
* @returns {string}
*/
export function simpleHash(str, seed = 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any chancce you can call this cyrb53Hash or something describing what it is; the source you chose has many hashes.

// IE doesn't support imul
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/imul#Polyfill
if (!Math.imul) {
Math.imul = function(opA, opB) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that we were not allowing polyfills.

I could be wrong but I thought I read / heard that somewhere...

opB |= 0; // ensure that opB is an integer. opA will automatically be coerced.
// floating points give us 53 bits of precision to work with plus 1 sign bit
// automatically handled for our convienence:
// 1. 0x003fffff /*opA & 0x000fffff*/ * 0x7fffffff /*opB*/ = 0x1fffff7fc00001
// 0x1fffff7fc00001 < Number.MAX_SAFE_INTEGER /*0x1fffffffffffff*/
var result = (opA & 0x003fffff) * opB;
// 2. We can remove an integer coersion from the statement above because:
// 0x1fffff7fc00001 + 0xffc00000 = 0x1fffffff800001
// 0x1fffffff800001 < Number.MAX_SAFE_INTEGER /*0x1fffffffffffff*/
if (opA & 0xffc00000) result += (opA & 0xffc00000) * opB | 0;
return result | 0;
};
}

let h1 = 0xdeadbeef ^ seed;
let h2 = 0x41c6ce57 ^ seed;
for (let i = 0, ch; i < str.length; i++) {
ch = str.charCodeAt(i);
h1 = Math.imul(h1 ^ ch, 2654435761);
h2 = Math.imul(h2 ^ ch, 1597334677);
}
h1 = Math.imul(h1 ^ (h1 >>> 16), 2246822507) ^ Math.imul(h2 ^ (h2 >>> 13), 3266489909);
h2 = Math.imul(h2 ^ (h2 >>> 16), 2246822507) ^ Math.imul(h1 ^ (h1 >>> 13), 3266489909);
return (4294967296 * (2097151 & h2) + (h1 >>> 0)).toString();
}
35 changes: 0 additions & 35 deletions test/spec/modules/userId_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1591,13 +1591,6 @@ describe('User ID', function() {
sinon.assert.notCalled(mockGetId);
sinon.assert.calledOnce(mockDecode);
sinon.assert.calledOnce(mockExtendId);

let consent = gdprDataHandler.getConsentData();
let userIdStoredConsent = JSON.parse(coreStorage.getCookie(CONSENT_LOCAL_STORAGE_NAME));
expect(userIdStoredConsent.gdprApplies).to.equal(consent.gdprApplies);
expect(userIdStoredConsent.gdprApplies).to.equal(testConsentData.gdprApplies);
expect(userIdStoredConsent.consentString).to.equal(consent.consentString);
expect(userIdStoredConsent.consentString).to.equal(testConsentData.consentData);
});

it('calls getId if no stored consent data but refresh is needed', function () {
Expand All @@ -1611,13 +1604,6 @@ describe('User ID', function() {
sinon.assert.calledOnce(mockGetId);
sinon.assert.calledOnce(mockDecode);
sinon.assert.notCalled(mockExtendId);

let consent = gdprDataHandler.getConsentData();
let userIdStoredConsent = JSON.parse(coreStorage.getCookie(CONSENT_LOCAL_STORAGE_NAME));
expect(userIdStoredConsent.gdprApplies).to.equal(consent.gdprApplies);
expect(userIdStoredConsent.gdprApplies).to.equal(testConsentData.gdprApplies);
expect(userIdStoredConsent.consentString).to.equal(consent.consentString);
expect(userIdStoredConsent.consentString).to.equal(testConsentData.consentData);
});

it('calls getId if empty stored consent and refresh not needed', function () {
Expand All @@ -1633,13 +1619,6 @@ describe('User ID', function() {
sinon.assert.calledOnce(mockGetId);
sinon.assert.calledOnce(mockDecode);
sinon.assert.notCalled(mockExtendId);

let consent = gdprDataHandler.getConsentData();
let userIdStoredConsent = JSON.parse(coreStorage.getCookie(CONSENT_LOCAL_STORAGE_NAME));
expect(userIdStoredConsent.gdprApplies).to.equal(consent.gdprApplies);
expect(userIdStoredConsent.gdprApplies).to.equal(testConsentData.gdprApplies);
expect(userIdStoredConsent.consentString).to.equal(consent.consentString);
expect(userIdStoredConsent.consentString).to.equal(testConsentData.consentData);
});

it('calls getId if stored consent does not match current consent and refresh not needed', function () {
Expand All @@ -1659,13 +1638,6 @@ describe('User ID', function() {
sinon.assert.calledOnce(mockGetId);
sinon.assert.calledOnce(mockDecode);
sinon.assert.notCalled(mockExtendId);

let consent = gdprDataHandler.getConsentData();
let userIdStoredConsent = JSON.parse(coreStorage.getCookie(CONSENT_LOCAL_STORAGE_NAME));
expect(userIdStoredConsent.gdprApplies).to.equal(consent.gdprApplies);
expect(userIdStoredConsent.gdprApplies).to.equal(testConsentData.gdprApplies);
expect(userIdStoredConsent.consentString).to.equal(consent.consentString);
expect(userIdStoredConsent.consentString).to.equal(testConsentData.consentData);
});

it('does not call getId if stored consent matches current consent and refresh not needed', function () {
Expand All @@ -1685,13 +1657,6 @@ describe('User ID', function() {
sinon.assert.notCalled(mockGetId);
sinon.assert.calledOnce(mockDecode);
sinon.assert.calledOnce(mockExtendId);

let consent = gdprDataHandler.getConsentData();
let userIdStoredConsent = JSON.parse(coreStorage.getCookie(CONSENT_LOCAL_STORAGE_NAME));
expect(userIdStoredConsent.gdprApplies).to.equal(consent.gdprApplies);
expect(userIdStoredConsent.gdprApplies).to.equal(testConsentData.gdprApplies);
expect(userIdStoredConsent.consentString).to.equal(consent.consentString);
expect(userIdStoredConsent.consentString).to.equal(testConsentData.consentData);
});
});
});
Expand Down
20 changes: 20 additions & 0 deletions test/spec/utils_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,5 +1177,25 @@ describe('Utils', function () {
}
expect(utils.deepEqual(obj1, obj2)).to.equal(false);
});

describe('simpleHash', function() {
it('should return the same hash for the same string', function() {
const stringOne = 'string1';
expect(utils.simpleHash(stringOne)).to.equal(utils.simpleHash(stringOne));
});
it('should return a different hash for the same string with different seeds', function() {
const stringOne = 'string1';
expect(utils.simpleHash(stringOne, 1)).to.not.equal(utils.simpleHash(stringOne, 2));
});
it('should return a different hash for different strings with the same seed', function() {
const stringOne = 'string1';
const stringTwo = 'string2';
expect(utils.simpleHash(stringOne)).to.not.equal(utils.simpleHash(stringTwo));
});
it('should return a string value, not a number', function() {
const stringOne = 'string1';
expect(typeof utils.simpleHash(stringOne)).to.equal('string');
});
});
});
});