diff --git a/lib/I18n.php b/lib/I18n.php index 1bf20a2f..2e06ac51 100644 --- a/lib/I18n.php +++ b/lib/I18n.php @@ -127,14 +127,13 @@ class I18n } else { $args[0] = self::$_translations[$messageId]; } - // encode any non-integer arguments and the message ID, if it doesn't contain a link or keyboard input + // encode any non-integer arguments, but not the message itself + // The message ID comes from trusted sources (code or translation JSON files), + // while parameters may come from untrusted sources and need HTML entity encoding + // to prevent XSS attacks when the message is inserted into HTML context $argsCount = count($args); - for ($i = 0; $i < $argsCount; ++$i) { - if ($i === 0) { - if (str_contains($args[0], '')) { - continue; - } - } elseif (is_int($args[$i])) { + for ($i = 1; $i < $argsCount; ++$i) { + if (is_int($args[$i])) { continue; } $args[$i] = self::encode($args[$i]); diff --git a/tst/I18nTest.php b/tst/I18nTest.php index 9e196103..118fe23b 100644 --- a/tst/I18nTest.php +++ b/tst/I18nTest.php @@ -182,7 +182,20 @@ class I18nTest extends TestCase $result = htmlspecialchars($input, ENT_QUOTES | ENT_HTML5 | ENT_DISALLOWED, 'UTF-8', false); $this->assertEquals($result, I18n::encode($input), 'encodes HTML entities'); $this->assertEquals('some ' . $result . ' + 1', I18n::_('some %s + %d', $input, 1), 'encodes parameters in translations'); - $this->assertEquals($result . $result, I18n::_($input . '%s', $input), 'encodes message ID as well, when no link'); + // Message ID should NOT be encoded (it comes from trusted source), only the parameter should be + $this->assertEquals($input . $result, I18n::_($input . '%s', $input), 'encodes only parameters, not message ID'); + } + + public function testFrenchApostropheInMessage() + { + $_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'fr'; + I18n::loadTranslations(); + // The French translation should not have the apostrophe encoded + // Original: "Le document n'existe pas, a expiré, ou a été supprimé." + // Should NOT become: "Le document n'existe pas, a expiré, ou a été supprimé." + $message = I18n::_('Document does not exist, has expired or has been deleted.'); + $this->assertFalse(strpos($message, ''') !== false, 'French apostrophe should not be encoded in translation message'); + $this->assertTrue(strpos($message, "n'existe") !== false, 'French apostrophe should be present as literal character'); } public function testFallbackAlwaysPresent()