From 49f95250b754c7603d5414ba198fdda7cf47546e Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 31 Aug 2018 20:44:17 +0100 Subject: [PATCH] mediawiki.user: Fix missing array initialization in generateRandomSessionId Array was not properly initialized and thus browsers that do not support Crypto API where displaying an error on console. The tests failed to catch this because assigning window.crypto to `undefined` does not work (it is a read-only property). This "fallback" test was actually testing the regular Crypto-based path a second time. Bug: T203275 Co-Authored-By: Timo Tijhof Change-Id: I8feecddf0878a739e560085f7897ebc3d8100c02 --- resources/src/mediawiki.user.js | 1 + .../mediawiki/mediawiki.user.test.js | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/resources/src/mediawiki.user.js b/resources/src/mediawiki.user.js index 251b108078..9f6f845f74 100644 --- a/resources/src/mediawiki.user.js +++ b/resources/src/mediawiki.user.js @@ -55,6 +55,7 @@ rnds = new Uint16Array( 5 ); crypto.getRandomValues( rnds ); } else { + rnds = new Array( 5 ); // 0x10000 is 2^16 so the operation below will return a number // between 2^16 and zero for ( i = 0; i < 5; i++ ) { diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js index f223ef75f2..751155daff 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js @@ -2,15 +2,19 @@ QUnit.module( 'mediawiki.user', QUnit.newMwEnvironment( { setup: function () { this.server = this.sandbox.useFakeServer(); - this.crypto = window.crypto; - this.msCrypto = window.msCrypto; + // Cannot stub by simple assignment because read-only. + // Instead, stub in tests by using 'delete', and re-create + // in teardown using the original descriptor (including its + // accessors and readonly settings etc.) + this.crypto = Object.getOwnPropertyDescriptor( window, 'crypto' ); + this.msCrypto = Object.getOwnPropertyDescriptor( window, 'msCrypto' ); }, teardown: function () { if ( this.crypto ) { - window.crypto = this.crypto; + Object.defineProperty( window, 'crypto', this.crypto ); } if ( this.msCrypto ) { - window.msCrypto = this.msCrypto; + Object.defineProperty( window, 'msCrypto', this.msCrypto ); } } } ) ); @@ -81,12 +85,14 @@ 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; - } + delete window.crypto; + delete window.msCrypto; + // Assert that the above actually worked. If we use the wrong method + // of stubbing, JavaScript silently continues and we need to know that + // it was the wrong method. As of writing, assigning undefined is + // ineffective as the window property for Crypto is read-only. + // However, deleting does work. (T203275) + assert.strictEqual( window.crypto || window.msCrypto, undefined, 'fallback is active' ); result = mw.user.generateRandomSessionId(); assert.strictEqual( typeof result, 'string', 'type' ); -- 2.20.1