Merge "API: Use U+001F (Unit Separator) for separating multi-valued parameters"

This commit is contained in:
jenkins-bot 2016-08-30 01:45:03 +00:00 committed by Gerrit Code Review
commit 837f40d6be
10 changed files with 257 additions and 15 deletions

View file

@ -47,6 +47,9 @@ production.
collation, you will need to run the updateCollation.php maintenance script.
* Two new codes have been added to #time parser function: "xit" for days in current
month, and "xiz" for days passed in the year, both in Iranian calendar.
* mw.Api has a new option, useUS, to use U+001F (Unit Separator) when
appropriate for sending multi-valued parameters. This defaults to true when
the mw.Api instance seems to be for the local wiki.
=== External library changes in 1.28 ===
@ -84,6 +87,9 @@ production.
action=createaccount, action=linkaccount, and action=changeauthenticationdata
in the query string is now deprecated and outputs a warning. They should be
submitted in the POST body instead.
* (T141960) Multi-valued parameters may now be separated using U+001F (Unit Separator)
instead of the pipe character. This will be useful if some of the multiple
values need to contain pipes, e.g. for action=options.
=== Action API internal changes in 1.28 ===
* Added a new hook, 'ApiMakeParserOptions', to allow extensions to better

View file

@ -394,6 +394,32 @@ class WebRequest {
}
}
/**
* Fetch a scalar from the input without normalization, or return $default
* if it's not set.
*
* Unlike self::getVal(), this does not perform any normalization on the
* input value.
*
* @since 1.28
* @param string $name
* @param string|null $default Optional default
* @return string
*/
public function getRawVal( $name, $default = null ) {
$name = strtr( $name, '.', '_' ); // See comment in self::getGPCVal()
if ( isset( $this->data[$name] ) && !is_array( $this->data[$name] ) ) {
$val = $this->data[$name];
} else {
$val = $default;
}
if ( is_null( $val ) ) {
return $val;
} else {
return (string)$val;
}
}
/**
* Fetch a scalar from the input or return $default if it's not set.
* Returns a string. Arrays are discarded. Useful for

View file

@ -1000,6 +1000,26 @@ abstract class ApiBase extends ContextSource {
$type = $this->getModuleManager()->getNames( $paramName );
}
}
$request = $this->getMain()->getRequest();
$rawValue = $request->getRawVal( $encParamName );
if ( $rawValue === null ) {
$rawValue = $default;
}
// Preserve U+001F for self::parseMultiValue(), or error out if that won't be called
if ( isset( $value ) && substr( $rawValue, 0, 1 ) === "\x1f" ) {
if ( $multi ) {
// This loses the potential $wgContLang->checkTitleEncoding() transformation
// done by WebRequest for $_GET. Let's call that a feature.
$value = join( "\x1f", $request->normalizeUnicode( explode( "\x1f", $rawValue ) ) );
} else {
$this->dieUsage(
"U+001F multi-value separation may only be used for multi-valued parameters.",
'badvalue_notmultivalue'
);
}
}
}
if ( isset( $value ) && ( $multi || is_array( $type ) ) ) {
@ -1145,6 +1165,24 @@ abstract class ApiBase extends ContextSource {
return $value;
}
/**
* Split a multi-valued parameter string, like explode()
* @since 1.28
* @param string $value
* @param int $limit
* @return string[]
*/
protected function explodeMultiValue( $value, $limit ) {
if ( substr( $value, 0, 1 ) === "\x1f" ) {
$sep = "\x1f";
$value = substr( $value, 1 );
} else {
$sep = '|';
}
return explode( $sep, $value, $limit );
}
/**
* Return an array of values that were given in a 'a|b|c' notation,
* after it optionally validates them against the list allowed values.
@ -1159,13 +1197,13 @@ abstract class ApiBase extends ContextSource {
* @return string|string[] (allowMultiple ? an_array_of_values : a_single_value)
*/
protected function parseMultiValue( $valueName, $value, $allowMultiple, $allowedValues ) {
if ( trim( $value ) === '' && $allowMultiple ) {
if ( ( trim( $value ) === '' || trim( $value ) === "\x1f" ) && $allowMultiple ) {
return [];
}
// This is a bit awkward, but we want to avoid calling canApiHighLimits()
// because it unstubs $wgUser
$valuesList = explode( '|', $value, self::LIMIT_SML2 + 1 );
$valuesList = $this->explodeMultiValue( $value, self::LIMIT_SML2 + 1 );
$sizeLimit = count( $valuesList ) > self::LIMIT_SML1 && $this->mMainModule->canApiHighLimits()
? self::LIMIT_SML2
: self::LIMIT_SML1;

View file

@ -1481,14 +1481,14 @@
"api-help-param-deprecated": "Deprecated.",
"api-help-param-required": "This parameter is required.",
"api-help-datatypes-header": "Data types",
"api-help-datatypes": "Some parameter types in API requests need further explanation:\n;boolean\n:Boolean parameters work like HTML checkboxes: if the parameter is specified, regardless of value, it is considered true. For a false value, omit the parameter entirely.\n;timestamp\n:Timestamps may be specified in several formats. ISO 8601 date and time is recommended. All times are in UTC, any included timezone is ignored.\n:* ISO 8601 date and time, <kbd><var>2001</var>-<var>01</var>-<var>15</var>T<var>14</var>:<var>56</var>:<var>00</var>Z</kbd> (punctuation and <kbd>Z</kbd> are optional)\n:* ISO 8601 date and time with (ignored) fractional seconds, <kbd><var>2001</var>-<var>01</var>-<var>15</var>T<var>14</var>:<var>56</var>:<var>00</var>.<var>00001</var>Z</kbd> (dashes, colons, and <kbd>Z</kbd> are optional)\n:* MediaWiki format, <kbd><var>2001</var><var>01</var><var>15</var><var>14</var><var>56</var><var>00</var></kbd>\n:* Generic numeric format, <kbd><var>2001</var>-<var>01</var>-<var>15</var> <var>14</var>:<var>56</var>:<var>00</var></kbd> (optional timezone of <kbd>GMT</kbd>, <kbd>+<var>##</var></kbd>, or <kbd>-<var>##</var></kbd> is ignored)\n:* EXIF format, <kbd><var>2001</var>:<var>01</var>:<var>15</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:*RFC 2822 format (timezone may be omitted), <kbd><var>Mon</var>, <var>15</var> <var>Jan</var> <var>2001</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:* RFC 850 format (timezone may be omitted), <kbd><var>Monday</var>, <var>15</var>-<var>Jan</var>-<var>2001</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:* C ctime format, <kbd><var>Mon</var> <var>Jan</var> <var>15</var> <var>14</var>:<var>56</var>:<var>00</var> <var>2001</var></kbd>\n:* Seconds since 1970-01-01T00:00:00Z as a 1 to 13 digit integer (excluding <kbd>0</kbd>)\n:* The string <kbd>now</kbd>",
"api-help-datatypes": "Some parameter types in API requests need further explanation:\n;boolean\n:Boolean parameters work like HTML checkboxes: if the parameter is specified, regardless of value, it is considered true. For a false value, omit the parameter entirely.\n;timestamp\n:Timestamps may be specified in several formats. ISO 8601 date and time is recommended. All times are in UTC, any included timezone is ignored.\n:* ISO 8601 date and time, <kbd><var>2001</var>-<var>01</var>-<var>15</var>T<var>14</var>:<var>56</var>:<var>00</var>Z</kbd> (punctuation and <kbd>Z</kbd> are optional)\n:* ISO 8601 date and time with (ignored) fractional seconds, <kbd><var>2001</var>-<var>01</var>-<var>15</var>T<var>14</var>:<var>56</var>:<var>00</var>.<var>00001</var>Z</kbd> (dashes, colons, and <kbd>Z</kbd> are optional)\n:* MediaWiki format, <kbd><var>2001</var><var>01</var><var>15</var><var>14</var><var>56</var><var>00</var></kbd>\n:* Generic numeric format, <kbd><var>2001</var>-<var>01</var>-<var>15</var> <var>14</var>:<var>56</var>:<var>00</var></kbd> (optional timezone of <kbd>GMT</kbd>, <kbd>+<var>##</var></kbd>, or <kbd>-<var>##</var></kbd> is ignored)\n:* EXIF format, <kbd><var>2001</var>:<var>01</var>:<var>15</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:*RFC 2822 format (timezone may be omitted), <kbd><var>Mon</var>, <var>15</var> <var>Jan</var> <var>2001</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:* RFC 850 format (timezone may be omitted), <kbd><var>Monday</var>, <var>15</var>-<var>Jan</var>-<var>2001</var> <var>14</var>:<var>56</var>:<var>00</var></kbd>\n:* C ctime format, <kbd><var>Mon</var> <var>Jan</var> <var>15</var> <var>14</var>:<var>56</var>:<var>00</var> <var>2001</var></kbd>\n:* Seconds since 1970-01-01T00:00:00Z as a 1 to 13 digit integer (excluding <kbd>0</kbd>)\n:* The string <kbd>now</kbd>\n;alternative multiple-value separator\n:Parameters that take multiple values are normally submitted with the values separated using the pipe character, e.g. <kbd>param=value1|value2</kbd> or <kbd>param=value1%7Cvalue2</kbd>. If a value must contain the pipe character, use U+001F (Unit Separator) as the separator ''and'' prefix the value with U+001F, e.g. <kbd>param=%1Fvalue1%1Fvalue2</kbd>.",
"api-help-param-type-limit": "Type: integer or <kbd>max</kbd>",
"api-help-param-type-integer": "Type: {{PLURAL:$1|1=integer|2=list of integers}}",
"api-help-param-type-boolean": "Type: boolean ([[Special:ApiHelp/main#main/datatypes|details]])",
"api-help-param-type-password": "",
"api-help-param-type-timestamp": "Type: {{PLURAL:$1|1=timestamp|2=list of timestamps}} ([[Special:ApiHelp/main#main/datatypes|allowed formats]])",
"api-help-param-type-user": "Type: {{PLURAL:$1|1=user name|2=list of user names}}",
"api-help-param-list": "{{PLURAL:$1|1=One of the following values|2=Values (separate with <kbd>{{!}}</kbd>)}}: $2",
"api-help-param-list": "{{PLURAL:$1|1=One of the following values|2=Values (separate with <kbd>{{!}}</kbd> or [[Special:ApiHelp/main#main/datatypes|alternative]])}}: $2",
"api-help-param-list-can-be-empty": "{{PLURAL:$1|0=Must be empty|Can be empty, or $2}}",
"api-help-param-limit": "No more than $1 allowed.",
"api-help-param-limit2": "No more than $1 ($2 for bots) allowed.",
@ -1496,7 +1496,7 @@
"api-help-param-integer-max": "The {{PLURAL:$1|1=value|2=values}} must be no greater than $3.",
"api-help-param-integer-minmax": "The {{PLURAL:$1|1=value|2=values}} must be between $2 and $3.",
"api-help-param-upload": "Must be posted as a file upload using multipart/form-data.",
"api-help-param-multi-separate": "Separate values with <kbd>|</kbd>.",
"api-help-param-multi-separate": "Separate values with <kbd>|</kbd> or [[Special:ApiHelp/main#main/datatypes|alternative]].",
"api-help-param-multi-max": "Maximum number of values is {{PLURAL:$1|$1}} ({{PLURAL:$2|$2}} for bots).",
"api-help-param-default": "Default: $1",
"api-help-param-default-empty": "Default: <span class=\"apihelp-empty\">(empty)</span>",

View file

@ -116,10 +116,24 @@
capsuleWidget: {
getApiValue: function () {
return this.getItemsData().join( '|' );
var items = this.getItemsData();
if ( items.join( '' ).indexOf( '|' ) === -1 ) {
return items.join( '|' );
} else {
return '\x1f' + items.join( '\x1f' );
}
},
setApiValue: function ( v ) {
this.setItemsFromData( v === undefined || v === '' ? [] : String( v ).split( '|' ) );
if ( v === undefined || v === '' || v === '\x1f' ) {
this.setItemsFromData( [] );
} else {
v = String( v );
if ( v.indexOf( '\x1f' ) !== 0 ) {
this.setItemsFromData( v.split( '|' ) );
} else {
this.setItemsFromData( v.substr( 1 ).split( '\x1f' ) );
}
}
},
apiCheckValid: function () {
var ok = this.getApiValue() !== undefined || suppressErrors;

View file

@ -9,6 +9,9 @@
* `options` to mw.Api constructor.
* @property {Object} defaultOptions.parameters Default query parameters for API requests.
* @property {Object} defaultOptions.ajax Default options for jQuery#ajax.
* @property {boolean} defaultOptions.useUS Whether to use U+001F when joining multi-valued
* parameters (since 1.28). Default is true if ajax.url is not set, false otherwise for
* compatibility.
* @private
*/
var defaultOptions = {
@ -95,6 +98,8 @@
options.ajax.url = String( options.ajax.url );
}
options = $.extend( { useUS: !options.ajax || !options.ajax.url }, options );
options.parameters = $.extend( {}, defaultOptions.parameters, options.parameters );
options.ajax = $.extend( {}, defaultOptions.ajax, options.ajax );
@ -147,14 +152,19 @@
*
* @private
* @param {Object} parameters (modified in-place)
* @param {boolean} useUS Whether to use U+001F when joining multi-valued parameters.
*/
preprocessParameters: function ( parameters ) {
preprocessParameters: function ( parameters, useUS ) {
var key;
// Handle common MediaWiki API idioms for passing parameters
for ( key in parameters ) {
// Multiple values are pipe-separated
if ( $.isArray( parameters[ key ] ) ) {
parameters[ key ] = parameters[ key ].join( '|' );
if ( !useUS || parameters[ key ].join( '' ).indexOf( '|' ) === -1 ) {
parameters[ key ] = parameters[ key ].join( '|' );
} else {
parameters[ key ] = '\x1f' + parameters[ key ].join( '\x1f' );
}
}
// Boolean values are only false when not given at all
if ( parameters[ key ] === false || parameters[ key ] === undefined ) {
@ -186,7 +196,7 @@
delete parameters.token;
}
this.preprocessParameters( parameters );
this.preprocessParameters( parameters, this.defaults.useUS );
// If multipart/form-data has been requested and emulation is possible, emulate it
if (

View file

@ -41,9 +41,13 @@
value = options[ name ] === null ? null : String( options[ name ] );
// Can we bundle this option, or does it need a separate request?
bundleable =
( value === null || value.indexOf( '|' ) === -1 ) &&
( name.indexOf( '|' ) === -1 && name.indexOf( '=' ) === -1 );
if ( this.defaults.useUS ) {
bundleable = name.indexOf( '=' ) === -1;
} else {
bundleable =
( value === null || value.indexOf( '|' ) === -1 ) &&
( name.indexOf( '|' ) === -1 && name.indexOf( '=' ) === -1 );
}
if ( bundleable ) {
if ( value !== null ) {

View file

@ -43,4 +43,80 @@ class ApiBaseTest extends ApiTestCase {
);
}
/**
* @dataProvider provideGetParameterFromSettings
* @param string|null $input
* @param array $paramSettings
* @param mixed $expected
* @param string[] $warnings
*/
public function testGetParameterFromSettings( $input, $paramSettings, $expected, $warnings ) {
$mock = new MockApi();
$wrapper = TestingAccessWrapper::newFromObject( $mock );
$context = new DerivativeContext( $mock );
$context->setRequest( new FauxRequest( $input !== null ? [ 'foo' => $input ] : [] ) );
$wrapper->mMainModule = new ApiMain( $context );
if ( $expected instanceof UsageException ) {
try {
$wrapper->getParameterFromSettings( 'foo', $paramSettings, true );
} catch ( UsageException $ex ) {
$this->assertEquals( $expected, $ex );
}
} else {
$result = $wrapper->getParameterFromSettings( 'foo', $paramSettings, true );
$this->assertSame( $expected, $result );
$this->assertSame( $warnings, $mock->warnings );
}
}
public static function provideGetParameterFromSettings() {
$c0 = '';
$enc = '';
for ( $i = 0; $i < 32; $i++ ) {
$c0 .= chr( $i );
$enc .= ( $i === 9 || $i === 10 || $i === 13 )
? chr( $i )
: '<27>';
}
return [
'Basic param' => [ 'bar', null, 'bar', [] ],
'String param' => [ 'bar', '', 'bar', [] ],
'String param, defaulted' => [ null, '', '', [] ],
'String param, empty' => [ '', 'default', '', [] ],
'String param, required, empty' => [
'',
[ ApiBase::PARAM_DFLT => 'default', ApiBase::PARAM_REQUIRED => true ],
new UsageException( 'The foo parameter must be set', 'nofoo' ),
[]
],
'Multi-valued parameter' => [
'a|b|c',
[ ApiBase::PARAM_ISMULTI => true ],
[ 'a', 'b', 'c' ],
[]
],
'Multi-valued parameter, alternative separator' => [
"\x1fa|b\x1fc|d",
[ ApiBase::PARAM_ISMULTI => true ],
[ 'a|b', 'c|d' ],
[]
],
'Multi-valued parameter, other C0 controls' => [
$c0,
[ ApiBase::PARAM_ISMULTI => true ],
[ $enc ],
[]
],
'Multi-valued parameter, other C0 controls (2)' => [
"\x1f" . $c0,
[ ApiBase::PARAM_ISMULTI => true ],
[ substr( $enc, 0, -3 ), '' ],
[]
],
];
}
}

View file

@ -1,12 +1,18 @@
<?php
class MockApi extends ApiBase {
public $warnings = [];
public function execute() {
}
public function __construct() {
}
public function setWarning( $warning ) {
$this->warnings[] = $warning;
}
public function getAllowedParams() {
return [
'filename' => null,

View file

@ -18,10 +18,10 @@
assert.deepEqual( stub.getCall( 0 ).args, [ { foo: 'bar' } ], '#saveOptions called correctly' );
} );
QUnit.test( 'saveOptions', function ( assert ) {
QUnit.test( 'saveOptions without Unit Separator', function ( assert ) {
QUnit.expect( 13 );
var api = new mw.Api();
var api = new mw.Api( { useUS: false } );
// We need to respond to the request for token first, otherwise the other requests won't be sent
// until after the server.respond call, which confuses sinon terribly. This sucks a lot.
@ -75,4 +75,66 @@
}
} );
} );
QUnit.test( 'saveOptions with Unit Separator', function ( assert ) {
QUnit.expect( 14 );
var api = new mw.Api( { useUS: true } );
// We need to respond to the request for token first, otherwise the other requests won't be sent
// until after the server.respond call, which confuses sinon terribly. This sucks a lot.
api.getToken( 'options' );
this.server.respond(
/meta=tokens&type=csrf/,
[ 200, { 'Content-Type': 'application/json' },
'{ "query": { "tokens": { "csrftoken": "+\\\\" } } }' ]
);
api.saveOptions( {} ).done( function () {
assert.ok( true, 'Request completed: empty case' );
} );
api.saveOptions( { foo: 'bar' } ).done( function () {
assert.ok( true, 'Request completed: simple' );
} );
api.saveOptions( { foo: 'bar', baz: 'quux' } ).done( function () {
assert.ok( true, 'Request completed: two options' );
} );
api.saveOptions( { foo: 'bar|quux', bar: 'a|b|c', baz: 'quux' } ).done( function () {
assert.ok( true, 'Request completed: bundleable with unit separator' );
} );
api.saveOptions( { foo: 'bar|quux', bar: 'a|b|c', 'baz=baz': 'quux' } ).done( function () {
assert.ok( true, 'Request completed: not bundleable with unit separator' );
} );
api.saveOptions( { foo: null } ).done( function () {
assert.ok( true, 'Request completed: reset an option' );
} );
api.saveOptions( { 'foo|bar=quux': null } ).done( function () {
assert.ok( true, 'Request completed: reset an option, not bundleable' );
} );
// Requests are POST, match requestBody instead of url
this.server.respond( function ( request ) {
switch ( request.requestBody ) {
// simple
case 'action=options&format=json&formatversion=2&change=foo%3Dbar&token=%2B%5C':
// two options
case 'action=options&format=json&formatversion=2&change=foo%3Dbar%7Cbaz%3Dquux&token=%2B%5C':
// bundleable with unit separator
case 'action=options&format=json&formatversion=2&change=%1Ffoo%3Dbar%7Cquux%1Fbar%3Da%7Cb%7Cc%1Fbaz%3Dquux&token=%2B%5C':
// not bundleable with unit separator
case 'action=options&format=json&formatversion=2&optionname=baz%3Dbaz&optionvalue=quux&token=%2B%5C':
case 'action=options&format=json&formatversion=2&change=%1Ffoo%3Dbar%7Cquux%1Fbar%3Da%7Cb%7Cc&token=%2B%5C':
// reset an option
case 'action=options&format=json&formatversion=2&change=foo&token=%2B%5C':
// reset an option, not bundleable
case 'action=options&format=json&formatversion=2&optionname=foo%7Cbar%3Dquux&token=%2B%5C':
assert.ok( true, 'Repond to ' + request.requestBody );
request.respond( 200, { 'Content-Type': 'application/json' },
'{ "options": "success" }' );
break;
default:
assert.ok( false, 'Unexpected request: ' + request.requestBody );
}
} );
} );
}( mediaWiki ) );