diff --git a/ChangeLog b/ChangeLog index 4331f0a9f815994ab9385e314eb66ebf65519e81..133825760fa7cacbcac8ff7773762b0e56308ac8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ train-2012.05.25 (in progress): - * + * many KPI improvements: #1597, #1613 + * code cleanup: #1599, #1602 + * verification links sent before deployment, should still work after - transitional code required by issue #1000: #1592 + * repair load_gen: #1596 + * fixes to mysql reconnection logic - processes can now reconnect while out of pool and only having /__heartbeat__ polled: #1608 train-2012.05.14: * Password is now requested in dialog for new user signup: #1000, #290 diff --git a/lib/db/mysql_wrapper.js b/lib/db/mysql_wrapper.js index 45edfd7de9411ad1fbb41618544d635cb62ee1f1..931ff97fbeb9c17df5ff700bc5c596cc36a9f72d 100644 --- a/lib/db/mysql_wrapper.js +++ b/lib/db/mysql_wrapper.js @@ -39,14 +39,15 @@ exports.createClient = function(options) { logger.warn("database connection down: " + e.toString()); }); }, - ping: function(cb) { - if (stalled) { - process.nextTick(function() { - cb("database is intentionally stalled"); - }); - } else { - this.realClient.ping(cb); - } + ping: function(client_cb) { + // ping queries are added to the front of the pending work queue. they are + // a priority, as they are used by load balancers that want to know the health + // of the system. + queryQueue.unshift({ + ping: true, + cb: client_cb + }); + this._runNextQuery(); }, _runNextQuery: function() { var self = this; @@ -93,25 +94,41 @@ exports.createClient = function(options) { } }, config.get('database.max_query_time_ms')); - this.realClient.query(work.query, work.args, function(err, r) { - // if we want to simulate a "stalled" mysql connection, we simply - // ignore the results from a query. - if (stalled) return; + if (work.ping) { + this.realClient.ping(function(err) { + if (stalled) { + return invokeCallback(work.cb, "database is intentionally stalled"); + } - clearTimeout(slowQueryTimer); - slowQueryTimer = null; - consecutiveFailures = 0; + clearTimeout(slowQueryTimer); + slowQueryTimer = null; + consecutiveFailures = 0; - // report query time for all queries via statsd - var reqTime = new Date - work.startTime; - statsd.timing('query_time', reqTime); + invokeCallback(work.cb, err); - // report failed queries via statsd - if (err) statsd.increment('failed_query'); + self._runNextQuery(); + }); + } else { + this.realClient.query(work.query, work.args, function(err, r) { + // if we want to simulate a "stalled" mysql connection, we simply + // ignore the results from a query. + if (stalled) return; - invokeCallback(work.cb, err, r); - self._runNextQuery(); - }); + clearTimeout(slowQueryTimer); + slowQueryTimer = null; + consecutiveFailures = 0; + + // report query time for all queries via statsd + var reqTime = new Date - work.startTime; + statsd.timing('query_time', reqTime); + + // report failed queries via statsd + if (err) statsd.increment('failed_query'); + + invokeCallback(work.cb, err, r); + self._runNextQuery(); + }); + } }, query: function() { var client_cb; diff --git a/lib/wsapi/interaction_data.js b/lib/wsapi/interaction_data.js index 629dc55cc28c0de46746aff24db2c3da3f67173d..d3f7573fd5f63a136f7d8a203d82c9e78b296f52 100644 --- a/lib/wsapi/interaction_data.js +++ b/lib/wsapi/interaction_data.js @@ -9,7 +9,8 @@ const coarse = require('../coarse_user_agent_parser'), querystring = require('querystring'), und = require('underscore'), urlparse = require('urlparse'), - wsapi = require('../wsapi.js'); + wsapi = require('../wsapi.js'), + TEN_MIN_IN_MS = 10 * 60 * 1000; // Accept JSON formatted interaction data and send it to the KPI Backend @@ -28,12 +29,16 @@ var store = function (kpi_json, cb) { kpi_resp = function (res) { logger.debug('KPI Backend responded ' + res.statusCode); }; - // TODO - timestamp should be client or server side? - und.each(kpi_json, function (kpi) { + + // Out of concern for the user's privacy, round the server timestamp + // off to the nearest 10-minute mark. + und.each(kpi_json, function (kpi) { delete kpi.local_timestamp; if (! kpi.timestamp) { kpi.timestamp = new Date().getTime(); } + kpi.timestamp = kpi.timestamp - (kpi.timestamp % TEN_MIN_IN_MS); }); + if (!! config.get('kpi_backend_db_url')) { var post_data = querystring.stringify({ diff --git a/resources/static/shared/helpers.js b/resources/static/shared/helpers.js index 7f536f6b61a5ad65e156c14c86f14b239a060648..67278ec086a7de97c5651c83527554f1e5c56a94 100644 --- a/resources/static/shared/helpers.js +++ b/resources/static/shared/helpers.js @@ -53,6 +53,16 @@ return url; } + function whitelistFilter(obj, validKeys) { + var filtered = {}; + _.each(_.keys(obj), function(key) { + if (_.indexOf(validKeys, key) !== -1) { + filtered[key] = obj[key]; + } + }); + return filtered; + } + function cancelEvent(callback) { return function(event) { event && event.preventDefault(); @@ -112,6 +122,13 @@ */ toURL: toURL, + /** + * Filter an object by a whitelist of keys, returning a new object. + * @param {object} obj - the object to filter + * @param {object} [validKeys] - whitelisted keys + */ + whitelistFilter: whitelistFilter, + /** * Return a function that calls preventDefault on the event and then calls * the callback with the arguments. diff --git a/resources/static/shared/models/interaction_data.js b/resources/static/shared/models/interaction_data.js index d5ad94544acbc97e3712c26c0025caa6eed3cd3f..ed948c2031c398db24092558ee70ae6aa6ba999c 100644 --- a/resources/static/shared/models/interaction_data.js +++ b/resources/static/shared/models/interaction_data.js @@ -9,7 +9,8 @@ BrowserID.Models.InteractionData = (function() { var bid = BrowserID, storage = bid.getStorage(), network = bid.Network, - complete = bid.Helpers.complete; + complete = bid.Helpers.complete, + whitelistFilter = bid.Helpers.whitelistFilter; function getInteractionData() { var interactionData; @@ -89,7 +90,22 @@ BrowserID.Models.InteractionData = (function() { // XXX: should we even try to post data if it's larger than some reasonable // threshold? if (data && data.length !== 0) { - network.sendInteractionData(data, function() { + + // Scrub the data we are going to send and let only a set of whitelisted + // keys through. This will remove such values as local_timestamp, which + // we needed to calculate time offsets in our event stream, but which + // could be used to fingerprint users. + var filtered = []; + _.each(data, function(obj) { + filtered.push(whitelistFilter(obj, [ + 'event_stream', + 'lang', + 'screen_size', + 'sample_rate'] + )); + }); + + network.sendInteractionData(filtered, function() { clearStaged(); complete(oncomplete, true); }, function(status) { diff --git a/resources/static/test/cases/shared/helpers.js b/resources/static/test/cases/shared/helpers.js index c29b406421024f1d23dfbc8ba8b0ab994b4c9a37..04a80eb168d7e5426715a039b307255cb76622d8 100644 --- a/resources/static/test/cases/shared/helpers.js +++ b/resources/static/test/cases/shared/helpers.js @@ -104,6 +104,21 @@ equal(url, "https://browserid.org?email=testuser%40testuser.com&status=complete", "correct URL with GET parameters"); }); + test("whitelistFilter an object", function() { + var unfiltered = { + 'event_stream': [ ['pie', 6], ['coffee', 19], ['flan', 42] ], + 'secret': "ATTACK AT DAWN!", + 'location': "Zeta Minor", + 'lang': 'auld' }; + + var filtered = helpers.whitelistFilter(unfiltered, ['event_stream', 'lang']); + equal(typeof filtered.secret, 'undefined', 'non-whitelisted key removed'); + equal(typeof filtered.location, 'undefined', 'non-whitelisted key removed'); + equal(filtered.lang, 'auld', 'whitelisted string passed'); + equal(filtered.event_stream.length, 3, 'whitelisted list passed'); + equal(filtered.event_stream[2][1], 42, 'whitelisted list elements preserved'); + }); + test("simulate log on browser without console - no exception thrown", function() { var err, nativeConsole = window.console; diff --git a/resources/static/test/cases/shared/models/interaction_data.js b/resources/static/test/cases/shared/models/interaction_data.js index 5454ccfc6e88f1e0a8cd1e250e132521d50192ff..b66ce318adfac786cd8ac04aece3d44d1f69e995 100644 --- a/resources/static/test/cases/shared/models/interaction_data.js +++ b/resources/static/test/cases/shared/models/interaction_data.js @@ -23,36 +23,36 @@ }); test("after push, most recently pushed data available through getCurrent, getStaged gets previous data sets", function() { - model.push({ foo: "bar" }); - equal(model.getCurrent().foo, "bar", + model.push({ lang: "foo" }); + equal(model.getCurrent().lang, "foo", "after pushing new interaction data, it's returned from .getCurrent()"); equal(model.getStaged().length, 0, "no data is yet staged"); - model.push({ foo: "baz" }); + model.push({ lang: "bar" }); - equal(model.getCurrent().foo, "baz", "current points to new data set") + equal(model.getCurrent().lang, "bar", "current points to new data set") var staged = model.getStaged(); equal(staged.length, 1, "only one staged item"); - testObjectValuesEqual(staged[0], { foo: "bar" }); + testObjectValuesEqual(staged[0], { lang: "foo" }); }); test("setCurrent data overwrites current", function() { model.clearStaged(); - model.push({ foo: "bar" }); - model.setCurrent({ foo: "baz" }); - equal(model.getCurrent().foo, "baz", + model.push({ lang: "foo" }); + model.setCurrent({ lang: "bar" }); + equal(model.getCurrent().lang, "bar", "overwriting current interaction data works"); }); test("clearStaged clears staged interaction data but leaves current data unaffected", function() { - model.push({ foo: "bar" }); - model.push({ foo: "baz" }); + model.push({ lang: "foo" }); + model.push({ lang: "bar" }); model.clearStaged(); equal(model.getStaged().length, 0, "after clearStageding, interaction data is zero length"); - equal(model.getCurrent().foo, "baz", + equal(model.getCurrent().lang, "bar", "after clearStageding, current data is unaffected"); }); @@ -61,7 +61,7 @@ model.stageCurrent(); equal(model.getStaged().length, 0, "no data to staged"); - model.push({ foo: "bar" }); + model.push({ lang: "foo" }); model.stageCurrent(); equal(model.getStaged().length, 1, "current data staged"); @@ -77,7 +77,7 @@ // desired result - data is purged from staging table // The first pushed data will become staged. - model.push({ foo: "bar" }); + model.push({ lang: "foo" }); model.stageCurrent(); xhr.useResult("throttle"); @@ -87,7 +87,7 @@ // When the interaction_data next completes, this will be the only data // that is pushed. - model.push({ foo: "baz" }); + model.push({ lang: "bar", secret: "Attack at dawn!!!" }); model.stageCurrent(); xhr.useResult("valid"); @@ -97,7 +97,8 @@ previousSessionsData = JSON.parse(request.data).data; equal(previousSessionsData.length, 1, "sending correct result sets"); - equal(previousSessionsData[0].foo, "baz", "correct data sent"); + equal(previousSessionsData[0].lang, "bar", "correct data sent"); + equal(typeof previousSessionsData[0].secret, "undefined", "non-whitelisted valued stripped"); start(); }); });