API: Validate API error codes

Validate them in ApiMessageTrait when the message is created, and again
in ApiMain before they're included in the header.

This also introduces an "api-warning" log channel, since "api" is too
spammy for real use, and converts a few existing things to use it.

Bug: T208926
Change-Id: Ib2d8bd4d4a5d58af76431835ba783c148de7792a
Depends-On: Iced44f2602d57eea9a2d15aee5b8c9a50092b49c
Depends-On: I5c2747f527c30ded7a614feb26f5777d901bd512
Depends-On: I9c9bd8f5309518fcbab7179fb71d209c005e5e64

3 files changed, 77 insertions, 0 deletions

diff --git a/tests/phpunit/includes/api/ApiErrorFormatterTest.php b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
index d11e3143ea01..65a511d85bec 100644
--- a/tests/phpunit/includes/api/ApiErrorFormatterTest.php
+++ b/tests/phpunit/includes/api/ApiErrorFormatterTest.php
@@ -632,4 +632,23 @@ class ApiErrorFormatterTest extends MediaWikiLangTestCase {
 		];
 	}
 
+	/**
+	 * @dataProvider provideIsValidApiCode
+	 * @covers ApiErrorFormatter::isValidApiCode
+	 * @param string $code
+	 * @param bool $expect
+	 */
+	public function testIsValidApiCode( $code, $expect ) {
+		$this->assertSame( $expect, ApiErrorFormatter::isValidApiCode( $code ) );
+	}
+
+	public static function provideIsValidApiCode() {
+		return [
+			[ 'foo-bar_Baz123', true ],
+			[ 'foo bar', false ],
+			[ 'foo\\bar', false ],
+			[ 'internal_api_error_foo\\bar baz', true ],
+		];
+	}
+
 }
diff --git a/tests/phpunit/includes/api/ApiMainTest.php b/tests/phpunit/includes/api/ApiMainTest.php
index a6083cda8977..3299e73dc9f2 100644
--- a/tests/phpunit/includes/api/ApiMainTest.php
+++ b/tests/phpunit/includes/api/ApiMainTest.php
@@ -1010,6 +1010,15 @@ class ApiMainTest extends ApiTestCase {
 			MWExceptionHandler::getRedactedTraceAsString( $dbex )
 		)->inLanguage( 'en' )->useDatabase( false )->text();
 
+		// The specific exception doesn't matter, as long as it's namespaced.
+		$nsex = new MediaWiki\ShellDisabledError();
+		$nstrace = wfMessage( 'api-exception-trace',
+			get_class( $nsex ),
+			$nsex->getFile(),
+			$nsex->getLine(),
+			MWExceptionHandler::getRedactedTraceAsString( $nsex )
+		)->inLanguage( 'en' )->useDatabase( false )->text();
+
 		$apiEx1 = new ApiUsageException( null,
 			StatusValue::newFatal( new ApiRawMessage( 'An error', 'sv-error1' ) ) );
 		TestingAccessWrapper::newFromObject( $apiEx1 )->modulePath = 'foo+bar';
@@ -1017,6 +1026,13 @@ class ApiMainTest extends ApiTestCase {
 		$apiEx1->getStatusValue()->warning( new ApiRawMessage( 'Another warning', 'sv-warn2' ) );
 		$apiEx1->getStatusValue()->fatal( new ApiRawMessage( 'Another error', 'sv-error2' ) );
 
+		$badMsg = $this->getMockBuilder( ApiRawMessage::class )
+			 ->setConstructorArgs( [ 'An error', 'ignored' ] )
+			 ->setMethods( [ 'getApiCode' ] )
+			 ->getMock();
+		$badMsg->method( 'getApiCode' )->willReturn( "bad\nvalue" );
+		$apiEx2 = new ApiUsageException( null, StatusValue::newFatal( $badMsg ) );
+
 		return [
 			[
 				$ex,
@@ -1056,6 +1072,24 @@ class ApiMainTest extends ApiTestCase {
 				]
 			],
 			[
+				$nsex,
+				[ 'existing-error', 'internal_api_error_MediaWiki\ShellDisabledError' ],
+				[
+					'warnings' => [
+						[ 'code' => 'existing-warning', 'text' => 'existing warning', 'module' => 'main' ],
+					],
+					'errors' => [
+						[ 'code' => 'existing-error', 'text' => 'existing error', 'module' => 'main' ],
+						[
+							'code' => 'internal_api_error_MediaWiki\ShellDisabledError',
+							'text' => "[$reqId] Exception caught: " . $nsex->getMessage(),
+						]
+					],
+					'trace' => $nstrace,
+					'servedby' => wfHostname(),
+				]
+			],
+			[
 				$apiEx1,
 				[ 'existing-error', 'sv-error1', 'sv-error2' ],
 				[
@@ -1075,6 +1109,23 @@ class ApiMainTest extends ApiTestCase {
 					'servedby' => wfHostname(),
 				]
 			],
+			[
+				$apiEx2,
+				[ 'existing-error', '<invalid-code>' ],
+				[
+					'warnings' => [
+						[ 'code' => 'existing-warning', 'text' => 'existing warning', 'module' => 'main' ],
+					],
+					'errors' => [
+						[ 'code' => 'existing-error', 'text' => 'existing error', 'module' => 'main' ],
+						[ 'code' => "bad\nvalue", 'text' => 'An error' ],
+					],
+					'docref' => "See $doclink for API usage. Subscribe to the mediawiki-api-announce mailing " .
+						"list at &lt;https://lists.wikimedia.org/mailman/listinfo/mediawiki-api-announce&gt; " .
+						"for notice of API deprecations and breaking changes.",
+					'servedby' => wfHostname(),
+				]
+			]
 		];
 	}
 
diff --git a/tests/phpunit/includes/api/ApiMessageTest.php b/tests/phpunit/includes/api/ApiMessageTest.php
index c6f5a8e791f8..70114c2593b6 100644
--- a/tests/phpunit/includes/api/ApiMessageTest.php
+++ b/tests/phpunit/includes/api/ApiMessageTest.php
@@ -38,6 +38,10 @@ class ApiMessageTest extends MediaWikiTestCase {
 		$msg = new ApiMessage( 'apiwarn-baz' );
 		$this->assertSame( 'baz', $msg->getApiCode() );
 
+		// Weird "message key"
+		$msg = new ApiMessage( "<foo> bar\nbaz" );
+		$this->assertSame( '_foo__bar_baz', $msg->getApiCode() );
+
 		// BC case
 		$msg = new ApiMessage( 'actionthrottledtext' );
 		$this->assertSame( 'ratelimited', $msg->getApiCode() );
@@ -72,6 +76,9 @@ class ApiMessageTest extends MediaWikiTestCase {
 		return [
 			[ '' ],
 			[ 42 ],
+			[ 'A bad code' ],
+			[ 'Project:A_page_title' ],
+			[ "WTF\nnewlines" ],
 		];
 	}