diff --git a/services/sync/modules/util.js b/services/sync/modules/util.js index bdbcd3052952..5fc86218b652 100644 --- a/services/sync/modules/util.js +++ b/services/sync/modules/util.js @@ -215,30 +215,33 @@ let Utils = { }); }, - // Generates a brand-new globally unique identifier (GUID). + byteArrayToString: function byteArrayToString(bytes) { + return [String.fromCharCode(byte) for each (byte in bytes)].join(""); + }, + + /** + * Generate a string of random bytes. + */ + generateRandomBytes: function generateRandomBytes(length) { + let rng = Cc["@mozilla.org/security/random-generator;1"] + .createInstance(Ci.nsIRandomGenerator); + let bytes = rng.generateRandomBytes(length); + return Utils.byteArrayToString(bytes); + }, + + /** + * Encode byte string as base64url (RFC 4648). + */ + encodeBase64url: function encodeBase64url(bytes) { + return btoa(bytes).replace('+', '-', 'g').replace('/', '_', 'g'); + }, + + /** + * GUIDs are 9 random bytes encoded with base64url (RFC 4648). + * That makes them 12 characters long with 72 bits of entropy. + */ makeGUID: function makeGUID() { - // 70 characters that are not-escaped URL-friendly - const code = - "!()*-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~"; - - let guid = ""; - let num = 0; - let val; - - // Generate ten 70-value characters for a 70^10 (~61.29-bit) GUID - for (let i = 0; i < 10; i++) { - // Refresh the number source after using it a few times - if (i == 0 || i == 5) - num = Math.random(); - - // Figure out which code to use for the next GUID character - num *= 70; - val = Math.floor(num); - guid += code[val]; - num -= val; - } - - return guid; + return Utils.encodeBase64url(Utils.generateRandomBytes(9)); }, anno: function anno(id, anno, val, expire) { @@ -1210,10 +1213,7 @@ let Utils = { // Note that this is a different base32 alphabet to the one we use for // other tasks. It's lowercase, uses different letters, and needs to be // decoded with decodeKeyBase32, not just decodeBase32. - let rng = Cc["@mozilla.org/security/random-generator;1"] - .createInstance(Ci.nsIRandomGenerator); - let bytes = rng.generateRandomBytes(16); - return Utils.encodeKeyBase32(Utils.byteArrayToString(bytes)); + return Utils.encodeKeyBase32(Utils.generateRandomBytes(16)); }, /** diff --git a/services/sync/tests/unit/test_utils_makeGUID.js b/services/sync/tests/unit/test_utils_makeGUID.js index 0fb80b427183..727ca9d3e844 100644 --- a/services/sync/tests/unit/test_utils_makeGUID.js +++ b/services/sync/tests/unit/test_utils_makeGUID.js @@ -1,15 +1,29 @@ _("Make sure makeGUID makes guids of the right length/characters"); Cu.import("resource://services-sync/util.js"); +const base64url = + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_"; + function run_test() { - // XXX this could cause random failures as guids arent always unique... _("Create a bunch of guids to make sure they don't conflict"); let guids = []; for (let i = 0; i < 1000; i++) { let newGuid = Utils.makeGUID(); - _("Making sure guid has the right length without special characters:", newGuid); - do_check_eq(encodeURIComponent(newGuid).length, 10); - do_check_true(guids.every(function(g) g != newGuid)); + _("Generated " + newGuid); + + // Verify that the GUID's length is correct, even when it's URL encoded. + do_check_eq(newGuid.length, 12); + do_check_eq(encodeURIComponent(newGuid).length, 12); + + // Verify that the GUID only contains base64url characters + do_check_true(Array.every(newGuid, function(chr) { + return base64url.indexOf(chr) != -1; + })); + + // Verify uniqueness within our sample of 1000. This could cause random + // failures, but they should be extremely rare. Otherwise we'd have a + // problem with GUID collisions. + do_check_true(guids.every(function(g) { return g != newGuid; })); guids.push(newGuid); } }