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 <krinklemail@gmail.com>
Change-Id: I8feecddf0878a739e560085f7897ebc3d8100c02

diff --git a/resources/src/mediawiki.user.js b/resources/src/mediawiki.user.js
index 251b108..9f6f845 100644 (file)
--- 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 f223ef7..751155d 100644 (file)
--- 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' );