src: json_encode() throws when UTF-8 key repair collides two keys

Malformed key bytes become � so json_encode() never fails, but two
different bad keys can repair to the same text, and the rebuild kept
whichever came last - silent data loss. A collision now throws naming
the repaired key (HTML-encoded - keys are data, and handlers often
echo exception messages into a page). One-bad-key repair is unchanged.

Author: daveedis

CHANGELOG.md

@@ -25,7 +25,8 @@ Everything else is hardening and fixes.
   reflected XSS vector
 - **`json_encode()` survives malformed UTF-8 in keys** - bad bytes become
   � instead of the whole document returning false (values were already
-  handled)
+  handled). If the substitution would make two keys identical, it throws
+  rather than silently dropping a record
 - **`debug()` and `help()` escape `</xmp`** - a stored value containing
   `</xmp>` ended the debug block early, so the rest of it parsed as live
   HTML

src/SmartArrayBase.php

@@ -1838,9 +1838,11 @@ private function wrappingIterator(): Iterator
      *
      * Substitutes malformed UTF-8 in keys and values with � (U+FFFD) so json_encode($smartArray)
      * returns valid JSON instead of false. Nested SmartArrays scrub themselves when json_encode()
-     * descends into them.
+     * descends into them. If substitution makes two keys identical, throws instead of silently
+     * dropping a record.
      *
      * @return array The internal data array.
+     * @throws RuntimeException If key substitution makes two keys identical
      */
     public function jsonSerialize(): array
     {
@@ -1867,6 +1869,10 @@ public function jsonSerialize(): array
             if (is_string($value) && preg_match('//u', $value) !== 1) {
                 $value = json_decode(json_encode($value, JSON_INVALID_UTF8_SUBSTITUTE));
             }
+            if (array_key_exists($key, $data)) { // original keys were unique, so a repeat means substitution collapsed two keys
+                $keyDisplay = self::htmlEncode((string) $key); // SECURITY: keys are data, and exception handlers often echo messages into a page
+                throw new RuntimeException("jsonSerialize(): key '$keyDisplay' appears twice after malformed UTF-8 bytes were replaced with \u{FFFD}, which would silently drop a record. Fix the key encoding before calling json_encode().");
+            }
             $data[$key] = $value;
         }
         return $data;

tests/Unit/ConversionTest.php

@@ -4,6 +4,7 @@
 namespace Itools\SmartArray\Tests\Unit;
 
 use InvalidArgumentException;
+use RuntimeException;
 use Itools\SmartArray\SmartArray;
 use Itools\SmartArray\SmartArrayBase;
 use Itools\SmartArray\SmartArrayHtml;
@@ -464,5 +465,53 @@ public function testJsonSerializeKeepsMalformedKeyInPosition(string $class): voi
         $this->assertSame(['a', "caf\u{FFFD}", 'z'], array_keys($sa->jsonSerialize()));
     }
 
+    #[DataProvider('modeProvider')]
+    public function testJsonSerializeThrowsWhenScrubbedKeysCollide(string $class): void
+    {
+        // "caf\xE9" and "caf\xC0" both scrub to "caf�" - silently keeping only
+        // one record would be data loss, so it throws instead
+        $sa = $class::new(["caf\xE9" => 1, "caf\xC0" => 2]);
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage("key 'caf\u{FFFD}' appears twice after malformed UTF-8");
+
+        $sa->jsonSerialize();
+    }
+
+    #[DataProvider('modeProvider')]
+    public function testJsonSerializeThrowsWhenScrubbedKeyMatchesLiteralReplacementChar(string $class): void
+    {
+        // A malformed key can also collide with a valid key that already spells
+        // U+FFFD; both insert orders throw
+        $arrays = [
+            ["caf\xE9" => 1, "caf\u{FFFD}" => 2],
+            ["caf\u{FFFD}" => 1, "caf\xE9" => 2],
+        ];
+        foreach ($arrays as $array) {
+            $thrown = null;
+            try {
+                $class::new($array)->jsonSerialize();
+            } catch (RuntimeException $e) {
+                $thrown = $e;
+            }
+            $this->assertInstanceOf(RuntimeException::class, $thrown, 'collision must throw in both insert orders');
+        }
+    }
+
+    #[DataProvider('modeProvider')]
+    public function testJsonSerializeCollisionMessageEncodesTheKey(string $class): void
+    {
+        // Keys are data, and exception handlers often echo messages into a page
+        $sa = $class::new(["<b>\xE9" => 1, "<b>\xC0" => 2]);
+
+        try {
+            $sa->jsonSerialize();
+            $this->fail('Expected RuntimeException for colliding keys');
+        } catch (RuntimeException $e) {
+            $this->assertStringContainsString('&lt;b&gt;', $e->getMessage(), 'markup in the key must be encoded');
+            $this->assertStringNotContainsString('<b>', $e->getMessage());
+        }
+    }
+
     //endregion
 }