From 5c557e630ab6363cf59ede57976cc025ed0a010d Mon Sep 17 00:00:00 2001 From: Matthias Mullie Date: Fri, 25 May 2012 17:42:47 -0700 Subject: [PATCH] mediawiki.jqueryMsg: Fix double-escaped attributes in the parser E.g.: message: "Your feedback has been [$1 posted here]." In mediawiki.jqueryMsg.js, L31, the parser would (rightfully) escape XML entities in the parameter (a link) being assigned through $1 When actually adding the (already escaped) parameter as href- attribute of the link-to-be-built, it'll automatically have its HTML entities escaped once again (thus double-escaping &'s) This patch will identify regular links and links with replacement- url's in a different way; now the escaping can be moved to when the variables are actually being replaced (for normal parameter replacement) and can be processed differently (= not escaping) when dealing with a link. Tests: - Removed useless ok() tests. I accidentally introduced that pattern last year for no good reason. Slowly getting it out again. It either can't return false, or it tests unrelated things that have their own tests. - Simplified examples to reduce repetition of unrelated strings. - Improved assertion titles to better describe what is being tested to help find actual problems. (e.g. "Neutral from mw.user object" instead of "Gender neutral or unknown"). Change-Id: If060b75f5575f4c7c3a25e5280ae697171120e84 --- resources/mediawiki/mediawiki.jqueryMsg.js | 74 ++++++++-- .../mediawiki/mediawiki.jqueryMsg.test.js | 126 ++++++++++++------ 2 files changed, 149 insertions(+), 51 deletions(-) diff --git a/resources/mediawiki/mediawiki.jqueryMsg.js b/resources/mediawiki/mediawiki.jqueryMsg.js index 043ebcee0f..f690a1d30e 100644 --- a/resources/mediawiki/mediawiki.jqueryMsg.js +++ b/resources/mediawiki/mediawiki.jqueryMsg.js @@ -27,13 +27,10 @@ return function( args ) { var key = args[0]; var argsArray = $.isArray( args[1] ) ? args[1] : $.makeArray( args ).slice( 1 ); - var escapedArgsArray = $.map( argsArray, function( arg ) { - return typeof arg === 'string' ? mw.html.escape( arg ) : arg; - } ); try { - return parser.parse( key, escapedArgsArray ); + return parser.parse( key, argsArray ); } catch ( e ) { - return $( '' ).append( key + ': ' + e.message ); + return $( '' ).append( key + ': ' + e.message ); } }; } @@ -139,9 +136,9 @@ }, /** - * Fetch the message string associated with a key, return parsed structure. Memoized. + * Fetch the message string associated with a key, return parsed structure. Memoized. * Note that we pass '[' + key + ']' back for a missing message here. - * @param {String} key + * @param {String} key * @return {String|Array} string of '[key]' if message missing, simple string if possible, array of arrays if needs parsing */ getAst: function( key ) { @@ -280,8 +277,8 @@ var regularLiteral = makeRegexParser( /^[^{}[\]$\\]/ ); - var regularLiteralWithoutBar = makeRegexParser(/^[^{}[\]$\\|]/); - var regularLiteralWithoutSpace = makeRegexParser(/^[^{}[\]$\s]/); + var regularLiteralWithoutBar = makeRegexParser(/^[^{}[\]$\\|]/); + var regularLiteralWithoutSpace = makeRegexParser(/^[^{}[\]$\s]/); var backslash = makeStringParser( "\\" ); var anyCharacter = makeRegexParser( /^./ ); @@ -362,6 +359,22 @@ return result; } + // this is the same as the above extlink, except that the url is being passed on as a parameter + function extLinkParam() { + var result = sequence( [ + openExtlink, + dollar, + digits, + whitespace, + expression, + closeExtlink + ] ); + if ( result === null ) { + return null; + } + return [ 'LINKPARAM', parseInt( result[2], 10 ) - 1, result[4] ]; + } + var openLink = makeStringParser( '[[' ); var closeLink = makeStringParser( ']]' ); @@ -455,16 +468,18 @@ } var nonWhitespaceExpression = choice( [ - template, + template, link, + extLinkParam, extlink, replacement, literalWithoutSpace ] ); var paramExpression = choice( [ - template, + template, link, + extLinkParam, extlink, replacement, literalWithoutBar @@ -473,6 +488,7 @@ var expression = choice( [ template, link, + extLinkParam, extlink, replacement, literal @@ -588,7 +604,7 @@ }, /** - * Return replacement of correct index, or string if unavailable. + * Return escaped replacement of correct index, or string if unavailable. * Note that we expect the parsed parameter to be zero-based. i.e. $1 should have become [ 0 ]. * if the specified parameter is not found return the same string * (e.g. "$99" -> parameter 98 -> not found -> return "$99" ) @@ -598,7 +614,19 @@ */ replace: function( nodes, replacements ) { var index = parseInt( nodes[0], 10 ); - return index < replacements.length ? replacements[index] : '$' + ( index + 1 ); + + if ( index < replacements.length ) { + if ( typeof arg === 'string' ) { + // replacement is a string, escape it + return mw.html.escape( replacements[index] ); + } else { + // replacement is no string, don't touch! + return replacements[index]; + } + } else { + // index not found, fallback to displaying variable + return '$' + ( index + 1 ); + } }, /** @@ -636,6 +664,26 @@ return $el; }, + /** + * This is basically use a combination of replace + link (link with parameter + * as url), but we don't want to run the regular replace here-on: inserting a + * url as href-attribute of a link will automatically escape it already, so + * we don't want replace to (manually) escape it as well. + * TODO throw error if nodes.length > 1 ? + * @param {Array} of one element, integer, n >= 0 + * @return {String} replacement + */ + linkparam: function( nodes, replacements ) { + var replacement, + index = parseInt( nodes[0], 10 ); + if ( index < replacements.length) { + replacement = replacements[index]; + } else { + replacement = '$' + ( index + 1 ); + } + return this.link( [ replacement, nodes[1] ] ); + }, + /** * Transform parsed structure into pluralization * n.b. The first node may be a non-integer (for instance, a string representing an Arabic number). diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js index 2abe016a22..481a5bbbd4 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js @@ -1,55 +1,105 @@ module( 'mediawiki.jqueryMsg' ); -test( '-- Initial check', function() { +test( '-- Initial check', function () { expect( 1 ); ok( mw.jqueryMsg, 'mw.jqueryMsg defined' ); } ); -test( 'mw.jqueryMsg Plural', function() { - expect( 5 ); +test( 'mw.jqueryMsg Plural', function () { + expect( 3 ); var parser = mw.jqueryMsg.getMessageFunction(); - ok( parser, 'Parser Function initialized' ); - ok( mw.messages.set( 'plural-msg', 'Found $1 {{PLURAL:$1|item|items}}' ), 'mw.messages.set: Register' ); - equal( parser( 'plural-msg', 0 ) , 'Found 0 items', 'Plural test for english with zero as count' ); - equal( parser( 'plural-msg', 1 ) , 'Found 1 item', 'Singular test for english' ); - equal( parser( 'plural-msg', 2 ) , 'Found 2 items', 'Plural test for english' ); + + mw.messages.set( 'plural-msg', 'Found $1 {{PLURAL:$1|item|items}}' ); + equal( parser( 'plural-msg', 0 ), 'Found 0 items', 'Plural test for english with zero as count' ); + equal( parser( 'plural-msg', 1 ), 'Found 1 item', 'Singular test for english' ); + equal( parser( 'plural-msg', 2 ), 'Found 2 items', 'Plural test for english' ); } ); -test( 'mw.jqueryMsg Gender', function() { - expect( 16 ); - //TODO: These tests should be for mw.msg once mw.msg integrated with mw.jqueryMsg - var user = mw.user; +test( 'mw.jqueryMsg Gender', function () { + expect( 11 ); + // TODO: These tests should be for mw.msg once mw.msg integrated with mw.jqueryMsg + // TODO: English may not be the best language for these tests. Use a language like Arabic or Russian + var user = mw.user, + parser = mw.jqueryMsg.getMessageFunction(); + + // The values here are not significant, + // what matters is which of the values is choosen by the parser + mw.messages.set( 'gender-msg', '$1: {{GENDER:$2|blue|pink|green}}' ); + user.options.set( 'gender', 'male' ); - var parser = mw.jqueryMsg.getMessageFunction(); - ok( parser, 'Parser Function initialized' ); - //TODO: English may not be the best language for these tests. Use a language like Arabic or Russian - ok( mw.messages.set( 'gender-msg', '$1 reverted {{GENDER:$2|his|her|their}} last edit' ), 'mw.messages.set: Register' ); - equal( parser( 'gender-msg', 'Bob', 'male' ) , 'Bob reverted his last edit', 'Gender masculine' ); - equal( parser( 'gender-msg', 'Bob', user ) , 'Bob reverted his last edit', 'Gender masculine' ); + equal( + parser( 'gender-msg', 'Bob', 'male' ), + 'Bob: blue', + 'Masculine from string "male"' + ); + equal( + parser( 'gender-msg', 'Bob', user ), + 'Bob: blue', + 'Masculine from mw.user object' + ); + user.options.set( 'gender', 'unknown' ); - equal( parser( 'gender-msg', 'They', user ) , 'They reverted their last edit', 'Gender neutral or unknown' ); - equal( parser( 'gender-msg', 'Alice', 'female' ) , 'Alice reverted her last edit', 'Gender feminine' ); - equal( parser( 'gender-msg', 'User' ) , 'User reverted their last edit', 'Gender neutral' ); - equal( parser( 'gender-msg', 'User', 'unknown' ) , 'User reverted their last edit', 'Gender neutral' ); - ok( mw.messages.set( 'gender-msg-one-form', '{{GENDER:$1|User}} reverted last $2 {{PLURAL:$2|edit|edits}}' ), 'mw.messages.set: Register' ); - equal( parser( 'gender-msg-one-form', 'male', 10 ) , 'User reverted last 10 edits', 'Gender neutral and plural form' ); - equal( parser( 'gender-msg-one-form', 'female', 1 ) , 'User reverted last 1 edit', 'Gender neutral and singular form' ); - ok( mw.messages.set( 'gender-msg-lowercase', '{{gender:$1|he|she}} is awesome' ), 'mw.messages.set: Register' ); - equal( parser( 'gender-msg-lowercase', 'male' ) , 'he is awesome', 'Gender masculine' ); - equal( parser( 'gender-msg-lowercase', 'female' ) , 'she is awesome', 'Gender feminine' ); - ok( mw.messages.set( 'gender-msg-wrong', '{{gender}} is awesome' ), 'mw.messages.set: Register' ); - equal( parser( 'gender-msg-wrong', 'female' ) , ' is awesome', 'Wrong syntax used, but ignore the {{gender}}' ); + equal( + parser( 'gender-msg', 'Foo', user ), + 'Foo: green', + 'Neutral from mw.user object' ); + equal( + parser( 'gender-msg', 'Alice', 'female' ), + 'Alice: pink', + 'Feminine from string "female"' ); + equal( + parser( 'gender-msg', 'User' ), + 'User: green', + 'Neutral when no parameter given' ); + equal( + parser( 'gender-msg', 'User', 'unknown' ), + 'User: green', + 'Neutral from string "unknown"' + ); + + mw.messages.set( 'gender-msg-one-form', '{{GENDER:$1|User}}: $2 {{PLURAL:$2|edit|edits}}' ); + + equal( + parser( 'gender-msg-one-form', 'male', 10 ), + 'User: 10 edits', + 'Gender neutral and plural form' + ); + equal( + parser( 'gender-msg-one-form', 'female', 1 ), + 'User: 1 edit', + 'Gender neutral and singular form' + ); + + mw.messages.set( 'gender-msg-lowercase', '{{gender:$1|he|she}} is awesome' ); + equal( + parser( 'gender-msg-lowercase', 'male' ), + 'he is awesome', + 'Gender masculine' + ); + equal( + parser( 'gender-msg-lowercase', 'female' ), + 'she is awesome', + 'Gender feminine' + ); + + mw.messages.set( 'gender-msg-wrong', '{{gender}} test' ); + equal( + parser( 'gender-msg-wrong', 'female' ), + ' test', + 'Invalid syntax should result in {{gender}} simply being stripped away' + ); } ); -test( 'mw.jqueryMsg Grammar', function() { - expect( 5 ); +test( 'mw.jqueryMsg Grammar', function () { + expect( 2 ); var parser = mw.jqueryMsg.getMessageFunction(); - ok( parser, 'Parser Function initialized' ); - // Hope the grammar form grammar_case_foo is not valid in any language - ok( mw.messages.set( 'grammar-msg', 'Przeszukaj {{GRAMMAR:grammar_case_foo|{{SITENAME}}}}' ), 'mw.messages.set: Register' ); - equal( parser( 'grammar-msg' ) , 'Przeszukaj ' + mw.config.get( 'wgSiteName' ) , 'Grammar Test with sitename' ); - ok( mw.messages.set( 'grammar-msg-wrong-syntax', 'Przeszukaj {{GRAMMAR:grammar_case_xyz}}' ), 'mw.messages.set: Register' ); - equal( parser( 'grammar-msg-wrong-syntax' ) , 'Przeszukaj ' , 'Grammar Test with wrong grammar template syntax' ); + + // Assume the grammar form grammar_case_foo is not valid in any language + mw.messages.set( 'grammar-msg', 'Przeszukaj {{GRAMMAR:grammar_case_foo|{{SITENAME}}}}' ); + equal( parser( 'grammar-msg' ), 'Przeszukaj ' + mw.config.get( 'wgSiteName' ), 'Grammar Test with sitename' ); + + mw.messages.set( 'grammar-msg-wrong-syntax', 'Przeszukaj {{GRAMMAR:grammar_case_xyz}}' ); + equal( parser( 'grammar-msg-wrong-syntax' ), 'Przeszukaj ' , 'Grammar Test with wrong grammar template syntax' ); } ); -- 2.20.1