John Erling Blad has submitted this change and it was merged. Change subject: dv.util.inherit has new parameter for defining a constructor's name ......................................................................
dv.util.inherit has new parameter for defining a constructor's name - patch set 2: filter malicious characters from name given to inherit() Change-Id: I78bbf26d8d241574773888a3aa28bc12d34f1b46 --- M DataValues/resources/dataValues.util.js M DataValues/tests/qunit/dataValues.util.inherit.tests.js 2 files changed, 94 insertions(+), 19 deletions(-) Approvals: John Erling Blad: Verified; Looks good to me, approved Henning Snater: Verified jenkins-bot: Checked diff --git a/DataValues/resources/dataValues.util.js b/DataValues/resources/dataValues.util.js index 8788167..d64c70d 100644 --- a/DataValues/resources/dataValues.util.js +++ b/DataValues/resources/dataValues.util.js @@ -2,7 +2,7 @@ * @file * @ingroup DataValues * @licence GNU GPL v2+ - * @author Daniel Werner + * @author Daniel Werner < [email protected] > */ ( function( $, dv ) { 'use strict'; @@ -18,16 +18,25 @@ * Helper for prototypical inheritance. * @since 0.1 * - * @param {Function} base Constructor which will be used for the prototype chain. + * @param {string} name (optional) The name of the new constructor. This is handy for debugging + * purposes since instances of the constructor might be displayed under that name. + * @param {Function} base Constructor which will be used for the prototype chain. This function + * will not be the constructor returned by the function but will be called by it. * @param {Function} [constructor] for overwriting base constructor. Can be omitted. * @param {Object} [members] properties overwriting or extending those of the base. * @return Function Constructor of the new, extended type. + * + * @throws {Error} In case a malicious function name is given or a reserved word is used */ - dv.util.inherit = function( base, constructor, members ) { - // allow to omit constructor since it can be inherited directly. But if given, require it as - // second parameter for readability. If no constructor, second parameter is the prototype - // extension object. - if( members === undefined ) { + dv.util.inherit = function( name, base, constructor, members ) { + // the name is optional + if( typeof name !== 'string' ) { + members = constructor; constructor = base; base = name; name = false; + } + + // allow to omit constructor since it can be inherited directly. But if given, require it as second parameter + // for readability. If no constructor, second parameter is the prototype extension object. + if( !members ) { if( $.isFunction( constructor ) ) { members = {}; } else { @@ -35,7 +44,22 @@ constructor = false; } } - var NewConstructor = constructor || function() { base.apply( this, arguments ); }; + // if no name is given, find suitable constructor's name + name = name || constructor.name || ( base.name ? base.name + '_SubProto' : 'SomeInherited' ); + // make sure name is just a function name and not some executable JavaScript + name = name.replace( /(?:(^\d+)|[^\w$])/ig, '' ); + + if( !name ) { // only bad characters were in the name! + throw new Error( 'Bad constructor name given. Only word characters and $ are allowed.' ); + } + + // function we execute in our real constructor created by evil eval: + var evilsSeed = constructor || base, + NewConstructor; + + // for creating a named function with a variable name, there is just no other way... + eval( 'NewConstructor = function ' + name + + '(){ evilsSeed.apply( this, arguments ); }' ); var NewPrototype = function(){}; // new constructor for avoiding base constructor and with it any side-effects NewPrototype.prototype = base.prototype; diff --git a/DataValues/tests/qunit/dataValues.util.inherit.tests.js b/DataValues/tests/qunit/dataValues.util.inherit.tests.js index d09b35f..a301bdd 100644 --- a/DataValues/tests/qunit/dataValues.util.inherit.tests.js +++ b/DataValues/tests/qunit/dataValues.util.inherit.tests.js @@ -8,7 +8,7 @@ * @licence GNU GPL v2+ * @author Daniel Werner */ -( function( dv, $, QUnit, undefined ) { +( function( dv, $, QUnit ) { 'use strict'; QUnit.module( 'dataValues.util.inherit', QUnit.newMwEnvironment() ); @@ -35,7 +35,13 @@ else if( constructor === null ) { // constructor omitted, check for right param mapping: C = dv.util.inherit( base, members ); - var C2 = dv.util.inherit( base, C.prototype.constructor, members ); + var C2 = dv.util.inherit( base, C.prototype.constructor, members ), + origConstructorOfC = C.prototype.constructor; + + // constructors will never be the same since a new function will be created in inherit(), + // so we have to set them to the same to test for all the other prototype members. + C.prototype.constructor = null; + C2.prototype.constructor = null; QUnit.assert.deepEqual( C.prototype, @@ -43,6 +49,7 @@ 'inherit() works as expected if "constructor" parameter was omitted.' ); C2 = null; + C.prototype.constructor = origConstructorOfC; } else if( members === null ) { C = dv.util.inherit( base, constructor ); @@ -60,14 +67,6 @@ ( new C() ) instanceof base && ( new C() ) instanceof C, '"instanceof" is working like it should' ); - - if( constructor !== null ) { - QUnit.assert.equal( - C, - constructor, - 'prototypes constructor property is set to given constructor' - ); - } var proto = $.extend( {}, C.prototype ); if( members === null || !members.hasOwnProperty( 'constructor' ) ) { @@ -89,7 +88,7 @@ foo: 'baa' }, - inheritConstructor = function() { + inheritConstructor = function InheritTestConstructor() { this.foo = 'test'; }, @@ -120,9 +119,20 @@ var C1 = inheritTest( Object, inheritConstructor, null ); inheritConstructorTest( C1 ); + QUnit.assert.equal( + C1.name, + inheritConstructor.name, + 'Constructor returned by inherit() takes name of given constructor function' + ); + // inherit from C2: var C2 = inheritTest( C1, null, inheritMembers ); inheritConstructorTest( C2 ); + + QUnit.assert.ok( + C2.name.indexOf( inheritConstructor.name ) === 0, + 'Constructor returned by inherit() uses name of given constructor plus suffix' + ); } ); QUnit.test( 'inherit( base, constructor, members )', function( assert ) { @@ -131,4 +141,45 @@ inheritConstructorTest( C ); } ); + QUnit.test( 'inherit( name, ... ) with different names', function( assert ) { + var names = [ + [ '$' ], + [ '_' ], + [ '$foo' ], + [ '_foo' ], + [ 'foo' ], + [ 'foo42' ], + [ '42foo', 'foo' ], + [ 'function()xxx(){};', 'functionxxx' ], + [ 'xyz;${]123', 'xyz$123' ], + [ ';a;1;$;b;_;', 'a1$b_' ], + [ '();', false ], + [ '42', false ], // can't start function name with number + [ 'class', false ], // reserved word + [ 'function', false ] // reserved word + ]; + + $.each( names, function( i, definition ) { + var testName = definition[0], + expectedName = definition[1] || ( definition[1] === undefined ? testName : false ); + + if( !expectedName ) { + assert.throws( + function() { + dv.util.inherit( testName, Object ); + }, + 'inherit( \'' + testName + '\', ... ); will throw and error because of malicious constructor name.' + ); + } else { + assert.equal( + dv.util.inherit( testName, Object ).name, + expectedName, + 'inherit( \'' + testName + '\', ... ); will use "' + expectedName + '" as name.' + ); + + } + + } ); + } ); + }( dataValues, jQuery, QUnit ) ); -- To view, visit https://gerrit.wikimedia.org/r/50157 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I78bbf26d8d241574773888a3aa28bc12d34f1b46 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/DataValues Gerrit-Branch: master Gerrit-Owner: Daniel Werner <[email protected]> Gerrit-Reviewer: Henning Snater <[email protected]> Gerrit-Reviewer: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: John Erling Blad <[email protected]> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
