Add utility function to determine if a locale is valid syntax; this will...

chrome/common/extensions/extension_l10n_util.cc

Index: chrome/common/extensions/extension_l10n_util.cc
===================================================================
--- chrome/common/extensions/extension_l10n_util.cc	(revision 67716)
+++ chrome/common/extensions/extension_l10n_util.cc	(working copy)
@@ -240,6 +240,51 @@
 
   return true;
 }
+
+bool IsValidLocaleSyntax(const std::string& locale) {
+  // This implements a simple approximation of RFC 5646; it will accept all

[jungshik at Google, 2010/12/10 18:32:17]

nit: BCP 47 (Best Current Practice 47 :
http://www.rfc-editor.org/rfc/bcp/bcp47.txt ) is a more widely used reference
(and easier to remember :-)). A bit more importantly, more 'stable' name than
RFC 5646, I guess. 

+  // valid strings and reject (but not all) invalid ones.
+
+  // Check that the length is plausible.
+  if (locale.size() < 2 || locale.size() >= ULOC_FULLNAME_CAPACITY)
+    return false;
+
+  // Check that all characters are alphanumeric, hyphen, or underscore.

[Nebojša Ćirić, 2010/12/10 17:19:13]

What about @ and = signs in de-DE@collation=phonebook? Locale validation is
tough, given so many standards in use (BCP 47, TR#35...).

+  for (size_t i = 0; i < locale.size(); i++) {
+    char ch = locale[i];
+    if (!isalnum(ch) && ch != '-' && ch != '_')

[jungshik at Google, 2010/12/10 18:32:17]

isalnum is locale-dependent (C-library locale) and shouldn't be used.
string_util may have something already. If not, you have to add one to check for
a-z, A-Z and 0-9.

As for "@key=value" syntax,  it's obsolete according to UTR 35
(http://www.unicode.org/reports/tr35/#Unicode_Language_and_Locale_Identifiers :
section 3.4). For the immediate need of ours, I guess we don't need to be
backward compatible with old syntax.

[Nebojša Ćirić, 2010/12/10 18:38:50]

I think IsAsciiAlpha && IsAsciiDigit would work here. Look at
trunk/src/chrome/common/extensions/extension_message_bundle.cc, IsValidName for
example use.

On 2010/12/10 18:32:17, Jungshik Shin wrote:

+      return false;
+  }
+
+  // Check that the initial token before the first hyphen or underscore is
+  // 1, 2, or 3 characters, and all others are 1 - 8 characters.
+  // (Tokenize/StringTokenizer don't work here, they collapse multiple
+  // delimiters into one.)
+  int token_len = 0;
+  int token_index = 0;
+  for (size_t i = 0; i < locale.size(); i++) {
+    char ch = locale[i];
+    if (ch == '-' || ch == '_') {
+      if (token_index == 0 && (token_len < 1 || token_len > 3)) {

[jungshik at Google, 2010/12/10 18:32:17]

can you tighten up a bit here? although BCP 47 reserves a single digit tag for
extension (section 3.7), I don't think any has been registered/used. So, the
first tag can be assumed to be in [a-zA-Z]

+        return false;
+      } else if (token_len < 1 || token_len > 8) {
+        return false;
+      }
+      token_index++;
+      token_len = 0;
+    } else {
+      token_len++;
+    }
+  }
+  if (token_index == 0 && (token_len < 1 || token_len > 3)) {
+    return false;
+  } else if (token_len < 1 || token_len > 8) {
+    return false;
+  }
+
+  return true;
+}

[Nebojša Ćirić, 2010/12/10 17:19:13]

I think app/l10n_util would be a better place for this function (and
GetParentLocales) - since they are not related to extension implementation
details.

[jungshik at Google, 2010/12/10 18:32:17]

I agree

+
 // Loads contents of the messages file for given locale. If file is not found,
 // or there was parsing error we return NULL and set |error|.
 // Caller owns the returned object.