Merge "mediawiki.api: Prevent misusing #saveOptions"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Mon, 18 Feb 2019 19:53:40 +0000 (19:53 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 18 Feb 2019 19:53:40 +0000 (19:53 +0000)
resources/src/mediawiki.api/options.js
tests/qunit/suites/resources/mediawiki.api/mediawiki.api.options.test.js

index f0ca272..2f69a7a 100644 (file)
@@ -3,6 +3,8 @@
  */
 ( function () {
 
+       var saveOptionsRequests = {};
+
        $.extend( mw.Api.prototype, {
 
                /**
                 * If necessary, the options will be saved using several sequential API requests. Only one promise
                 * is always returned that will be resolved when all requests complete.
                 *
+                * If a request from a previous #saveOptions call is still pending, this will wait for it to be
+                * completed, otherwise MediaWiki gets sad. No requests are sent for anonymous users, as they
+                * would fail anyway. See T214963.
+                *
                 * @param {Object} options Options as a `{ name: value, … }` object
                 * @return {jQuery.Promise}
                 */
                saveOptions: function ( options ) {
                        var name, value, bundleable,
                                grouped = [],
+                               promise;
+
+                       // Logged-out users can't have user options; we can't depend on mw.user, that'd be circular
+                       if ( mw.config.get( 'wgUserName' ) === null ) {
+                               return $.Deferred().reject( 'notloggedin' ).promise();
+                       }
+
+                       // If another options request to this API is pending, wait for it first
+                       if (
+                               saveOptionsRequests[ this.defaults.ajax.url ] &&
+                               // Avoid long chains of promises, they may cause memory leaks
+                               saveOptionsRequests[ this.defaults.ajax.url ].state() === 'pending'
+                       ) {
+                               promise = saveOptionsRequests[ this.defaults.ajax.url ].then( function () {
+                                       // Don't expose the old promise's result, it would be confusing
+                                       return $.Deferred().resolve();
+                               }, function () {
+                                       return $.Deferred().resolve();
+                               } );
+                       } else {
                                promise = $.Deferred().resolve();
+                       }
 
                        for ( name in options ) {
                                value = options[ name ] === null ? null : String( options[ name ] );
                                }.bind( this ) );
                        }
 
+                       saveOptionsRequests[ this.defaults.ajax.url ] = promise;
+
                        return promise;
                }
 
index 549deb0..5691a1b 100644 (file)
@@ -1,5 +1,8 @@
 ( function () {
        QUnit.module( 'mediawiki.api.options', QUnit.newMwEnvironment( {
+               config: {
+                       wgUserName: 'Foo'
+               },
                setup: function () {
                        this.server = this.sandbox.useFakeServer();
                        this.server.respondImmediately = true;
                        } )
                );
        } );
+
+       QUnit.test( 'saveOptions (anonymous)', function ( assert ) {
+               var promise, test = this;
+
+               mw.config.set( 'wgUserName', null );
+               promise = new mw.Api().saveOptions( { foo: 'bar' } );
+
+               assert.rejects( promise, /notloggedin/, 'Can not save options while not logged in' );
+
+               return promise
+                       .catch( function () {
+                               return $.Deferred().resolve();
+                       } )
+                       .then( function () {
+                               assert.strictEqual( test.server.requests.length, 0, 'No requests made' );
+                       } );
+       } );
 }() );