Index: includes/locale.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/locale.inc,v retrieving revision 1.195 diff -u -p -r1.195 locale.inc --- includes/locale.inc 16 Nov 2008 19:41:14 -0000 1.195 +++ includes/locale.inc 9 Dec 2008 14:57:35 -0000 @@ -673,7 +673,7 @@ function locale_translate_import_form_su } else { drupal_set_message(t('File to import not found.'), 'error'); - return 'admin/build/translate/import'; + $form_state['redirect'] = 'admin/build/translate/import'; } $form_state['redirect'] = 'admin/build/translate'; @@ -834,7 +834,31 @@ function locale_translate_edit_form(&$fo } /** + * Check that a string is safe to be added or imported as a translation. + * + * This test can be used to detect possibly bad translation strings. It should + * not have any false positives. But it is only a test, not a transformation, + * as it destroys valid HTML. + */ +function locale_string_is_safe($string) { + // Tag list like filter_xss_admin(), but omitting a few not needed for translation. + return decode_entities($string) == decode_entities(filter_xss($string, array('a', 'abbr', 'acronym', 'address', 'b', 'bdo', 'big', 'blockquote', 'br', 'caption', 'cite', 'code', 'col', 'colgroup', 'dd', 'del', 'dfn', 'dl', 'dt', 'em', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'i', 'ins', 'kbd', 'li', 'ol', 'p', 'pre', 'q', 'samp', 'small', 'span', 'strong', 'sub', 'sup', 'table', 'tbody', 'td', 'tfoot', 'th', 'thead', 'tr', 'tt', 'ul', 'var'))); +} + +/** + * Validate string editing form submissions. + */ +function locale_translate_edit_form_validate($form, &$form_state) { + foreach ($form_state['values']['translations'] as $key => $value) { + if (!locale_string_is_safe($value)) { + form_set_error('translations', t('The submitted string may contain unsafe HTML: %string', array('%string' => $value))); + } + } +} + +/** * Process string editing form submissions. + * * Saves all translations of one string submitted from a form. */ function locale_translate_edit_form_submit($form, &$form_state) { @@ -1012,7 +1036,7 @@ function _locale_import_po($file, $langc } // Get status information on import process. - list($headerdone, $additions, $updates, $deletes) = _locale_import_one_string('db-report'); + list($headerdone, $additions, $updates, $deletes, $skips) = _locale_import_one_string('db-report'); if (!$headerdone) { drupal_set_message(t('The translation file %filename appears to have a missing or malformed header.', array('%filename' => $file->filename)), 'error'); @@ -1026,6 +1050,7 @@ function _locale_import_po($file, $langc menu_rebuild(); drupal_set_message(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => $additions, '%update' => $updates, '%delete' => $deletes))); + drupal_set_message(t('In addtion, @skips strings were skipped because they are possibly unsafe.', array('@skips' => $skips))); watchdog('locale', 'Imported %file into %locale: %number new strings added, %update updated and %delete removed.', array('%file' => $file->filename, '%locale' => $langcode, '%number' => $additions, '%update' => $updates, '%delete' => $deletes)); return TRUE; } @@ -1216,7 +1241,7 @@ function _locale_import_message($message * Text group to import PO file into (eg. 'default' for interface translations) */ function _locale_import_one_string($op, $value = NULL, $mode = NULL, $lang = NULL, $file = NULL, $group = 'default') { - static $report = array(0, 0, 0); + static $report = array('additions' => 0, 'updates' => 0, 'deletes' => 0, 'skips' => 0); static $headerdone = FALSE; static $strings = array(); @@ -1232,7 +1257,7 @@ function _locale_import_one_string($op, // Called at end of import to inform the user case 'db-report': - return array($headerdone, $report[0], $report[1], $report[2]); + return array($headerdone, $report['additions'], $report['updates'], $report['deletes'], $report['skips']); // Store the string we got in the database. case 'db-store': @@ -1311,19 +1336,24 @@ function _locale_import_one_string_db(&$ $lid = db_result(db_query("SELECT lid FROM {locales_source} WHERE source = '%s' AND textgroup = '%s'", $source, $textgroup)); if (!empty($translation)) { - if ($lid) { + // Skip this string unless it passes a check for dangerous code. + if (!locale_string_is_safe($translation)) { + $report['skips']++; + $lid = 0; + } + elseif ($lid) { // We have this source string saved already. db_query("UPDATE {locales_source} SET location = '%s' WHERE lid = %d", $location, $lid); $exists = (bool) db_result(db_query("SELECT lid FROM {locales_target} WHERE lid = %d AND language = '%s'", $lid, $langcode)); if (!$exists) { // No translation in this language. db_query("INSERT INTO {locales_target} (lid, language, translation, plid, plural) VALUES (%d, '%s', '%s', %d, %d)", $lid, $langcode, $translation, $plid, $plural); - $report[0]++; + $report['additions']++; } elseif ($mode == LOCALE_IMPORT_OVERWRITE) { // Translation exists, only overwrite if instructed. db_query("UPDATE {locales_target} SET translation = '%s', plid = %d, plural = %d WHERE language = '%s' AND lid = %d", $translation, $plid, $plural, $langcode, $lid); - $report[1]++; + $report['updates']++; } } else { @@ -1331,13 +1361,13 @@ function _locale_import_one_string_db(&$ db_query("INSERT INTO {locales_source} (location, source, textgroup) VALUES ('%s', '%s', '%s')", $location, $source, $textgroup); $lid = db_result(db_query("SELECT lid FROM {locales_source} WHERE source = '%s' AND textgroup = '%s'", $source, $textgroup)); db_query("INSERT INTO {locales_target} (lid, language, translation, plid, plural) VALUES (%d, '%s', '%s', %d, %d)", $lid, $langcode, $translation, $plid, $plural); - $report[0]++; + $report['additions']++; } } elseif ($mode == LOCALE_IMPORT_OVERWRITE) { // Empty translation, remove existing if instructed. db_query("DELETE FROM {locales_target} WHERE language = '%s' AND lid = %d AND plid = %d AND plural = %d", $translation, $langcode, $lid, $plid, $plural); - $report[2]++; + $report['deletes']++; } return $lid; Index: modules/locale/locale.test =================================================================== RCS file: /cvs/drupal/drupal/modules/locale/locale.test,v retrieving revision 1.11 diff -u -p -r1.11 locale.test --- modules/locale/locale.test 29 Nov 2008 09:33:51 -0000 1.11 +++ modules/locale/locale.test 9 Dec 2008 14:57:35 -0000 @@ -4,8 +4,8 @@ class LocaleTestCase extends DrupalWebTestCase { function getInfo() { return array( - 'name' => t('String translate'), - 'description' => 'Adds a new locale and translates its name', + 'name' => t('String translate and validate'), + 'description' => 'Adds a new locale and translates its name. Checks the validation of translation strings.', 'group' => 'Locale', ); } @@ -110,6 +110,64 @@ class LocaleTestCase extends DrupalWebTe $this->drupalPost('admin/build/translate/search', $search, t('Search')); $this->assertNoText($name, 'Search now can not find the name'); } + + function testLocaleStringTest() { + global $base_url; + + // User to add language and strings + $admin_user = $this->drupalCreateUser(array('administer languages', 'access administration pages', 'translate interface')); + $this->drupalLogin($admin_user); + $langcode = str_replace('simpletest_', 'si-', $this->randomName(6)); + // The English name for the language. This will be translated. + $name = $this->randomName(16); + // The native name for the language. + $native = $this->randomName(16); + // The domain prefix. Not tested yet. + $prefix = strtolower(str_replace('si-', '', $langcode)); + // This is the language indicator on the translation search screen for + // untranslated strings. Copied straight from locale.inc. + $language_indicator = "$langcode "; + // These will be the invalid translations of $name. + $key = $this->randomName(16); + $bad_translations[$key] = "" . $key; + $key = $this->randomName(16); + $bad_translations[$key] = '' . $key; + $key = $this->randomName(16); + $bad_translations[$key] = '<' . $key; + $key = $this->randomName(16); + $bad_translations[$key] ="" . $key; + + // Add language. + $edit = array ( + 'langcode' => $langcode, + 'name' => $name, + 'native' => $native, + 'prefix' => $prefix, + 'direction' => '0', + ); + $this->drupalPost('admin/settings/language/add', $edit, t('Add custom language')); + // Add string. + t($name, array(), $langcode); + // Reset locale cache. + $search = array ( + 'string' => $name, + 'language' => 'all', + 'translation' => 'all', + 'group' => 'all', + ); + $this->drupalPost('admin/build/translate/search', $search, t('Search')); + // Find the edit path + $content = $this->drupalGetContent(); + $this->assertTrue(preg_match('@(admin/build/translate/edit/[0-9]+)@', $content, $matches), t('Found the edit path')); + $path = $matches[0]; + foreach ($bad_translations as $key => $translation) { + $edit = array ( + "translations[$langcode]" => $translation, + ); + $this->drupalPost($path, $edit, t('Save translations')); + $this->assertText(t('The submitted string may contain unsafe HTML'), t('The string was rejected as unsafe.')); + } + } } /** @@ -154,6 +212,19 @@ class LocaleImportFunctionalTest extends // The importation should have create 7 strings. $this->assertRaw(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => 7, '%update' => 0, '%delete' => 0)), t('The translation file was successfully imported')); + + // Try importing a .po file with script. + $name = tempnam(file_directory_temp(), "po_"); + file_put_contents($name, $this->getBadPoFile()); + $this->drupalPost('admin/build/translate/import', array( + 'langcode' => 'fr', + 'files[file]' => $name, + ), t('Import')); + unlink($name); + // The importation should have created 1 string and rejected 2. + $this->assertRaw(t('The translation was successfully imported. There are %number newly created translated strings, %update strings were updated and %delete strings were removed.', array('%number' => 1, '%update' => 0, '%delete' => 0)), t('The translation file was successfully imported.')); + $this->assertRaw(t('In addtion, @skips strings were skipped because they are possibly unsafe.', array('@skips' => 2)), t('Unsafe strings were skipped.')); + } /** @@ -191,4 +262,29 @@ msgid "Sunday" msgstr "dimanche" EOF; } + + /** + * Helper function that returns a proper .po file. + */ + function getBadPoFile() { + return <<< EOF +msgid "" +msgstr "" +"Project-Id-Version: Drupal 6\\n" +"MIME-Version: 1.0\\n" +"Content-Type: text/plain; charset=UTF-8\\n" +"Content-Transfer-Encoding: 8bit\\n" +"Plural-Forms: nplurals=2; plural=(n > 1);\\n" + +msgid "Save configuration" +msgstr "Enregistrer la configuration" + +msgid "edit" +msgstr "modifier" + +msgid "delete" +msgstr "supprimer" + +EOF; + } }