From 683bfad2683691b1d048df390863e83b0ebe6fc9 Mon Sep 17 00:00:00 2001 From: Max Semenik Date: Thu, 1 Aug 2019 18:22:14 -0700 Subject: [PATCH] Bail out gracefully on non-SVG files Bug: T229459 --- .phpcs.xml | 1 + i18n/en.json | 1 + i18n/qqq.json | 1 + public/assets/i18n/app/en.json | 1 + public/assets/i18n/app/qqq.json | 1 + src/Controller/TranslateController.php | 51 ++++++++++++++++---- src/Exception/InvalidFormatException.php | 14 ++++++ src/Service/Retriever.php | 6 +++ tests/Controller/TranslateControllerTest.php | 8 +++ 9 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 src/Exception/InvalidFormatException.php diff --git a/.phpcs.xml b/.phpcs.xml index 4ab85427..1fc70ae8 100644 --- a/.phpcs.xml +++ b/.phpcs.xml @@ -11,6 +11,7 @@ node_modules/ var/ public/assets + .docker diff --git a/i18n/en.json b/i18n/en.json index 58c68423..783523b2 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -25,6 +25,7 @@ "translate-button": "Translate", "no-translations": "This file does not have any labels available for translation. Please pick another image.", "not-found": "The file you requested could not be found. Please pick another image.", + "invalid-format": "Only SVG files are supported.", "pick-another": "← pick another file", "view-on-commons": "View on Commons", diff --git a/i18n/qqq.json b/i18n/qqq.json index 5be4dfd5..d6e7cdd2 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -29,6 +29,7 @@ "translate-button": "Search button text.\n{{Identical|Translate}}", "no-translations": "Error message displayed for SVGs that have nothing to translate.", "not-found": "Error message displayed if the requested file could not be found.", + "invalid-format": "Error message displayed if the requested file is not an SVG image.", "pick-another": "Label for backlink from the translate page to the search page.", "view-on-commons": "Link to view the file on Commons. The whole phrase is linked.", "opens-in-new-tab": "Tooltip text for links that will open in a new browser tab.", diff --git a/public/assets/i18n/app/en.json b/public/assets/i18n/app/en.json index 58c68423..783523b2 100644 --- a/public/assets/i18n/app/en.json +++ b/public/assets/i18n/app/en.json @@ -25,6 +25,7 @@ "translate-button": "Translate", "no-translations": "This file does not have any labels available for translation. Please pick another image.", "not-found": "The file you requested could not be found. Please pick another image.", + "invalid-format": "Only SVG files are supported.", "pick-another": "← pick another file", "view-on-commons": "View on Commons", diff --git a/public/assets/i18n/app/qqq.json b/public/assets/i18n/app/qqq.json index 5be4dfd5..d6e7cdd2 100644 --- a/public/assets/i18n/app/qqq.json +++ b/public/assets/i18n/app/qqq.json @@ -29,6 +29,7 @@ "translate-button": "Search button text.\n{{Identical|Translate}}", "no-translations": "Error message displayed for SVGs that have nothing to translate.", "not-found": "Error message displayed if the requested file could not be found.", + "invalid-format": "Error message displayed if the requested file is not an SVG image.", "pick-another": "Label for backlink from the translate page to the search page.", "view-on-commons": "Link to view the file on Commons. The whole phrase is linked.", "opens-in-new-tab": "Tooltip text for links that will open in a new browser tab.", diff --git a/src/Controller/TranslateController.php b/src/Controller/TranslateController.php index 06608f6f..0f6cf4fd 100644 --- a/src/Controller/TranslateController.php +++ b/src/Controller/TranslateController.php @@ -4,6 +4,7 @@ namespace App\Controller; use App\Exception\ImageNotFoundException; +use App\Exception\InvalidFormatException; use App\Model\Svg\SvgFile; use App\Model\Title; use App\OOUI\TranslationsFieldset; @@ -55,18 +56,15 @@ public function translate( // Fetch the SVG from Commons. try { + $this->assertFileType($normalizedFilename); $path = $cache->getPath($filename); } catch (ImageNotFoundException $exception) { - $notFoundMessage = $this->renderView( - 'error_message.html.twig', - ['msg_name' => 'not-found'] - ); - // Flash the message to show to the user under the search form. - $this->addFlash('search-errors', (string)$notFoundMessage); - // Also flash the failed filename so we can populate the search form. - $this->addFlash('search-redirect', $normalizedFilename); - return $this->redirectToRoute('home'); + return $this->showError('not-found', $normalizedFilename); + } catch (InvalidFormatException $exception) { + return $this->showError('invalid-format', $normalizedFilename); } + + $svgFile = new SvgFile($path); // If there are no strings to translate, tell the user. @@ -227,4 +225,39 @@ public function updownload( return $this->redirectToRoute('translate', ['filename' => $filename]); } } + + /** + * Shows an error related to the selected file + * + * @param string $messageKey + * @param string $fileName + * @return Response + */ + private function showError(string $messageKey, string $fileName): Response + { + $message = $this->renderView( + 'error_message.html.twig', + ['msg_name' => $messageKey] + ); + // Flash the message to show to the user under the search form. + $this->addFlash('search-errors', (string)$message); + // Also flash the failed filename so we can populate the search form. + $this->addFlash('search-redirect', $fileName); + + return $this->redirectToRoute('home'); + } + + /** + * Throws an exception if the given filename is not of an SVG file. + * Exceptions are used to unify handling with other places that might encounter this problem. + * + * @param string $fileName + */ + private function assertFileType(string $fileName): void + { + $extension = mb_strtolower(pathinfo($fileName, PATHINFO_EXTENSION)); + if ('svg' !== $extension) { + throw new InvalidFormatException($extension); + } + } } diff --git a/src/Exception/InvalidFormatException.php b/src/Exception/InvalidFormatException.php new file mode 100644 index 00000000..2fe0a485 --- /dev/null +++ b/src/Exception/InvalidFormatException.php @@ -0,0 +1,14 @@ +get($url); + $contentType = $response->getHeader('Content-Type'); + $contentType = reset($contentType); + if ('image/svg+xml' !== $contentType) { + throw new InvalidFormatException($contentType); + } return $response->getBody()->getContents(); } } diff --git a/tests/Controller/TranslateControllerTest.php b/tests/Controller/TranslateControllerTest.php index f9fa8956..aa600b91 100644 --- a/tests/Controller/TranslateControllerTest.php +++ b/tests/Controller/TranslateControllerTest.php @@ -19,4 +19,12 @@ public function testExists(): void static::assertEquals(200, $response->getStatusCode()); static::assertContains('Test.svg', $crawler->filter('h1')->text()); } + + public function testNonSvgHandling(): void + { + $client = static::createClient(); + $client->request('GET', '/File:Test.jpg'); + $response = $client->getResponse(); + static::assertEquals(302, $response->getStatusCode()); + } }