Merge pull request #2043 from rugk/agent/fix-accept-language-validation

fix: improve HTTP_ACCEPT_LANGUAGE language extraction

.styleci.yml

@@ -3,7 +3,6 @@ preset: recommended
 risky: false
 
 enabled:
-  - align_equals
   - concat_with_spaces
   - no_empty_comment
   - pre_increment
@@ -26,7 +25,6 @@ disabled:
   - phpdoc_summary
   - post_increment
   - single_line_after_imports
-  - unalign_equals
 
 finder:
   path:

lib/I18n.php

@@ -14,6 +14,9 @@ namespace PrivateBin;
 use AppendIterator;
 use GlobIterator;
 
+use function array_key_exists;
+use function in_array;
+
 /**
  * I18n
  *
@@ -116,8 +119,8 @@ class I18n
         array_unshift($args, $messageId);
         if (is_array(self::$_translations[$messageId])) {
             $number = (int) $args[1];
-            $key    = self::_getPluralForm($number);
-            $max    = count(self::$_translations[$messageId]) - 1;
+            $key = self::_getPluralForm($number);
+            $max = count(self::$_translations[$messageId]) - 1;
             if ($key > $max) {
                 $key = $max;
             }
@@ -176,23 +179,47 @@ class I18n
         }
         // find a translation file matching the browsers language preferences
         else {
-            self::$_language = self::_getMatchingLanguage(
-                self::_normalizeBrowserLanguages(
-                    self::getBrowserLanguages(), $availableLanguages
-                ),
-                $availableLanguages
-            );
+            self::$_language = self::_getLanguageFromHttpHeader($availableLanguages);
         }
 
         // load translations
         if (self::$_language === 'en') {
             self::$_translations = [];
         } else {
-            $data                = file_get_contents(self::_getPath(self::$_language . '.json'));
+            $data = file_get_contents(self::_getPath(self::$_language . '.json'));
             self::$_translations = Json::decode($data);
         }
     }
 
+    /**
+     * get languages bsed on the HTTP header
+     *
+     * @access private
+     * @static
+     * @param  array $availableLanguages
+     * @return string
+     */
+    private static function _getLanguageFromHttpHeader($availableLanguages)
+    {
+        if (!array_key_exists('HTTP_ACCEPT_LANGUAGE', $_SERVER)) {
+            return self::$_languageFallback;
+        }
+
+        // as per https://stackoverflow.com/a/9051957/5008962 we can use the I10n module
+        // if loaded, but it only returns the primary preference (aka one string)
+        if (function_exists('locale_accept_from_http')) {
+            $detectedBestLanguage = locale_accept_from_http($_SERVER['HTTP_ACCEPT_LANGUAGE']);
+            if (in_array($detectedBestLanguage, $availableLanguages)) {
+                return $detectedBestLanguage;
+            }
+        }
+
+        return self::_getMatchingLanguage(
+            self::_normalizeBrowserLanguages(self::getBrowserLanguages(), $availableLanguages),
+            $availableLanguages
+        );
+    }
+
     /**
      * get list of available translations based on files found
      *
@@ -204,7 +231,7 @@ class I18n
     {
         if (count(self::$_availableLanguages) === 0) {
             self::$_availableLanguages[] = 'en'; // en.json is not part of the release archive
-            $languageIterator            = new AppendIterator();
+            $languageIterator = new AppendIterator();
             $languageIterator->append(new GlobIterator(self::_getPath('??.json')));
             $languageIterator->append(new GlobIterator(self::_getPath('???.json'))); // for jbo
             $languageIterator->append(new GlobIterator(self::_getPath('??-??.json'))); // for regional variants like zh-tw
@@ -234,7 +261,7 @@ class I18n
             $languageRanges = explode(',', trim($_SERVER['HTTP_ACCEPT_LANGUAGE']));
             foreach ($languageRanges as $languageRange) {
                 if (preg_match(
-                    '/(\*|[a-zA-Z0-9]{1,8}(?:-[a-zA-Z0-9]{1,8})*)(?:\s*;\s*q\s*=\s*(0(?:\.\d{0,3})|1(?:\.0{0,3})))?/',
+                    '/^(\*|[a-zA-Z0-9]{1,8}(?:-[a-zA-Z0-9]{1,8})*)(?:\s*;\s*q\s*=\s*(0(?:\.\d{0,3})|1(?:\.0{0,3})))?$/',
                     trim($languageRange), $match
                 )) {
                     if (!isset($match[2])) {
@@ -265,7 +292,7 @@ class I18n
     protected static function _normalizeBrowserLanguages($languages, $availableLanguages)
     {
         // Base zh stays before zh-tw because regional locales are appended after base locales.
-        $hasSimplifiedChinese  = in_array('zh', $availableLanguages, true);
+        $hasSimplifiedChinese = in_array('zh', $availableLanguages, true);
         $hasTraditionalChinese = in_array('zh-tw', $availableLanguages, true);
         foreach ($languages as $quality => $languageRanges) {
             foreach ($languageRanges as $index => $languageRange) {
@@ -313,7 +340,7 @@ class I18n
     {
         $file = self::_getPath('languages.json');
         if (count(self::$_languageLabels) === 0 && is_readable($file)) {
-            $data                  = file_get_contents($file);
+            $data = file_get_contents($file);
             self::$_languageLabels = Json::decode($data);
         }
         if (count($languages) === 0) {
@@ -442,18 +469,18 @@ class I18n
     protected static function _getMatchingLanguage($acceptedLanguages, $availableLanguages)
     {
         $matches = [];
-        $any     = false;
+        $any = false;
         foreach ($acceptedLanguages as $acceptedQuality => $acceptedValues) {
             $acceptedQuality = floatval($acceptedQuality);
             if ($acceptedQuality === 0.0) {
                 continue;
             }
-            foreach ($availableLanguages as $availableValue) {
-                $availableQuality = 1.0;
-                foreach ($acceptedValues as $acceptedValue) {
-                    if ($acceptedValue === '*') {
-                        $any = true;
-                    }
+            foreach ($acceptedValues as $acceptedValue) {
+                if ($acceptedValue === '*') {
+                    $any = true;
+                }
+                foreach ($availableLanguages as $availableValue) {
+                    $availableQuality = 1.0;
                     $matchingGrade = self::_matchLanguage($acceptedValue, $availableValue);
                     if ($matchingGrade > 0) {
                         $q = (string) ($acceptedQuality * $availableQuality * $matchingGrade);

tst/I18nTest.php

@@ -44,7 +44,7 @@ class I18nTest extends TestCase
     public function testTranslationFallback()
     {
         $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'foobar';
-        $messageId                       = 'It does not matter if the message ID exists';
+        $messageId = 'It does not matter if the message ID exists';
         I18n::loadTranslations();
         $this->assertEquals($messageId, I18n::_($messageId), 'fallback to en');
         I18n::getLanguageLabels();
@@ -80,6 +80,14 @@ class I18nTest extends TestCase
         $this->assertEquals('2 heures', I18n::_('%d hours', 2), '2 hours in French');
     }
 
+    public function testEqualQualityPreservesBrowserOrder()
+    {
+        $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'fr;q=0.8,de;q=0.8';
+        I18n::loadTranslations();
+
+        $this->assertEquals('fr', I18n::getLanguage());
+    }
+
     public function testBulgarianMonthExpirationTranslation()
     {
         $_COOKIE['lang'] = 'bg';
@@ -212,6 +220,12 @@ class I18nTest extends TestCase
         $this->assertEquals('14 minut',  I18n::_('%d minutes', 14), '14 minutes in Czech');
     }
 
+    public function testInvalidBrowserLanguageQualityIsIgnored()
+    {
+        $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'de;q=2,en;q=0.8';
+        $this->assertSame(['0.8' => ['en']], I18n::getBrowserLanguages());
+    }
+
     public function testBrowserLanguageAnyDetection()
     {
         $_SERVER['HTTP_ACCEPT_LANGUAGE'] = '*';
@@ -230,7 +244,7 @@ class I18nTest extends TestCase
     {
         $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'foobar';
         I18n::loadTranslations();
-        $input  = '&<>"\'/`=';
+        $input = '&<>"\'/`=';
         $result = htmlspecialchars($input, ENT_QUOTES | ENT_HTML5 | ENT_DISALLOWED, 'UTF-8', false);
         $this->assertEquals($result, I18n::encode($input), 'encodes HTML entities');
         $this->assertEquals('<a>some ' . $result . ' + 1</a>', I18n::_('<a>some %s + %d</a>', $input, 1), 'encodes parameters in translations');
@@ -313,7 +327,7 @@ class I18nTest extends TestCase
     public function testMessageIdsExistInAllLanguages()
     {
         $messageIds = [];
-        $languages  = [];
+        $languages = [];
         foreach (new DirectoryIterator(PATH . 'i18n') as $file) {
             $fileNameLength = strlen($file->getFilename());
             if ($fileNameLength === 7) {       // xx.json
@@ -325,9 +339,9 @@ class I18nTest extends TestCase
             } else {
                 continue;
             }
-            $languageJson         = file_get_contents($file->getPathname());
-            $languageMessageIds   = array_keys(Json::decode($languageJson));
-            $messageIds           = array_unique(array_merge($messageIds, $languageMessageIds));
+            $languageJson = file_get_contents($file->getPathname());
+            $languageMessageIds = array_keys(Json::decode($languageJson));
+            $messageIds = array_unique(array_merge($messageIds, $languageMessageIds));
             $languages[$language] = $languageMessageIds;
         }
         foreach ($messageIds as $messageId) {