From 6e5999e6177e288da3383165d3b7cc6e331ec14a Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 12 Dec 2017 16:19:54 +1300 Subject: [PATCH 1/2] API Refactor input and output format for TinyMCE 4 compatibility --- readme.md | 5 ++ src/Handling/SpellCheckMiddleware.php | 16 +++--- src/Handling/SpellController.php | 69 ++++++++++++++---------- tests/SpellControllerTest.php | 78 ++++++++++----------------- 4 files changed, 84 insertions(+), 84 deletions(-) diff --git a/readme.md b/readme.md index c40d546..f3f53df 100644 --- a/readme.md +++ b/readme.md @@ -15,6 +15,11 @@ Ensure that your server is setup with [hunspell](http://hunspell.sourceforge.net Install the spellcheck module with composer, using `composer require silverstripe/spellcheck ^2.0`, or downloading the module and extracting to the 'spellcheck' directory under your project root. +## Requirements + +* SilverStripe 4.0.2 or above +* Hunspell + **Note:** this version is compatible with SilverStripe 4. For SilverStripe 3, please see [the 1.x release line](https://github.com/silverstripe/silverstripe-spellcheck/tree/1.0). ## Configuration diff --git a/src/Handling/SpellCheckMiddleware.php b/src/Handling/SpellCheckMiddleware.php index 48b1c20..00532c9 100644 --- a/src/Handling/SpellCheckMiddleware.php +++ b/src/Handling/SpellCheckMiddleware.php @@ -2,6 +2,7 @@ namespace SilverStripe\SpellCheck\Handling; +use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\Middleware\HTTPMiddleware; use SilverStripe\Core\Config\Configurable; @@ -28,12 +29,13 @@ public function process(HTTPRequest $request, callable $delegate) HTMLEditorConfig::get($editor)->enablePlugins('spellchecker'); HTMLEditorConfig::get($editor)->addButtonsToLine(2, 'spellchecker'); $token = SecurityToken::inst(); - HTMLEditorConfig::get($editor)->setOption('spellchecker_rpc_url', $token->addToUrl('spellcheck/')); - HTMLEditorConfig::get($editor)->setOption('browser_spellcheck', false); - HTMLEditorConfig::get($editor)->setOption( - 'spellchecker_languages', - '+'.implode(', ', $this->getLanguages()) - ); + HTMLEditorConfig::get($editor) + ->setOption('spellchecker_rpc_url', Director::absoluteURL($token->addToUrl('spellcheck/'))) + ->setOption('browser_spellcheck', false) + ->setOption( + 'spellchecker_languages', + implode(',', $this->getLanguages()) + ); return $delegate($request); } @@ -41,7 +43,7 @@ public function process(HTTPRequest $request, callable $delegate) /** * Check languages to set * - * @return array + * @return string[] */ public function getLanguages() { diff --git a/src/Handling/SpellController.php b/src/Handling/SpellController.php index 3ca5d79..6e02b55 100644 --- a/src/Handling/SpellController.php +++ b/src/Handling/SpellController.php @@ -99,26 +99,20 @@ public function setProvider(SpellProvider $provider) /** * Parse the output response * - * @param string $id Request ID * @param array|null $result Result data * @param array|null $error Error data * @param int $code HTTP Response code */ - protected function result($id, $result, $error = null, $code = 200) + protected function result($result, $code = 200) { $this->response->setStatusCode($code); - $this->response->setBody(json_encode(array( - 'id' => $id ? preg_replace('/\W/', '', $id) : null, // Cleanup id - 'result' => $result, - 'error' => $error - ))); + $this->response->setBody(json_encode($result)); return $this->response; } protected function success($result) { - $data = $this->getRequestData(); - return $this->result($data['id'], $result); + return $this->result($result); } /** @@ -129,14 +123,11 @@ protected function success($result) */ protected function error($message, $code) { - $error = array( - 'errstr' => $message, - 'errfile' => '', - 'errline' => null, - 'errcontext' => '', - 'level' => 'FATAL' - ); - return $this->result(null, null, $error, $code); + $error = [ + 'error' => $message, + ]; + + return $this->result($error, $code); } public function index() @@ -169,13 +160,16 @@ public function index() } // Check params and request type - if (!Director::is_ajax() || empty($data['method']) || empty($data['params']) || count($data['params']) < 2) { + if (!Director::is_ajax() || empty($data['method']) || empty($data['lang'])) { return $this->error(_t(__CLASS__ . '.InvalidRequest', 'Invalid request'), 400); } // Check locale - $params = $data['params']; - $locale = $params[0]; + $locale = $data['lang']; + + // Check if the locale is actually a language + $locale = i18n::getData()->localeFromLang($locale); + if (!in_array($locale, self::get_locales())) { return $this->error(_t(__CLASS__ . '.InvalidLocale', 'Not supported locale'), 400); } @@ -189,12 +183,10 @@ public function index() // Perform action try { $method = $data['method']; - $words = $params[1]; + $words = explode(' ', $data['text']); switch ($method) { - case 'checkWords': - return $this->success($provider->checkWords($locale, $words)); - case 'getSuggestions': - return $this->success($provider->getSuggestions($locale, $words)); + case 'spellcheck': + return $this->success($this->assembleData($locale, $words)); default: return $this->error( _t( @@ -210,6 +202,29 @@ public function index() } } + /** + * Assemble an output data structure that is expected for TinyMCE 4 + * + * @see https://www.tinymce.com/docs/plugins/spellchecker/#spellcheckerresponseformat + * + * @param string $locale + * @param string[] $words + * @return array + */ + protected function assembleData($locale, $words) + { + $result = [ + 'words' => [], + ]; + + $misspelledWords = $this->getProvider()->checkWords($locale, $words); + foreach ($misspelledWords as $word) { + $result['words'][$word] = $this->getProvider()->getSuggestions($locale, $word); + } + + return $result; + } + /** * Ensures the response has the correct headers */ @@ -234,9 +249,7 @@ protected function getRequestData() // Check if data needs to be parsed if ($this->data === null) { // Parse data from input - $result = $this->request->requestVar('json_data') - ?: file_get_contents("php://input"); - $this->data = $result ? json_decode($result, true) : array(); + $this->data = $this->request->postVars(); } return $this->data; } diff --git a/tests/SpellControllerTest.php b/tests/SpellControllerTest.php index de0f9f7..1b45cc2 100644 --- a/tests/SpellControllerTest.php +++ b/tests/SpellControllerTest.php @@ -71,7 +71,7 @@ public function testSecurityID() $response = $this->get('spellcheck', Injector::inst()->create(Session::class, $session)); $this->assertEquals(400, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); - $this->assertEquals($tokenError, $jsonBody->error->errstr); + $this->assertEquals($tokenError, $jsonBody->error); // Test request with correct token (will fail with an unrelated error) $response = $this->get( @@ -79,13 +79,13 @@ public function testSecurityID() Injector::inst()->create(Session::class, $session) ); $jsonBody = json_decode($response->getBody()); - $this->assertNotEquals($tokenError, $jsonBody->error->errstr); + $this->assertNotEquals($tokenError, $jsonBody->error); // Test request with check disabled Config::modify()->set(SpellController::class, 'enable_security_token', false); $response = $this->get('spellcheck', Injector::inst()->create(Session::class, $session)); $jsonBody = json_decode($response->getBody()); - $this->assertNotEquals($tokenError, $jsonBody->error->errstr); + $this->assertNotEquals($tokenError, $jsonBody->error); } /** @@ -102,20 +102,20 @@ public function testPermissions() $this->logInWithPermission('ADMIN'); $response = $this->get('spellcheck'); $jsonBody = json_decode($response->getBody()); - $this->assertNotEquals($securityError, $jsonBody->error->errstr); + $this->assertNotEquals($securityError, $jsonBody->error); // Test insufficient permissions $this->logInWithPermission('CMS_ACCESS_CMSMain'); $response = $this->get('spellcheck'); $this->assertEquals(403, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); - $this->assertEquals($securityError, $jsonBody->error->errstr); + $this->assertEquals($securityError, $jsonBody->error); // Test disabled permissions Config::modify()->set(SpellController::class, 'required_permission', false); $response = $this->get('spellcheck'); $jsonBody = json_decode($response->getBody()); - $this->assertNotEquals($securityError, $jsonBody->error->errstr); + $this->assertNotEquals($securityError, $jsonBody->error); } /** @@ -128,47 +128,29 @@ public function testInputRejection() Config::modify()->set(SpellController::class, 'required_permission', false); $invalidRequest = _t('SilverStripe\\SpellCheck\\Handling\\SpellController.InvalidRequest', 'Invalid request'); - // Test checkWords acceptance - $dataCheckWords = array( - 'id' => 'c0', - 'method' => 'checkWords', - 'params' => array( - 'en_NZ', - array('collor', 'colour', 'color', 'onee', 'correct') - ) - ); - $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataCheckWords))); - $this->assertEquals(200, $response->getStatusCode()); - $jsonBody = json_decode($response->getBody()); - $this->assertEquals('c0', $jsonBody->id); - $this->assertEquals(array("collor", "color", "onee"), $jsonBody->result); - - // Test getSuggestions acceptance - $dataGetSuggestions = array( - 'id' => '//c1//', // Should be reduced to only alphanumeric characters - 'method' => 'getSuggestions', - 'params' => array( - 'en_NZ', - 'collor' - - ) - ); - $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataGetSuggestions))); + // Test spellcheck acceptance + $mockData = [ + 'method' => 'spellcheck', + 'lang' => 'en_NZ', + 'text' => 'Collor is everywhere', + ]; + $response = $this->post('spellcheck', ['ajax' => true] + $mockData); $this->assertEquals(200, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); - $this->assertEquals('c1', $jsonBody->id); - $this->assertEquals(array('collar', 'colour'), $jsonBody->result); + $this->assertNotEmpty($jsonBody->words); + $this->assertNotEmpty($jsonBody->words->collor); + $this->assertEquals(['collar', 'colour'], $jsonBody->words->collor); // Test non-ajax rejection - $response = $this->post('spellcheck', array('json_data' => json_encode($dataCheckWords))); + $response = $this->post('spellcheck', $mockData); $this->assertEquals(400, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); - $this->assertEquals($invalidRequest, $jsonBody->error->errstr); + $this->assertEquals($invalidRequest, $jsonBody->error); // Test incorrect method - $dataInvalidMethod = $dataCheckWords; + $dataInvalidMethod = $mockData; $dataInvalidMethod['method'] = 'validate'; - $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataInvalidMethod))); + $response = $this->post('spellcheck', ['ajax' => true] + $dataInvalidMethod); $this->assertEquals(400, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); $this->assertEquals( @@ -177,29 +159,27 @@ public function testInputRejection() "Unsupported method '{method}'", array('method' => 'validate') ), - $jsonBody->error->errstr + $jsonBody->error ); // Test missing method - $dataNoMethod = $dataCheckWords; + $dataNoMethod = $mockData; unset($dataNoMethod['method']); - $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataNoMethod))); + $response = $this->post('spellcheck', ['ajax' => true] + $dataNoMethod); $this->assertEquals(400, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); - $this->assertEquals($invalidRequest, $jsonBody->error->errstr); + $this->assertEquals($invalidRequest, $jsonBody->error); // Test unsupported locale - $dataWrongLocale = $dataCheckWords; - $dataWrongLocale['params'] = array( - 'de_DE', - array('collor', 'colour', 'color', 'onee', 'correct') - ); - $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataWrongLocale))); + $dataWrongLocale = $mockData; + $dataWrongLocale['lang'] = 'de_DE'; + + $response = $this->post('spellcheck', ['ajax' => true] + $dataWrongLocale); $this->assertEquals(400, $response->getStatusCode()); $jsonBody = json_decode($response->getBody()); $this->assertEquals(_t( 'SilverStripe\\SpellCheck\\Handling\\.InvalidLocale', 'Not supported locale' - ), $jsonBody->error->errstr); + ), $jsonBody->error); } } From dc94796d745a4dd6887b5f28f08992418ba6c17a Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Thu, 14 Dec 2017 11:10:30 +1300 Subject: [PATCH 2/2] FIX Check if locale is actually a language before getting the locale from it --- src/Handling/SpellController.php | 30 +++++++++++++++---- tests/SpellControllerTest.php | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/Handling/SpellController.php b/src/Handling/SpellController.php index 6e02b55..8c2a4c2 100644 --- a/src/Handling/SpellController.php +++ b/src/Handling/SpellController.php @@ -165,12 +165,8 @@ public function index() } // Check locale - $locale = $data['lang']; - - // Check if the locale is actually a language - $locale = i18n::getData()->localeFromLang($locale); - - if (!in_array($locale, self::get_locales())) { + $locale = $this->getLocale($data); + if (!$locale) { return $this->error(_t(__CLASS__ . '.InvalidLocale', 'Not supported locale'), 400); } @@ -253,4 +249,26 @@ protected function getRequestData() } return $this->data; } + + /** + * Get the locale from the provided "lang" argument, which could be either a language code or locale + * + * @param array $data + * @return string|false + */ + protected function getLocale(array $data) + { + $locale = $data['lang']; + + // Check if the locale is actually a language + if (strpos($locale, '_') === false) { + $locale = i18n::getData()->localeFromLang($locale); + } + + if (!in_array($locale, self::get_locales())) { + return false; + } + + return $locale; + } } diff --git a/tests/SpellControllerTest.php b/tests/SpellControllerTest.php index 1b45cc2..ade59a4 100644 --- a/tests/SpellControllerTest.php +++ b/tests/SpellControllerTest.php @@ -118,6 +118,55 @@ public function testPermissions() $this->assertNotEquals($securityError, $jsonBody->error); } + /** + * @param string $lang + * @param int $expectedStatusCode + * @dataProvider langProvider + */ + public function testBothLangAndLocaleInputResolveToLocale($lang, $expectedStatusCode) + { + $this->logInWithPermission('ADMIN'); + Config::modify()->set(SpellController::class, 'enable_security_token', false); + + $mockData = [ + 'ajax' => true, + 'method' => 'spellcheck', + 'lang' => $lang, + 'text' => 'Collor is everywhere', + ]; + $response = $this->post('spellcheck', $mockData); + $this->assertEquals($expectedStatusCode, $response->getStatusCode()); + } + + /** + * @return array[] + */ + public function langProvider() + { + return [ + 'english_language' => [ + 'en', // assumes en_US is the default locale for "en" language + 200, + ], + 'english_locale' => [ + 'en_NZ', + 200, + ], + 'invalid_language' => [ + 'ru', + 400, + ], + 'other_valid_language' => [ + 'fr', // assumes fr_FR is the default locale for "en" language + 200, + ], + 'other_valid_locale' => [ + 'fr_FR', + 200, + ], + ]; + } + /** * Ensure that invalid input is correctly rejected */