Code injection in the way Symfony implements translation caching in FrameworkBundle

@@ -45,7 +45,7 @@ public function testTransWithoutCaching()
     {
         $translator = $this->getTranslator($this->getLoader());
         $translator->setLocale('fr');
-        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR'));
+        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8'));
 
         $this->assertEquals('foo (FR)', $translator->trans('foo'));
         $this->assertEquals('bar (EN)', $translator->trans('bar'));
@@ -54,14 +54,15 @@ public function testTransWithoutCaching()
         $this->assertEquals('no translation', $translator->trans('no translation'));
         $this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
         $this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
+        $this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
     }
 
     public function testTransWithCaching()
     {
         // prime the cache
         $translator = $this->getTranslator($this->getLoader(), array('cache_dir' => $this->tmpDir));
         $translator->setLocale('fr');
-        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR'));
+        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8'));
 
         $this->assertEquals('foo (FR)', $translator->trans('foo'));
         $this->assertEquals('bar (EN)', $translator->trans('bar'));
@@ -70,12 +71,13 @@ public function testTransWithCaching()
         $this->assertEquals('no translation', $translator->trans('no translation'));
         $this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
         $this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
+        $this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
 
         // do it another time as the cache is primed now
         $loader = $this->getMock('Symfony\Component\Translation\Loader\LoaderInterface');
         $translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir));
         $translator->setLocale('fr');
-        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR'));
+        $translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8'));
 
         $this->assertEquals('foo (FR)', $translator->trans('foo'));
         $this->assertEquals('bar (EN)', $translator->trans('bar'));
@@ -84,6 +86,7 @@ public function testTransWithCaching()
         $this->assertEquals('no translation', $translator->trans('no translation'));
         $this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
         $this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
+        $this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
     }
 
     public function testGetLocale()
@@ -175,6 +178,13 @@ protected function getLoader()
                 'other choice' => '{0} other choice 0 (PT-BR)|{1} other choice 1 (PT-BR)|]1,Inf] other choice inf (PT-BR)',
             ))))
         ;
+        $loader
+            ->expects($this->at(5))
+            ->method('load')
+            ->will($this->returnValue($this->getCatalogue('fr.UTF-8', array(
+                'foobarbaz' => 'foobarbaz (fr.UTF-8)',
+            ))))
+        ;
 
         return $loader;
     }
@@ -205,6 +215,7 @@ public function getTranslator($loader, $options = array())
         $translator->addResource('loader', 'foo', 'es');
         $translator->addResource('loader', 'foo', 'pt-PT'); // European Portuguese
         $translator->addResource('loader', 'foo', 'pt_BR'); // Brazilian Portuguese
+        $translator->addResource('loader', 'foo', 'fr.UTF-8');
 
         return $translator;
     }

@@ -97,8 +97,10 @@ protected function loadCatalogue($locale)
 
             $fallbackContent = '';
             $current = '';
+            $replacementPattern = '/[^a-z0-9_]/i';
             foreach ($this->computeFallbackLocales($locale) as $fallback) {
-                $fallbackSuffix = ucfirst(str_replace('-', '_', $fallback));
+                $fallbackSuffix = ucfirst(preg_replace($replacementPattern, '_', $fallback));
+                $currentSuffix = ucfirst(preg_replace($replacementPattern, '_', $current));
 
                 $fallbackContent .= sprintf(<<<EOF
 \$catalogue%s = new MessageCatalogue('%s', %s);
@@ -110,7 +112,7 @@ protected function loadCatalogue($locale)
                     $fallbackSuffix,
                     $fallback,
                     var_export($this->catalogues[$fallback]->all(), true),
-                    ucfirst(str_replace('-', '_', $current)),
+                    $currentSuffix,
                     $fallbackSuffix
                 );
                 $current = $fallback;

@@ -17,6 +17,33 @@
 
 class TranslatorTest extends \PHPUnit_Framework_TestCase
 {
+
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testConstructorInvalidLocale($locale)
+    {
+        $translator = new Translator($locale, new MessageSelector());
+    }
+
+    /**
+     * @dataProvider getValidLocalesTests
+     */
+    public function testConstructorValidLocale($locale)
+    {
+        $translator = new Translator($locale, new MessageSelector());
+
+        $this->assertEquals($locale, $translator->getLocale());
+    }
+
+    public function testConstructorWithoutLocale()
+    {
+        $translator = new Translator(null, new MessageSelector());
+
+        $this->assertNull($translator->getLocale());
+    }
+
     public function testSetGetLocale()
     {
         $translator = new Translator('en', new MessageSelector());
@@ -27,6 +54,27 @@ public function testSetGetLocale()
         $this->assertEquals('fr', $translator->getLocale());
     }
 
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSetInvalidLocale($locale)
+    {
+        $translator = new Translator('fr', new MessageSelector());
+        $translator->setLocale($locale);
+    }
+
+    /**
+     * @dataProvider getValidLocalesTests
+     */
+    public function testSetValidLocale($locale)
+    {
+        $translator = new Translator($locale, new MessageSelector());
+        $translator->setLocale($locale);
+
+        $this->assertEquals($locale, $translator->getLocale());
+    }
+
     public function testSetFallbackLocales()
     {
         $translator = new Translator('en', new MessageSelector());
@@ -55,6 +103,27 @@ public function testSetFallbackLocalesMultiple()
         $this->assertEquals('bar (fr)', $translator->trans('bar'));
     }
 
+
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSetFallbackInvalidLocales($locale)
+    {
+        $translator = new Translator('fr', new MessageSelector());
+        $translator->setFallbackLocales(array('fr', $locale));
+    }
+
+    /**
+     * @dataProvider getValidLocalesTests
+     */
+    public function testSetFallbackValidLocales($locale)
+    {
+        $translator = new Translator($locale, new MessageSelector());
+        $translator->setFallbackLocales(array('fr', $locale));
+        // no assertion. this method just asserts that no exception is thrown
+    }
+
     public function testTransWithFallbackLocale()
     {
         $translator = new Translator('fr_FR', new MessageSelector());
@@ -67,6 +136,26 @@ public function testTransWithFallbackLocale()
         $this->assertEquals('foobar', $translator->trans('bar'));
     }
 
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testAddResourceInvalidLocales($locale)
+    {
+        $translator = new Translator('fr', new MessageSelector());
+        $translator->addResource('array', array('foo' => 'foofoo'), $locale);
+    }
+
+    /**
+     * @dataProvider getValidLocalesTests
+     */
+    public function testAddResourceValidLocales($locale)
+    {
+        $translator = new Translator('fr', new MessageSelector());
+        $translator->addResource('array', array('foo' => 'foofoo'), $locale);
+        // no assertion. this method just asserts that no exception is thrown
+    }
+
     public function testAddResourceAfterTrans()
     {
         $translator = new Translator('fr', new MessageSelector());
@@ -164,6 +253,32 @@ public function testTrans($expected, $id, $translation, $parameters, $locale, $d
         $this->assertEquals($expected, $translator->trans($id, $parameters, $domain, $locale));
     }
 
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testTransInvalidLocale($locale)
+    {
+        $translator = new Translator('en', new MessageSelector());
+        $translator->addLoader('array', new ArrayLoader());
+        $translator->addResource('array', array('foo' => 'foofoo'), 'en');
+
+        $translator->trans('foo', array(), '', $locale);
+    }
+
+    /**
+     * @dataProvider      getValidLocalesTests
+     */
+    public function testTransValidLocale($locale)
+    {
+        $translator = new Translator('en', new MessageSelector());
+        $translator->addLoader('array', new ArrayLoader());
+        $translator->addResource('array', array('foo' => 'foofoo'), 'en');
+
+        $translator->trans('foo', array(), '', $locale);
+        // no assertion. this method just asserts that no exception is thrown
+    }
+
     /**
      * @dataProvider getFlattenedTransTests
      */
@@ -188,6 +303,33 @@ public function testTransChoice($expected, $id, $translation, $number, $paramete
         $this->assertEquals($expected, $translator->transChoice($id, $number, $parameters, $domain, $locale));
     }
 
+    /**
+     * @dataProvider      getInvalidLocalesTests
+     * @expectedException \InvalidArgumentException
+     */
+    public function testTransChoiceInvalidLocale($locale)
+    {
+        $translator = new Translator('en', new MessageSelector());
+        $translator->addLoader('array', new ArrayLoader());
+        $translator->addResource('array', array('foo' => 'foofoo'), 'en');
+
+        $translator->transChoice('foo', 1, array(), '', $locale);
+    }
+
+    /**
+     * @dataProvider      getValidLocalesTests
+     */
+    public function testTransChoiceValidLocale($locale)
+    {
+        $translator = new Translator('en', new MessageSelector());
+        $translator->addLoader('array', new ArrayLoader());
+        $translator->addResource('array', array('foo' => 'foofoo'), 'en');
+
+        $translator->transChoice('foo', 1, array(), '', $locale);
+        // no assertion. this method just asserts that no exception is thrown
+    }
+
+
     public function getTransFileTests()
     {
         return array(
@@ -257,6 +399,39 @@ public function getTransChoiceTests()
         );
     }
 
+    public function getInvalidLocalesTests()
+    {
+        return array(
+            array('fr FR'),
+            array('français'),
+            array('fr+en'),
+            array('utf#8'),
+            array('fr&en'),
+            array('fr~FR'),
+            array(' fr'),
+            array('fr '),
+            array('fr*'),
+            array('fr/FR'),
+            array('fr\\FR'),
+        );
+    }
+
+    public function getValidLocalesTests()
+    {
+        return array(
+            array(''),
+            array(null),
+            array('fr'),
+            array('francais'),
+            array('FR'),
+            array('frFR'),
+            array('fr-FR'),
+            array('fr_FR'),
+            array('fr.FR'),
+            array('fr-FR.UTF8'),
+        );
+    }
+
     public function testTransChoiceFallback()
     {
         $translator = new Translator('ru', new MessageSelector());

@@ -28,6 +28,8 @@ interface TranslatorInterface
      * @param string $domain     The domain for the message
      * @param string $locale     The locale
      *
+     * @throws \InvalidArgumentException If the locale contains invalid characters
+     *
      * @return string The translated string
      *
      * @api
@@ -43,6 +45,8 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
      * @param string  $domain     The domain for the message
      * @param string  $locale     The locale
      *
+     * @throws \InvalidArgumentException If the locale contains invalid characters
+     *
      * @return string The translated string
      *
      * @api
@@ -54,6 +58,8 @@ public function transChoice($id, $number, array $parameters = array(), $domain =
      *
      * @param string $locale The locale
      *
+     * @throws \InvalidArgumentException If the locale contains invalid characters
+     *
      * @api
      */
     public function setLocale($locale);

@@ -59,11 +59,13 @@ class Translator implements TranslatorInterface
      * @param string               $locale   The locale
      * @param MessageSelector|null $selector The message selector for pluralization
      *
+     * @throws \InvalidArgumentException If a locale contains invalid characters
+     *
      * @api
      */
     public function __construct($locale, MessageSelector $selector = null)
     {
-        $this->locale = $locale;
+        $this->setLocale($locale);
         $this->selector = $selector ?: new MessageSelector();
     }
 
@@ -88,6 +90,8 @@ public function addLoader($format, LoaderInterface $loader)
      * @param string $locale   The locale
      * @param string $domain   The domain
      *
+     * @throws \InvalidArgumentException If the locale contains invalid characters
+     *
      * @api
      */
     public function addResource($format, $resource, $locale, $domain = null)
@@ -96,6 +100,8 @@ public function addResource($format, $resource, $locale, $domain = null)
             $domain = 'messages';
         }
 
+        $this->assertValidLocale($locale);
+
         $this->resources[$locale][] = array($format, $resource, $domain);
 
         if (in_array($locale, $this->fallbackLocales)) {
@@ -112,6 +118,7 @@ public function addResource($format, $resource, $locale, $domain = null)
      */
     public function setLocale($locale)
     {
+        $this->assertValidLocale($locale);
         $this->locale = $locale;
     }
 
@@ -130,6 +137,8 @@ public function getLocale()
      *
      * @param string|array $locales The fallback locale(s)
      *
+     * @throws \InvalidArgumentException If a locale contains invalid characters
+     *
      * @deprecated since 2.3, to be removed in 3.0. Use setFallbackLocales() instead.
      *
      * @api
@@ -144,13 +153,19 @@ public function setFallbackLocale($locales)
      *
      * @param array $locales The fallback locales
      *
+     * @throws \InvalidArgumentException If a locale contains invalid characters
+     *
      * @api
      */
     public function setFallbackLocales(array $locales)
     {
         // needed as the fallback locales are linked to the already loaded catalogues
         $this->catalogues = array();
 
+        foreach ($locales as $locale) {
+            $this->assertValidLocale($locale);
+        }
+
         $this->fallbackLocales = $locales;
     }
 
@@ -175,6 +190,8 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
     {
         if (null === $locale) {
             $locale = $this->getLocale();
+        } else {
+            $this->assertValidLocale($locale);
         }
 
         if (null === $domain) {
@@ -197,6 +214,8 @@ public function transChoice($id, $number, array $parameters = array(), $domain =
     {
         if (null === $locale) {
             $locale = $this->getLocale();
+        } else {
+            $this->assertValidLocale($locale);
         }
 
         if (null === $domain) {
@@ -279,4 +298,18 @@ protected function computeFallbackLocales($locale)
 
         return array_unique($locales);
     }
+
+    /**
+     * Asserts that the locale is valid, throws an Exception if not.
+     *
+     * @param string $locale Locale to tests
+     *
+     * @throws \InvalidArgumentException If the locale contains invalid characters
+     */
+    private function assertValidLocale($locale)
+    {
+        if (0 !== preg_match('/[^a-z0-9_\\.\\-]+/i', $locale, $match)) {
+            throw new \InvalidArgumentException(sprintf('Invalid locale: %s.', $locale));
+        }
+    }
 }