Last modified: 2014-03-23 09:39:17 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T57701, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 55701 - Using unchecked input as array keys may cause TypeErrors
Using unchecked input as array keys may cause TypeErrors
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UniversalLanguageSelector (Other open bugs)
unspecified
All All
: High normal with 1 vote (vote)
: ---
Assigned To: Kartik Mistry
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-14 16:01 UTC by TMg
Modified: 2014-03-23 09:39 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description TMg 2013-10-14 16:01:10 UTC
As suggested in bug 54646 I report this as a separate issue. Unfortunately all I have is this screenshot:

https://skydrive.live.com/?cid=89518794CBF0E4D2&id=89518794CBF0E4D2!520

There is not much to see in the screenshot, basically three lines in the Firefox web console:

ULS: Unknown language doi. (load.php:236)
ULS: Unknown language fonipa. (load.php:236)
TypeError: $.ime.sources[name] is undefined (load.php:108)

It seems this error stopped execution of all following scripts including my [[dewiki:User:TMg/autoFormatter]]. It seems this happened in the moment my script tried to focus the edit window. Some ULS code is loaded and executed in this moment. From the users point of view my script stopped working.

I spend some hours debugging and checking all scripts involved (starting with mine, of course) but wasn't able to reproduce this issue. The screenshot suggests it's something in the ULS code.

While debugging I saw code like this:

$.ime.languages[<key>].<property>
$.ime.sources[<key>].<property>

This code may possibly fail. If the <key> comes from a source that is somewhere broken (e.g. from an old ULS version) there may be no array element with the given key. Accessing a <property> of an undefined array element makes the code crash. My suggestion is to add a few simple checks to some of these lines.

One of these lines is in the prepareInputMethods function which may be the line "108" from the screenshot:

https://github.com/wikimedia/jquery.ime/blob/master/src/jquery.ime.selector.js

Relevant source:

var language = $.ime.languages[languageCode];
$.each( language.inputmethods, function ( index, inputmethod ) {
  var name = $.ime.sources[inputmethod].name;
}

The problem is that the keys are read from the "languages" array but are used to access the "sources" array. If these two arrays are out of sync (for whatever reason) the code crashs.

Suggestion:

var language = $.ime.languages[languageCode];
$.each( language.inputmethods, function ( index, inputmethod ) {
  var source = $.ime.sources[inputmethod];
  if ( !source ) {
    continue;
  }
  var name = source.name;
}

Since I'm not really sure if this is the only line that may cause the described problem the same check should be added to a few more lines, if appropriate.
Comment 1 Siebrand Mazeland 2013-10-29 12:58:05 UTC
Tracked in mingle as https://mingle.corp.wikimedia.org/projects/internationalization/cards/3812
Comment 2 Santhosh Thottingal 2013-10-30 12:21:35 UTC
https://github.com/wikimedia/jquery.ime/commit/ea9ee736 must be the cause. The id of the input method was changed. That itself is harmless. But when the old source is served from cache, it can create problems. Eventhough it is very unlikely that new problems arise because of that old change, a validation as suggested above is nice.
Comment 4 Santhosh Thottingal 2013-11-26 06:41:53 UTC
https://gerrit.wikimedia.org/r/#/c/97068 ported this to MediaWiki. Marking fixed.
Comment 5 Gerrit Notification Bot 2014-03-22 15:53:44 UTC
Change 120212 had a related patch set uploaded by Thiemo Mättig (WMDE):
Make sure script execution doesn't stop by assuming unchecked input is set

https://gerrit.wikimedia.org/r/120212
Comment 6 Gerrit Notification Bot 2014-03-23 03:49:25 UTC
Change 120212 merged by jenkins-bot:
Make sure script execution doesn't stop by assuming unchecked input is set

https://gerrit.wikimedia.org/r/120212

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links