diff --git a/modules/fabrickIdSystem.js b/modules/fabrickIdSystem.js index e61b377eefa..bb838788f07 100644 --- a/modules/fabrickIdSystem.js +++ b/modules/fabrickIdSystem.js @@ -46,7 +46,7 @@ export const fabrickIdSubmodule = { if (window.fabrickMod1) { window.fabrickMod1(configParams, consentData, cacheIdObj); } - if (!configParams || typeof configParams.apiKey !== 'string') { + if (!configParams || !configParams.apiKey || typeof configParams.apiKey !== 'string') { utils.logError('fabrick submodule requires an apiKey.'); return; } @@ -55,28 +55,34 @@ export const fabrickIdSubmodule = { let keysArr = Object.keys(configParams); for (let i in keysArr) { let k = keysArr[i]; - if (k === 'url' || k === 'refererInfo') { + if (k === 'url' || k === 'refererInfo' || (k.length > 3 && k.substring(0, 3) === 'max')) { continue; } let v = configParams[k]; if (Array.isArray(v)) { for (let j in v) { - url += `${k}=${v[j]}&`; + if (typeof v[j] === 'string' || typeof v[j] === 'number') { + url += `${k}=${v[j]}&`; + } } - } else { + } else if (typeof v === 'string' || typeof v === 'number') { url += `${k}=${v}&`; } } // pull off the trailing & url = url.slice(0, -1) const referer = _getRefererInfo(configParams); - const urls = new Set(); - url = truncateAndAppend(urls, url, 'r', referer.referer); + const refs = new Map(); + _setReferrer(refs, referer.referer); if (referer.stack && referer.stack[0]) { - url = truncateAndAppend(urls, url, 'r', referer.stack[0]); + _setReferrer(refs, referer.stack[0]); } - url = truncateAndAppend(urls, url, 'r', referer.canonicalUrl); - url = truncateAndAppend(urls, url, 'r', window.location.href); + _setReferrer(refs, referer.canonicalUrl); + _setReferrer(refs, window.location.href); + + refs.forEach(v => { + url = appendUrl(url, 'r', v, configParams); + }); const resp = function (callback) { const callbacks = { @@ -130,18 +136,48 @@ function _getBaseUrl(configParams) { } } -function truncateAndAppend(urls, url, paramName, s) { - if (s && url.length < 2000) { - if (s.length > 200) { - s = s.substring(0, 200); +function _setReferrer(refs, s) { + if (s) { + // store the longest one for the same URI + const url = s.split('?')[0]; + // OR store the longest one for the same domain + // const url = s.split('?')[0].replace('http://','').replace('https://', '').split('/')[0]; + if (refs.has(url)) { + const prevRef = refs.get(url); + if (s.length > prevRef.length) { + refs.set(url, s); + } + } else { + refs.set(url, s); + } + } +} + +export function appendUrl(url, paramName, s, configParams) { + const maxUrlLen = (configParams && configParams.maxUrlLen) || 2000; + const maxRefLen = (configParams && configParams.maxRefLen) || 1000; + const maxSpaceAvailable = (configParams && configParams.maxSpaceAvailable) || 50; + // make sure we have enough space left to make it worthwhile + if (s && url.length < (maxUrlLen - maxSpaceAvailable)) { + let thisMaxRefLen = maxUrlLen - url.length; + if (thisMaxRefLen > maxRefLen) { + thisMaxRefLen = maxRefLen; } - // Don't send the same url in multiple params - if (!urls.has(s)) { - urls.add(s); - return `${url}&${paramName}=${s}` + + s = `&${paramName}=${encodeURIComponent(s)}`; + + if (s.length >= thisMaxRefLen) { + s = s.substring(0, thisMaxRefLen); + if (s.charAt(s.length - 1) === '%') { + s = s.substring(0, thisMaxRefLen - 1); + } else if (s.charAt(s.length - 2) === '%') { + s = s.substring(0, thisMaxRefLen - 2); + } } + return `${url}${s}` + } else { + return url; } - return url; } submodule('userId', fabrickIdSubmodule); diff --git a/test/spec/modules/fabrickIdSystem_spec.js b/test/spec/modules/fabrickIdSystem_spec.js index cbd538816ab..c250c8e5e8b 100644 --- a/test/spec/modules/fabrickIdSystem_spec.js +++ b/test/spec/modules/fabrickIdSystem_spec.js @@ -1,7 +1,7 @@ import * as utils from '../../../src/utils.js'; import {server} from '../../mocks/xhr.js'; -import * as fabrickIdSystem from 'modules/fabrickIdSystem.js'; +import {fabrickIdSubmodule, appendUrl} from 'modules/fabrickIdSystem.js'; const defaultConfigParams = { apiKey: '123', @@ -10,26 +10,25 @@ const defaultConfigParams = { url: 'http://localhost:9999/test/mocks/fabrickId.json?' }; const responseHeader = {'Content-Type': 'application/json'} -const fabrickIdSubmodule = fabrickIdSystem.fabrickIdSubmodule; describe('Fabrick ID System', function() { let logErrorStub; beforeEach(function () { - logErrorStub = sinon.stub(utils, 'logError'); }); afterEach(function () { - logErrorStub.restore(); - fabrickIdSubmodule.getRefererInfoOverride = null; }); it('should log an error if no configParams were passed into getId', function () { + logErrorStub = sinon.stub(utils, 'logError'); fabrickIdSubmodule.getId(); expect(logErrorStub.calledOnce).to.be.true; + logErrorStub.restore(); }); it('should error on json parsing', function() { + logErrorStub = sinon.stub(utils, 'logError'); let submoduleCallback = fabrickIdSubmodule.getId({ name: 'fabrickId', params: defaultConfigParams @@ -44,11 +43,12 @@ describe('Fabrick ID System', function() { ); expect(callBackSpy.calledOnce).to.be.true; expect(logErrorStub.calledOnce).to.be.true; + logErrorStub.restore(); }); it('should truncate the params', function() { let r = ''; - for (let i = 0; i < 300; i++) { + for (let i = 0; i < 1500; i++) { r += 'r'; } let configParams = Object.assign({}, defaultConfigParams, { @@ -66,7 +66,7 @@ describe('Fabrick ID System', function() { submoduleCallback(callBackSpy); let request = server.requests[0]; r = ''; - for (let i = 0; i < 200; i++) { + for (let i = 0; i < 1000 - 3; i++) { r += 'r'; } expect(request.url).to.match(new RegExp(`r=${r}&r=`)); @@ -76,7 +76,6 @@ describe('Fabrick ID System', function() { JSON.stringify({}) ); expect(callBackSpy.calledOnce).to.be.true; - expect(logErrorStub.calledOnce).to.be.false; }); it('should complete successfully', function() { @@ -101,6 +100,35 @@ describe('Fabrick ID System', function() { JSON.stringify({}) ); expect(callBackSpy.calledOnce).to.be.true; - expect(logErrorStub.calledOnce).to.be.false; + }); + + it('should truncate 2', function() { + let configParams = { + maxUrlLen: 10, + maxRefLen: 5, + maxSpaceAvailable: 2 + }; + + let url = appendUrl('', 'r', '123', configParams); + expect(url).to.equal('&r=12'); + + url = appendUrl('12345', 'r', '678', configParams); + expect(url).to.equal('12345&r=67'); + + url = appendUrl('12345678', 'r', '9', configParams); + expect(url).to.equal('12345678'); + + configParams.maxRefLen = 8; + url = appendUrl('', 'r', '1234&', configParams); + expect(url).to.equal('&r=1234'); + + url = appendUrl('', 'r', '123&', configParams); + expect(url).to.equal('&r=123'); + + url = appendUrl('', 'r', '12&', configParams); + expect(url).to.equal('&r=12%26'); + + url = appendUrl('', 'r', '1&&', configParams); + expect(url).to.equal('&r=1%26'); }); });