From c4e9f987f1dfa15f652747347dfd403f47393b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Tue, 4 Feb 2025 13:31:32 +0100 Subject: [PATCH] rest: Return a 400 for invalid render IDs Why: - The REST API takes an optional renderid param when converting HTML back to source wikitext, which is user-provided and may be invalid. - Invalid render IDs cause an InvalidArgumentException to be thrown that causes a 500 response. What: - Introduce a new error message for invalid render IDs in the REST API. - Return a 400 with this new error message for HTML reverse-parses with an invalid render ID. Bug: T385568 Change-Id: I062419fe8952329a39781a49cdca2e94c3996447 (cherry picked from commit cd1d42a5066e4bcb9b9d4ed9b4f7714fd428fea3) --- .../Handler/Helper/HtmlInputTransformHelper.php | 9 ++++++++- includes/Rest/i18n/en.json | 1 + includes/Rest/i18n/qqq.json | 1 + .../Helper/HtmlInputTransformHelperTest.php | 14 ++++++++++++++ 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/includes/Rest/Handler/Helper/HtmlInputTransformHelper.php b/includes/Rest/Handler/Helper/HtmlInputTransformHelper.php index abe1fe0178a..7378b41369f 100644 --- a/includes/Rest/Handler/Helper/HtmlInputTransformHelper.php +++ b/includes/Rest/Handler/Helper/HtmlInputTransformHelper.php @@ -368,7 +368,14 @@ class HtmlInputTransformHelper { throw new LocalizedHttpException( new MessageValue( "rest-bad-etag", [ $key ] ), 400 ); } } else { - $originalRendering = ParsoidRenderID::newFromKey( $key ); + try { + $originalRendering = ParsoidRenderID::newFromKey( $key ); + } catch ( InvalidArgumentException $e ) { + throw new LocalizedHttpException( + new MessageValue( 'rest-parsoid-bad-render-id', [ $key ] ), + 400 + ); + } } } elseif ( !empty( $original['html'] ) || !empty( $original['data-parsoid'] ) ) { // NOTE: We might have an incomplete PageBundle here, with no HTML but with data-parsoid! diff --git a/includes/Rest/i18n/en.json b/includes/Rest/i18n/en.json index ebecc4fd98a..9555efa630d 100644 --- a/includes/Rest/i18n/en.json +++ b/includes/Rest/i18n/en.json @@ -65,6 +65,7 @@ "rest-unsupported-target-format": "The requested target format is not supported.", "rest-parsoid-resource-exceeded": "Resource limit exceeded", "rest-parsoid-error": "Parsoid error.", + "rest-parsoid-bad-render-id": "Bad Parsoid render ID: $1", "rest-bad-stash-key": "Bad stash key.", "rest-html-key-expected": "Expected html key in body", "rest-invalid-transform": "Invalid transform: $1 to $2", diff --git a/includes/Rest/i18n/qqq.json b/includes/Rest/i18n/qqq.json index bd7a33191e8..32eca9e94cb 100644 --- a/includes/Rest/i18n/qqq.json +++ b/includes/Rest/i18n/qqq.json @@ -69,6 +69,7 @@ "rest-transform-missing-title": "Error message for REST API debugging, shown when no title or wikitext is provided", "rest-unsupported-target-format": "Error message for REST API debugging, shown when the requested target format is not supported", "rest-parsoid-resource-exceeded": "Error message for REST API debugging, shown when a resource limit is exceeded while converting between wikitext and HTML.", + "rest-parsoid-bad-render-id": "Error message for REST API debugging, shown when a ParsoidRenderID object cannot be created from the provided key. Parameters:\n* $1: The key", "rest-parsoid-error": "Error message for REST API debugging, indicating an unspecified backend error occurred while converting between wikitext and HTML.\n\n[[:mw:Parsoid|Parsoid]] is the name of a software library; do not translate that name.", "rest-bad-stash-key": "Error message for REST API debugging, shown when When the rednerid or etag given in the request is not a valid stash key.", "rest-html-key-expected": "Error message for REST API debugging, shown when when the \"html\" key is missing from the request body", diff --git a/tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlInputTransformHelperTest.php b/tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlInputTransformHelperTest.php index 556657cb11d..e960f5d3a39 100644 --- a/tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlInputTransformHelperTest.php +++ b/tests/phpunit/integration/includes/Rest/Handler/Helper/HtmlInputTransformHelperTest.php @@ -1188,6 +1188,20 @@ class HtmlInputTransformHelperTest extends MediaWikiIntegrationTestCase { $helper->getContent(); } + public function testHandlesInvalidRenderID(): void { + $page = $this->getExistingTestPage( __METHOD__ ); + + $body = [ 'html' => 'hi', 'original' => [ 'renderid' => 'foo' ] ]; + $params = []; + + $this->expectExceptionObject( new LocalizedHttpException( + new MessageValue( 'rest-parsoid-bad-render-id', [ 'foo' ] ), + 400 + ) ); + + $this->newHelper( [], StatsFactory::newNull(), $page, $body, $params ); + } + private function newHtmlToContentTransform( $html, $methodOverrides = [] ): HtmlToContentTransform { $transform = $this->getMockBuilder( HtmlToContentTransform::class ) ->onlyMethods( array_keys( $methodOverrides ) )