From 43abcd93867996e32890fa101ae09255ccc24847 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 1 Jun 2009 22:40:22 -0700 Subject: Security pass over all controller code. Mostly adding CSRF checking and verifying user permissions, but there are several above-the-bar changes: 1) Server add is now only available to admins. This is a hard requirement because we have to limit server access (eg: server_add::children) to a user subset and the current permission model doesn't include that. Easiest fix is to restrict to admins. Got rid of the server_add permission. 2) We now know check permissions at every level, which means in controllers AND in helpers. This "belt and suspenders" approach will give us defense in depth in case we overlook it in one area. 3) We now do CSRF checking in every controller method that changes the code, in addition to the Forge auto-check. Again, defense in depth and it makes scanning the code for security much simpler. 4) Moved Simple_Uploader_Controller::convert_filename_to_title to item:convert_filename_to_title 5) Fixed a bug in sending notification emails. 6) Fixed the Organize code to verify that you only have access to your own tasks. In general, added permission checks to organize which had pretty much no validation code. I did my best to verify every feature that I touched. --- modules/gallery/controllers/l10n_client.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'modules/gallery/controllers/l10n_client.php') diff --git a/modules/gallery/controllers/l10n_client.php b/modules/gallery/controllers/l10n_client.php index 17520051..c3a76659 100644 --- a/modules/gallery/controllers/l10n_client.php +++ b/modules/gallery/controllers/l10n_client.php @@ -20,7 +20,9 @@ class L10n_Client_Controller extends Controller { public function save() { access::verify_csrf(); - user::active()->admin or access::forbidden(); + if (!user::active()->admin) { + access::forbidden(); + } $input = Input::instance(); $message = $input->post("l10n-message-source"); @@ -58,6 +60,9 @@ class L10n_Client_Controller extends Controller { public function toggle_l10n_mode() { access::verify_csrf(); + if (!user::active()->admin) { + access::forbidden(); + } $session = Session::instance(); $session->set("l10n_mode", @@ -89,6 +94,10 @@ class L10n_Client_Controller extends Controller { } public static function l10n_form() { + if (!user::active()->admin) { + access::forbidden(); + } + $calls = I18n::instance()->call_log(); if ($calls) { -- cgit v1.2.3 From 1cfed1fac1f2aa109e96c2aa5c9f66002610b0f8 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 2 Jun 2009 00:43:04 -0700 Subject: Extend L10n client to provide UI for plural translation. Ticket 148. --- modules/gallery/controllers/l10n_client.php | 102 ++++++++++++++++------------ modules/gallery/css/l10n_client.css | 14 ++-- modules/gallery/helpers/l10n_client.php | 76 +++++++++++++++++++++ modules/gallery/js/l10n_client.js | 100 +++++++++++++++++++++------ modules/gallery/libraries/I18n.php | 27 +++++--- modules/gallery/views/l10n_client.html.php | 41 ++++++++++- 6 files changed, 275 insertions(+), 85 deletions(-) (limited to 'modules/gallery/controllers/l10n_client.php') diff --git a/modules/gallery/controllers/l10n_client.php b/modules/gallery/controllers/l10n_client.php index c3a76659..aa93a758 100644 --- a/modules/gallery/controllers/l10n_client.php +++ b/modules/gallery/controllers/l10n_client.php @@ -24,11 +24,36 @@ class L10n_Client_Controller extends Controller { access::forbidden(); } - $input = Input::instance(); - $message = $input->post("l10n-message-source"); - $translation = $input->post("l10n-edit-target"); - $key = I18n::get_message_key($message); $locale = I18n::instance()->locale(); + $input = Input::instance(); + $key = $input->post("l10n-message-key"); + + $root_message = ORM::factory("incoming_translation") + ->where(array("key" => $key, + "locale" => "root")) + ->find(); + + if (!$root_message->loaded) { + throw new Exception("@todo bad request data / illegal state"); + } + $is_plural = I18n::is_plural_message(unserialize($root_message->message)); + + if ($is_plural) { + $plural_forms = l10n_client::plural_forms($locale); + $translation = array(); + foreach($plural_forms as $plural_form) { + $value = $input->post("l10n-edit-plural-translation-$plural_form"); + if (null === $value || !is_string($value)) { + throw new Exception("@todo bad request data"); + } + $translation[$plural_form] = $value; + } + } else { + $translation = $input->post("l10n-edit-translation"); + if (null === $translation || !is_string($translation)) { + throw new Exception("@todo bad request data"); + } + } $entry = ORM::factory("outgoing_translation") ->where(array("key" => $key, @@ -38,7 +63,7 @@ class L10n_Client_Controller extends Controller { if (!$entry->loaded) { $entry->key = $key; $entry->locale = $locale; - $entry->message = serialize($message); + $entry->message = $root_message->message; $entry->base_revision = null; } @@ -71,19 +96,6 @@ class L10n_Client_Controller extends Controller { url::redirect("albums/1"); } - private static function _l10n_client_form() { - $form = new Forge("l10n_client/save", "", "post", array("id" => "gL10nClientSaveForm")); - $group = $form->group("l10n_message"); - $group->hidden("l10n-message-source")->value(""); - $group->textarea("l10n-edit-target"); - $group->submit("l10n-edit-save")->value(t("Save translation")); - // TODO(andy_st): Avoiding multiple submit buttons for now (hassle with jQuery form plugin). - // $group->submit("l10n-edit-copy")->value(t("Copy source")); - // $group->submit("l10n-edit-clear")->value(t("Clear")); - - return $form; - } - private static function _l10n_client_search_form() { $form = new Forge("l10n_client/search", "", "post", array("id" => "gL10nSearchForm")); $group = $form->group("l10n_search"); @@ -94,41 +106,47 @@ class L10n_Client_Controller extends Controller { } public static function l10n_form() { - if (!user::active()->admin) { - access::forbidden(); - } - $calls = I18n::instance()->call_log(); + $locale = I18n::instance()->locale(); if ($calls) { + $translations = array(); + foreach (Database::instance() + ->select("key", "translation") + ->from("incoming_translations") + ->where(array("locale" => $locale)) + ->get() + ->as_array() as $row) { + $translations[$row->key] = unserialize($row->translation); + } + // Override incoming with outgoing... + foreach (Database::instance() + ->select("key", "translation") + ->from("outgoing_translations") + ->where(array("locale" => $locale)) + ->get() + ->as_array() as $row) { + $translations[$row->key] = unserialize($row->translation); + } + $string_list = array(); - foreach ($calls as $call) { + $cache = array(); + foreach ($calls as $key => $call) { list ($message, $options) = $call; - // Note: Don't interpolate placeholders for the actual translation input field. - // TODO: Use $options to generate a preview. - if (is_array($message)) { - // TODO: Handle plural forms. - // Translate each message. If it has a plural form, get - // the current locale's plural rules and all plural translations. - continue; - } - $source = $message; - $translation = ''; - $options_for_raw_translation = array(); - if (isset($options['count'])) { - $options_for_raw_translation['count'] = $options['count']; - } - if (I18n::instance()->has_translation($message, $options_for_raw_translation)) { - $translation = I18n::instance()->translate($message, $options_for_raw_translation); - } - $string_list[] = array('source' => $source, + // Ensure that the message is in the DB + l10n_scanner::process_message($message, $cache); + // Note: Not interpolating placeholders for the actual translation input field. + // TODO: Might show a preview w/ interpolations (using $options) + $translation = isset($translations[$key]) ? $translations[$key] : ''; + $string_list[] = array('source' => $message, + 'key' => $key, 'translation' => $translation); } $v = new View('l10n_client.html'); $v->string_list = $string_list; - $v->l10n_form = self::_l10n_client_form(); $v->l10n_search_form = self::_l10n_client_search_form(); + $v->plural_forms = l10n_client::plural_forms($locale); return $v; } diff --git a/modules/gallery/css/l10n_client.css b/modules/gallery/css/l10n_client.css index 8973715f..6616f511 100644 --- a/modules/gallery/css/l10n_client.css +++ b/modules/gallery/css/l10n_client.css @@ -145,7 +145,6 @@ how it wants to round. */ margin:0em; } - #l10n-client-string-editor { display:none; float:left; @@ -168,18 +167,13 @@ how it wants to round. */ #gL10nClientSaveForm { padding:0em;} - #gL10nClientSaveForm .form-textarea { - height:13em; - font-size:1em; line-height:1.25em; - width:95%;} - - #gL10nClientSaveForm .form-submit { - margin-top: 0em;} - - #l10n-client form ul, #l10n-client form li, #l10n-client form input[type=submit], #l10n-client form input[type=text] { display: inline ! important ; } + +#l10n-client form .hidden { + display: none; +} diff --git a/modules/gallery/helpers/l10n_client.php b/modules/gallery/helpers/l10n_client.php index d26739f5..4e905c6c 100644 --- a/modules/gallery/helpers/l10n_client.php +++ b/modules/gallery/helpers/l10n_client.php @@ -200,4 +200,80 @@ class l10n_client_Core { // @todo Move messages out of outgoing into incoming, using new rev? // @todo show which messages have been rejected / are pending? } + + /** + * Plural forms. + */ + static function plural_forms($locale) { + $parts = explode('_', $locale); + $language = $parts[0]; + + // Data from CLDR 1.6 (http://unicode.org/cldr/data/common/supplemental/plurals.xml). + // Docs: http://www.unicode.org/cldr/data/charts/supplemental/language_plural_rules.html + switch ($language) { + case 'az': + case 'fa': + case 'hu': + case 'ja': + case 'ko': + case 'my': + case 'to': + case 'tr': + case 'vi': + case 'yo': + case 'zh': + case 'bo': + case 'dz': + case 'id': + case 'jv': + case 'ka': + case 'km': + case 'kn': + case 'ms': + case 'th': + return array('other'); + + case 'ar': + return array('zero', 'one', 'two', 'few', 'many', 'other'); + + case 'lv': + return array('zero', 'one', 'other'); + + case 'ga': + case 'se': + case 'sma': + case 'smi': + case 'smj': + case 'smn': + case 'sms': + return array('one', 'two', 'other'); + + case 'ro': + case 'mo': + case 'lt': + case 'cs': + case 'sk': + case 'pl': + return array('one', 'few', 'other'); + + case 'hr': + case 'ru': + case 'sr': + case 'uk': + case 'be': + case 'bs': + case 'sh': + case 'mt': + return array('one', 'few', 'many', 'other'); + + case 'sl': + return array('one', 'two', 'few', 'other'); + + case 'cy': + return array('one', 'two', 'many', 'other'); + + default: // en, de, etc. + return array('one', 'other'); + } + } } \ No newline at end of file diff --git a/modules/gallery/js/l10n_client.js b/modules/gallery/js/l10n_client.js index f43671f1..efd956e2 100644 --- a/modules/gallery/js/l10n_client.js +++ b/modules/gallery/js/l10n_client.js @@ -90,6 +90,40 @@ jQuery.extend(Gallery, { this.setString = function(index, data) { l10n_client_data[index]['translation'] = data; } + // Display the source message + this.showSourceMessage = function(source, is_plural) { + if (is_plural) { + var pretty_source = '[one] - ' + source['one'] + "\n"; + pretty_source += '[other] - ' + source['other']; + } else { + var pretty_source = source; + } + $('#l10n-client-string-editor .source-text').text(pretty_source); + } + this.isPluralMessage = function(message) { + return typeof(message) == 'object'; + } + this.updateTranslationForm = function(translation, is_plural) { + $('.translationField').addClass('hidden'); + if (is_plural) { + if (typeof(translation) != 'object') { + translation = {}; + } + var num_plural_forms = plural_forms.length; + for (var i = 0; i < num_plural_forms; i++) { + var form = plural_forms[i]; + if (translation[form] == undefined) { + translation[form] = ''; + } + $('#l10n-edit-plural-translation-' + form) + .attr('value', translation[form]); + $('#plural-' + form).removeClass('hidden'); + } + } else { + $('#l10n-edit-translation').attr('value', translation); + $('#l10n-edit-translation').removeClass('hidden'); + } + } // Filter the the string list by a search string this.filter = function(search) { if(search == false || search == '') { @@ -126,11 +160,12 @@ Gallery.behaviors.l10nClient = function(context) { $('#l10n-client-string-select li').removeClass('active'); $(this).addClass('active'); var index = $('#l10n-client-string-select li').index(this); - - $('#l10n-client-string-editor .source-text').text(Gallery.l10nClient.getString(index, 'source')); - $("#gL10nClientSaveForm input[name='l10n-message-source']").val(Gallery.l10nClient.getString(index, 'source')); - $('#gL10nClientSaveForm #l10n-edit-target').val(Gallery.l10nClient.getString(index, 'translation')); - + var source = Gallery.l10nClient.getString(index, 'source'); + var key = Gallery.l10nClient.getString(index, 'key'); + var is_plural = Gallery.l10nClient.isPluralMessage(source); + Gallery.l10nClient.showSourceMessage(source, is_plural); + Gallery.l10nClient.updateTranslationForm(Gallery.l10nClient.getString(index, 'translation'), is_plural); + $("#gL10nClientSaveForm input[name='l10n-message-key']").val(key); Gallery.l10nClient.selected = index; }); @@ -165,23 +200,46 @@ Gallery.behaviors.l10nClient = function(context) { $('#gL10nClientSaveForm').ajaxForm({ dataType: "json", success: function(data) { - // Store string in local js - Gallery.l10nClient.setString(Gallery.l10nClient.selected, $('#gL10nClientSaveForm #l10n-edit-target').val()); - - // Mark string as translated. - $('#l10n-client-string-select li').eq(Gallery.l10nClient.selected).removeClass('untranslated').removeClass('active').addClass('translated').text($('#gL10nClientSaveForm #l10n-edit-target').val()); - - // Empty input fields. - $('#l10n-client-string-editor .source-text').html(''); - $('#gL10nClientSaveForm #l10n-edit-target').val(''); - $("#gL10nClientSaveForm input[name='l10n-message-source']").val(''); - }, - error: function(xmlhttp) { - // TODO: Localize this message - alert('An HTTP error @status occured (or empty response).'.replace('@status', xmlhttp.status)); - } - }); + var source = Gallery.l10nClient.getString(Gallery.l10nClient.selected, 'source'); + var is_plural = Gallery.l10nClient.isPluralMessage(source); + var num_plural_forms = plural_forms.length; + + // Store translation in local js + if (is_plural) { + var translation = {}; + for (var i = 0; i < num_plural_forms; i++) { + var form = plural_forms[i]; + translation[form] = $('#gL10nClientSaveForm #l10n-edit-plural-translation-' + form).attr('value'); + } + } else { + translation = $('#l10n-edit-translation').attr('value'); + } + Gallery.l10nClient.setString(Gallery.l10nClient.selected, translation); + + // Mark message as translated. + $('#l10n-client-string-select li').eq(Gallery.l10nClient.selected).removeClass('untranslated').removeClass('active').addClass('translated'); + + // Clear the translation form fields + Gallery.l10nClient.showSourceMessage('', false); + $('#gL10nClientSaveForm #l10n-edit-translation').val(''); + + for (var i = 0; i < num_plural_forms; i++) { + var form = plural_forms[i]; + $('#gL10nClientSaveForm #l10n-edit-plural-translation-' + form).val(''); + } + $("#gL10nClientSaveForm input[name='l10n-message-key']").val(''); + }, + error: function(xmlhttp) { + // TODO: Localize this message + alert('An HTTP error @status occured (or empty response).'.replace('@status', xmlhttp.status)); + } + }); + // TODO: Add copy/clear buttons (without ajax behavior) + /* "/> + "/> + */ + // TODO: Handle plurals in copy button // Copy source text to translation field on button click. $('#gL10nClientSaveForm #l10n-edit-copy').click(function() { diff --git a/modules/gallery/libraries/I18n.php b/modules/gallery/libraries/I18n.php index f2801169..03a6d8f6 100644 --- a/modules/gallery/libraries/I18n.php +++ b/modules/gallery/libraries/I18n.php @@ -148,30 +148,37 @@ class I18n_Core { public function has_translation($message, $options=null) { $locale = empty($options['locale']) ? $this->_config['default_locale'] : $options['locale']; - $count = empty($options['count']) ? null : $options['count']; - $values = $options; - unset($values['locale']); - $this->log($message, $options); $entry = $this->lookup($locale, $message); if (null === $entry) { return false; - } else if (!is_array($entry)) { + } else if (!is_array($message)) { return $entry !== ''; } else { - $plural_key = self::get_plural_key($locale, $count); - return isset($entry[$plural_key]) - && $entry[$plural_key] !== null - && $entry[$plural_key] !== ''; + if (!is_array($entry) || empty($entry)) { + return false; + } + // It would be better to verify that all the locale's plural forms have a non-empty + // translation, but this is fine for now. + foreach ($entry as $value) { + if ($value === '') { + return false; + } + } + return true; } } - public static function get_message_key($message) { + static function get_message_key($message) { $as_string = is_array($message) ? implode('|', $message) : $message; return md5($as_string); } + static function is_plural_message($message) { + return is_array($message); + } + private function interpolate($locale, $string, $values) { // TODO: Handle locale specific number formatting. diff --git a/modules/gallery/views/l10n_client.html.php b/modules/gallery/views/l10n_client.html.php index 8f4092c7..faa6e939 100644 --- a/modules/gallery/views/l10n_client.html.php +++ b/modules/gallery/views/l10n_client.html.php @@ -11,21 +11,58 @@ +
-
+ 

     

     

-      
+      
" id="gL10nClientSaveForm">
+        
+        
+        
+        
+        
+        
+        
+        
+        
+        "/>
+      

     

   

   
 
-- 
cgit v1.2.3