fix: disallow string callables (#53)

Skillshare · May 23, 2022 · 4ded2e2 · 4ded2e2
1 parent 4a34f67
commit 4ded2e2
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## 0.7.1 - 2022-05-23
+
+### Added
+
+- Nothing.
+
+### Changed
+
+- Nothing.
+
+### Deprecated
+
+- Nothing.
+
+### Removed
+
+- Nothing.
+
+### Fixed
+
+- Disallow the use of string callables in values passed to the formatter. Only array callables and closures are allowed.
+
 ## 0.7.0 - 2022-02-11
 
 ### Added

diff --git a/src/Intl/MessageFormat.php b/src/Intl/MessageFormat.php
@@ -42,6 +42,7 @@
 use function is_callable;
 use function is_int;
 use function is_numeric;
+use function is_string;
 use function preg_match;
 use function sprintf;
 
@@ -97,8 +98,6 @@ public function format(string $pattern, array $values = []): string
     }
 
     /**
-     * @param array<array-key, float | int | string | callable(string):string> $values
-     *
      * @throws Parser\Exception\IllegalParserUsageException
      * @throws Parser\Exception\InvalidArgumentException
      * @throws Parser\Exception\InvalidOffsetException
@@ -108,16 +107,27 @@ public function format(string $pattern, array $values = []): string
      * @throws Parser\Exception\UnableToParseMessageException
      * @throws UnableToFormatMessageException
      * @throws CollectionMismatchException
+     *
+     * @psalm-param array<array-key, float | int | string | callable(string=):string> $values
      */
     private function applyPreprocessing(string $pattern, array &$values = []): string
     {
-        $callbacks = array_filter($values, fn ($value): bool => is_callable($value));
+        /** @var array<array-key, callable(string=):string> $callbacks */
+        $callbacks = array_filter($values, fn ($value): bool => !is_string($value) && is_callable($value));
 
         // Remove the callbacks from the values, since we will use them below.
         foreach (array_keys($callbacks) as $key) {
             unset($values[$key]);
         }
 
+        /**
+         * This is to satisfy static analysis. At this point, $values should
+         * not contain any callables.
+         *
+         * @var array<array-key, float | int | string> $valuesWithoutCallables
+         */
+        $valuesWithoutCallables = &$values;
+
         $parserOptions = new Parser\Options();
         $parserOptions->shouldParseSkeletons = true;
 
@@ -130,15 +140,16 @@ private function applyPreprocessing(string $pattern, array &$values = []): strin
 
         assert($parsed->val instanceof Parser\Type\ElementCollection);
 
-        return (new Printer())->printAst($this->processAst($parsed->val, $callbacks, $values));
+        return (new Printer())->printAst($this->processAst($parsed->val, $callbacks, $valuesWithoutCallables));
     }
 
     /**
-     * @param array<array-key, callable(string):string> $callbacks
-     * @param array<array-key, float | int | string | callable(string):string> $values
+     * @param array<array-key, float | int | string> $values
      *
      * @throws CollectionMismatchException
      * @throws UnableToFormatMessageException
+     *
+     * @psalm-param array<array-key, callable(string=):string> $callbacks
      */
     private function processAst(
         Parser\Type\ElementCollection $ast,
@@ -179,11 +190,12 @@ private function processAst(
     }
 
     /**
-     * @param array<array-key, callable(string):string> $callbacks
-     * @param array<array-key, float | int | string | callable(string):string> $values
+     * @param array<array-key, float | int | string> $values
      *
      * @throws CollectionMismatchException
      * @throws UnableToFormatMessageException
+     *
+     * @psalm-param array<array-key, callable(string=):string> $callbacks
      */
     private function processTagElement(
         Parser\Type\TagElement $tagElement,
@@ -246,7 +258,7 @@ private function processTagElement(
      * @link https://tc39.es/ecma402/#sec-partitionnumberpattern
      * @link https://formatjs.io/docs/core-concepts/icu-syntax/#number-type
      *
-     * @param array<array-key, float | int | string | callable(string):string> $values
+     * @param array<array-key, float | int | string> $values
      */
     private function processNumberElement(
         Parser\Type\NumberElement $numberElement,
@@ -267,10 +279,10 @@ private function processNumberElement(
     }
 
     /**
-     * @param array<array-key, callable(string):string> $callbacks
-     *
      * @throws CollectionMismatchException
      * @throws UnableToFormatMessageException
+     *
+     * @psalm-param array<array-key, callable(string=):string> $callbacks
      */
     private function processLiteralElement(
         Parser\Type\LiteralElement $literalElement,

diff --git a/src/Intl/MessageFormatInterface.php b/src/Intl/MessageFormatInterface.php
@@ -71,9 +71,9 @@ interface MessageFormatInterface
      * );
      * ```
      *
-     * @param array<array-key, float | int | string | callable(string):string> $values
-     *
      * @throws UnableToFormatMessageException
+     *
+     * @psalm-param array<array-key, float | int | string | callable(string=):string> $values
      */
     public function format(string $pattern, array $values = []): string;
 }
diff --git a/tests/Intl/MessageFormatTest.php b/tests/Intl/MessageFormatTest.php
@@ -378,4 +378,54 @@ public function testProcessesNumberWithoutStyle(): void
 
         $this->assertSame($expected, $result);
     }
+
+    public function testArrayCallablesAndClosures(): void
+    {
+        $message = 'Hello, <firstName></firstName> <lastName></lastName>!';
+        $expected = 'Hello, Jane Doe!';
+
+        $user = new class {
+            public function getFirstName(): string
+            {
+                return 'Jane';
+            }
+
+            public function getLastName(): string
+            {
+                return 'Doe';
+            }
+        };
+
+        $locale = new Locale('en-US');
+        $formatter = new MessageFormat($locale);
+
+        $result = $formatter->format(
+            $message,
+            [
+                'firstName' => [$user, 'getFirstName'],
+                'lastName' => fn (): string => $user->getLastName(),
+            ],
+        );
+
+        $this->assertSame($expected, $result);
+    }
+
+    public function testStringsMustNotEvaluateAsCallables(): void
+    {
+        $message = 'Hello, {firstName} {lastName}!';
+        $expected = 'Hello, Ceil Floor!';
+
+        $locale = new Locale('en-US');
+        $formatter = new MessageFormat($locale);
+
+        $result = $formatter->format(
+            $message,
+            [
+                'firstName' => 'Ceil',
+                'lastName' => 'Floor',
+            ],
+        );
+
+        $this->assertSame($expected, $result);
+    }
 }