jenkins-bot has submitted this change and it was merged.
Change subject: mediawiki.user: Clean up crypto version of
generateRandomSessionId
......................................................................
mediawiki.user: Clean up crypto version of generateRandomSessionId
Follows-up 4860ea3ca6.
* Documented how the byteToHex padding works.
* Move for-statement to after var and function declarations.
* Use permalink for git url (master will change, making the link
less useful).
* Remove dead ", r" comma statement.
* Substitute 0x03 to match the other 3.
* Use 8 instead of arr.length. (Matching the other loops.)
* Use jQuery from closure instead of global $.
* Use $.trim instead of str.trim (new in ES5).
* Add test to assert consecutive return values are different.
Change-Id: I9f59cf60316091e435e4bc9dbd700b9c6e431dac
---
M resources/src/mediawiki/mediawiki.user.js
M tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
2 files changed, 69 insertions(+), 40 deletions(-)
Approvals:
Nuria: Looks good to me, approved
Fomafix: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/resources/src/mediawiki/mediawiki.user.js
b/resources/src/mediawiki/mediawiki.user.js
index c3ec3f3..04d9ec6 100644
--- a/resources/src/mediawiki/mediawiki.user.js
+++ b/resources/src/mediawiki/mediawiki.user.js
@@ -11,12 +11,6 @@
options = mw.user.options || new mw.Map(),
tokens = mw.user.tokens || new mw.Map();
- // Maps for number -> hex string conversion (with padding)
- // idea from: https://github.com/broofa/node-uuid/blob/master/uuid.js
- for ( i = 0; i < 256; i++ ) {
- byteToHex[i] = (i + 0x100).toString(16).substr(1);
- }
-
/**
* Get the current user's groups or rights
*
@@ -51,55 +45,64 @@
return deferreds[info].promise();
}
+ // Map from numbers 0-255 to a hex string (with padding)
+ for ( i = 0; i < 256; i++ ) {
+ // Padding: Add a full byte (0x100, 256) and strip the extra
character
+ byteToHex[i] = ( i + 256 ).toString( 16 ).slice( 1 );
+ }
+
mw.user = user = {
options: options,
tokens: tokens,
/**
- * Generate a random user session ID
+ * Generate a random user session ID.
+ *
* This information would potentially be stored in a cookie to
identify a user during a
- * session or series of sessions. Its uniqueness should
- * not be depended on unless the browser supports the crypto
API.
+ * session or series of sessions. Its uniqueness should not be
depended on unless the
+ * browser supports the crypto API.
*
* Known problems with Math.random():
* Using the Math.random function we have seen sets
- * with 1% of non uniques among 200.000 values with Safari
providing most of these.
+ * with 1% of non uniques among 200,000 values with Safari
providing most of these.
* Given the prevalence of Safari in mobile the percentage of
duplicates in
* mobile usages of this code is probably higher.
*
* Rationale:
* We need about 64 bits to make sure that probability of
collision
* on 500 million (5*10^8) is <= 1%
- * See:
https://en.wikipedia.org/wiki/Birthday_problem#Probability_table
+ * See
https://en.wikipedia.org/wiki/Birthday_problem#Probability_table
*
* @return {string} 64 bit integer in hex format, padded
*/
generateRandomSessionId: function () {
/*jshint bitwise:false */
- var rnds, i, r, cryptoObj, hexRnds = new Array( 8 );
- cryptoObj = window.crypto || window.msCrypto; // for IE
11
+ var rnds, i, r,
+ hexRnds = new Array( 8 ),
+ // Support: IE 11
+ crypto = window.crypto || window.msCrypto;
- if ( cryptoObj ) {
- // We fill an array with 8 random values, each
of which is 8 bits.
- // note that rnds is an array-like object not a
true array
+ // Based on
https://github.com/broofa/node-uuid/blob/bfd9f96127/uuid.js
+ if ( crypto ) {
+ // Fill an array with 8 random values, each of
which is 8 bits.
+ // Note that Uint8Array is array-like but does
not implement Array.
rnds = new Uint8Array( 8 );
- cryptoObj.getRandomValues( rnds );
+ crypto.getRandomValues( rnds );
} else {
rnds = new Array( 8 );
- // From:
https://github.com/broofa/node-uuid/blob/master/uuid.js
- for ( i = 0, r; i < 8; i++ ) {
- if ( ( i & 0x03 ) === 0 ) {
+ for ( i = 0; i < 8; i++ ) {
+ if ( ( i & 3 ) === 0 ) {
r = Math.random() * 0x100000000;
}
- rnds[i] = r >>> ( ( i & 0x03 ) << 3 ) &
0xff;
+ rnds[i] = r >>> ( ( i & 3 ) << 3 ) &
255;
}
}
- // convert to hex using byteToHex that already contains
padding
- for ( i = 0; i < rnds.length; i++ ) {
+ // Convert from number to hex
+ for ( i = 0; i < 8; i++ ) {
hexRnds[i] = byteToHex[rnds[i]];
}
- // concatenation of two random integers with entrophy n
and m
+ // Concatenation of two random integers with entrophy n
and m
// returns a string with entrophy n+m if those strings
are independent
return hexRnds.join( '' );
},
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
index fe22b7a..04e002dd 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js
@@ -1,7 +1,17 @@
-( function ( mw ) {
+( function ( mw, $ ) {
QUnit.module( 'mediawiki.user', QUnit.newMwEnvironment( {
setup: function () {
this.server = this.sandbox.useFakeServer();
+ this.crypto = window.crypto;
+ this.msCrypto = window.msCrypto;
+ },
+ teardown: function () {
+ if ( this.crypto ) {
+ window.crypto = this.crypto;
+ }
+ if ( this.msCrypto ) {
+ window.msCrypto = this.msCrypto;
+ }
}
} ) );
@@ -52,21 +62,37 @@
this.server.respond();
} );
- QUnit.test( 'session numbers', 4, function ( assert ) {
- /*global $:false */
- var sessionId = mw.user.generateRandomSessionId(),
- cryptoObj = window.crypto;
+ QUnit.test( 'generateRandomSessionId', 4, function ( assert ) {
+ var result, result2;
- assert.equal( typeof sessionId, 'string',
'generateRandomSessionId should return a string' );
- assert.equal( $.trim(sessionId), sessionId,
'generateRandomSessionId should not contain whitespace' );
- // pretend crypto API is not there and do same test, make sure
code runs
- // through Math.random loop
- window.crypto = undefined;
- sessionId = mw.user.generateRandomSessionId();
- assert.equal( typeof sessionId, 'string',
'generateRandomSessionId should return a string' );
- assert.equal( sessionId.trim(), sessionId,
'generateRandomSessionId should not be empty' );
- //restoring crypto object
- window.crypto = cryptoObj;
+ result = mw.user.generateRandomSessionId();
+ assert.equal( typeof result, 'string', 'type' );
+ assert.equal( $.trim( result ), result, 'no whitespace at
beginning or end' );
+ assert.equal( result.length, 16, 'size' );
+
+ result2 = mw.user.generateRandomSessionId();
+ assert.notEqual( result, result2, 'different when called
multiple times' );
} );
-}( mediaWiki ) );
+
+ QUnit.test( 'generateRandomSessionId (fallback)', 4, function ( assert
) {
+ var result, result2;
+
+ // Pretend crypto API is not there to test the Math.random
fallback
+ if ( window.crypto ) {
+ window.crypto = undefined;
+ }
+ if ( window.msCrypto ) {
+ window.msCrypto = undefined;
+ }
+
+ result = mw.user.generateRandomSessionId();
+ assert.equal( typeof result, 'string', 'type' );
+ assert.equal( $.trim( result ), result, 'no whitespace at
beginning or end' );
+ assert.equal( result.length, 16, 'size' );
+
+ result2 = mw.user.generateRandomSessionId();
+ assert.notEqual( result, result2, 'different when called
multiple times' );
+
+ } );
+}( mediaWiki, jQuery ) );
--
To view, visit https://gerrit.wikimedia.org/r/190641
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9f59cf60316091e435e4bc9dbd700b9c6e431dac
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Fomafix
Gerrit-Reviewer: Jack Phoenix <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: Nuria <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
