Properly implemented parseUserLanguagePreferences()

src/server/i18n.cpp

@@ -123,25 +123,59 @@ std::string ParameterizedMessage::getText(const std::string& lang) const
   return i18n::expandParameterizedString(lang, msgId, params);
 }
 
+namespace
+{
+
+LangPreference parseSingleLanguagePreference(const std::string& s)
+{
+  const size_t langStart = s.find_first_not_of(" \t\n");
+  if ( langStart == std::string::npos ) {
+    return {"", 0};
+  }
+
+  const size_t langEnd = s.find(';', langStart);
+  if ( langEnd == std::string::npos ) {
+    return {s.substr(langStart), 1};
+  }
+
+  const std::string lang = s.substr(langStart, langEnd - langStart);
+  // We don't care about langEnd == langStart which will result in an empty
+  // language name - it will be dismissed by parseUserLanguagePreferences()
+
+  float q = 1.0;
+  int nCharsScanned;
+  if ( 1 == sscanf(s.c_str() + langEnd + 1, "q=%f%n", &q, &nCharsScanned)
+       && langEnd + 1 + nCharsScanned == s.size() ) {
+    return {lang, q};
+  }
+
+  return {"", 0};
+}
+
+} // unnamed namespace
+
 UserLangPreferences parseUserLanguagePreferences(const std::string& s)
 {
-  // TODO: implement properly
-  const UserLangPreferences defaultPref{{"en", 1}};
-
-  if ( s.empty() )
-    return defaultPref;
-
-  for ( const char c :  s ) {
-    if ( ! std::isalpha(c) ) {
-      return defaultPref;
+  UserLangPreferences result;
+  std::istringstream iss(s);
+  std::string singleLangPrefStr;
+  while ( std::getline(iss, singleLangPrefStr, ',') )
+  {
+    const auto langPref = parseSingleLanguagePreference(singleLangPrefStr);
+    if ( !langPref.lang.empty() && langPref.preference > 0 ) {
+      result.push_back(langPref);
     }
   }
 
-  return {{s, 1}};
+  return result;
 }
 
 std::string selectMostSuitableLanguage(const UserLangPreferences& prefs)
 {
+  if ( prefs.empty() ) {
+    return "en";
+  }
+
   std::string bestLangSoFar("en");
   float bestScoreSoFar = 0;
   const auto& stringDb = getStringDb();

test/otherTools.cpp

@@ -20,6 +20,7 @@
 #include "gtest/gtest.h"
 #include "../src/tools/otherTools.h"
 #include "zim/suggestion_iterator.h"
+#include "../src/server/i18n.h"
 
 #include <regex>
 
@@ -172,3 +173,63 @@ R"EXPECTEDJSON([
 )EXPECTEDJSON"
   );
 }
+
+std::string toString(const kiwix::LangPreference& x)
+{
+  std::ostringstream oss;
+  oss << "{" << x.lang << ", " << x.preference << "}";
+  return oss.str();
+}
+
+std::string toString(const kiwix::UserLangPreferences& prefs) {
+  std::ostringstream oss;
+  for ( const auto& x : prefs )
+    oss << toString(x);
+  return oss.str();
+}
+
+TEST(I18n, parseUserLanguagePreferences)
+{
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("")),
+      ""
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("*")),
+      "{*, 1}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("fr")),
+      "{fr, 1}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("fr-CH")),
+      "{fr-CH, 1}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("fr, en-US")),
+      "{fr, 1}{en-US, 1}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;q=0.5")),
+      "{ru, 0.5}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("fr-CH,ru;q=0.5")),
+      "{fr-CH, 1}{ru, 0.5}"
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;q=0.5, *;q=0.1")),
+      "{ru, 0.5}{*, 0.1}"
+  );
+
+  // rejected input
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;")),
+      ""
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;q")),
+      ""
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;q=")),
+      ""
+  );
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("ru;0.8")),
+      ""
+  );
+
+  EXPECT_EQ(toString(kiwix::parseUserLanguagePreferences("fr,ru;0.8,en;q=0.5")),
+      "{fr, 1}{en, 0.5}"
+  );
+}

test/server.cpp

@@ -1113,8 +1113,8 @@ TEST_F(ServerTest, UserLanguageControl)
       /*url*/ "/ROOT/content/zimfile/invalid-article",
       /*Accept-Language:*/ "test;q=0.9, en;q=0.2",
       /*Request Cookie:*/       NO_COOKIE,
-      /*Response Set-Cookie:*/  "userlang=en",
-      /* expected <h1> */ "Not Found"
+      /*Response Set-Cookie:*/  "userlang=test",
+      /* expected <h1> */ "[I18N TESTING] Content not found, but at least the server is alive"
     },
   };