From 5131dac54956d6e0959ac3070b174133eafe343c Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Wed, 25 Mar 2026 08:53:26 +0100 Subject: [PATCH] fix(imap): handle korean Windows encoding ks_c_5601-1987/ks_c_5601-1989 Assisted-by: OpenCode:claude-haiku-4-5 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/IMAP/Charset/Converter.php | 84 ++++++++++++++++++++--- tests/Unit/IMAP/Charset/ConverterTest.php | 75 +++++++++++++++++--- 2 files changed, 138 insertions(+), 21 deletions(-) diff --git a/lib/IMAP/Charset/Converter.php b/lib/IMAP/Charset/Converter.php index f46b5642aa..8a804ef81f 100644 --- a/lib/IMAP/Charset/Converter.php +++ b/lib/IMAP/Charset/Converter.php @@ -11,11 +11,46 @@ use Horde_Mime_Part; use OCA\Mail\Exception\ServiceException; +use ValueError; use function in_array; use function is_string; class Converter { + /** + * Korean charset aliases used by Outlook/Windows that mbstring does not + * accept verbatim, mapped to UHC. Keys are lowercase because charset tokens + * are case-insensitive. + * + * @see http://lists.w3.org/Archives/Public/ietf-charsets/2001AprJun/0030.html + */ + private const CHARSET_MAP = [ + 'ks_c_5601-1987' => 'UHC', + 'ks_c_5601-1989' => 'UHC', + 'cp949' => 'UHC', + 'windows-949' => 'UHC', + ]; + + private function normalizeCharset(string $charset): string { + $charset = trim($charset); + $lowerCharset = strtolower($charset); + + return self::CHARSET_MAP[$lowerCharset] ?? $charset; + } + + /** + * @return list + */ + private function mbEncodings(): array { + /** @var list|null $encodings */ + static $encodings = null; + if ($encodings === null) { + $encodings = mb_list_encodings(); + } + + return $encodings; + } + /** * @param Horde_Mime_Part $p * @return string @@ -37,10 +72,17 @@ public function convert(Horde_Mime_Part $p): string { // The part specifies a charset if ($charset !== null) { - if (in_array($charset, mb_list_encodings(), true)) { - $converted = mb_convert_encoding($data, 'UTF-8', $charset); - } else { - $converted = iconv($charset, 'UTF-8', $data); + $normalizedCharset = $this->normalizeCharset($charset); + try { + if (in_array($normalizedCharset, $this->mbEncodings(), true)) { + $converted = mb_convert_encoding($data, 'UTF-8', $normalizedCharset); + } else { + $converted = @iconv($normalizedCharset, 'UTF-8', $data); + } + } catch (ValueError) { + // Invalid charset name, treat as null to use auto-detection below + $charset = null; + $converted = null; } if (is_string($converted)) { @@ -51,22 +93,44 @@ public function convert(Horde_Mime_Part $p): string { // No charset specified, let's ask mb if this could be UTF-8 $detectedCharset = mb_detect_encoding($data, 'UTF-8', true); if ($detectedCharset === false) { - // Fallback, non UTF-8 - $detectedCharset = mb_detect_encoding($data, null, true); + // Fallback, try common charsets (the default mb_detect_encoding order may miss some) + $detectedCharset = mb_detect_encoding($data, 'ISO-8859-1,ISO-8859-2,UTF-8,ASCII', true); } // Still UTF8, no need to convert if ($detectedCharset !== false && strtoupper($detectedCharset) === 'UTF-8') { return $data; } - $converted = @mb_convert_encoding($data, 'UTF-8', $charset); - if ($converted === false && $charset !== null) { + // Use detected charset when available, otherwise use original/normalized charset + if ($detectedCharset !== false) { + $sourceCharset = $detectedCharset; + } elseif ($charset !== null) { + $sourceCharset = $this->normalizeCharset($charset); + } else { + // Converting from an unknown source encoding would silently produce + // garbage, so give up instead of guessing. + throw new ServiceException('Could not determine message charset'); + } + + // Attempt conversion with the source charset + try { + $converted = @mb_convert_encoding($data, 'UTF-8', $sourceCharset); + } catch (ValueError) { + $converted = false; + } + + if ($converted === false) { // Might be a charset that PHP mb doesn't know how to handle, fall back to iconv - $converted = iconv($charset, 'UTF-8', $data); + try { + $converted = @iconv($sourceCharset, 'UTF-8', $data); + } catch (ValueError) { + // Invalid charset, conversion not possible + $converted = null; + } } if (!is_string($converted)) { - throw new ServiceException('Could not detect message charset'); + throw new ServiceException('Could not convert message charset'); } return $converted; } diff --git a/tests/Unit/IMAP/Charset/ConverterTest.php b/tests/Unit/IMAP/Charset/ConverterTest.php index ab39e2d76f..107ebb499b 100644 --- a/tests/Unit/IMAP/Charset/ConverterTest.php +++ b/tests/Unit/IMAP/Charset/ConverterTest.php @@ -60,27 +60,80 @@ public function dataProviderMimeParts(): array { $iso2022jpMimePart->setType('text/plain'); $iso2022jpMimePart->setCharset('ISO-2022-JP'); $iso2022jpMimePart->setContents(mb_convert_encoding('外せ園査リツハワ題', 'ISO-2022-JP', 'UTF-8')); - $iso2022jpMimePart_noCharset = new Horde_Mime_Part(); - $iso2022jpMimePart_noCharset->setContents('外せ園査リツハワ題'); - // Korean - not in mb nor iconv - // $iso106461MimePart = new Horde_Mime_Part(); - // $iso106461MimePart->setType('text/plain'); - // $iso106461MimePart->setCharset('ISO 10646-1'); - //$iso106461MimePart->setContents(iconv('UTF-8', 'ISO 10646-1', '언론·출판은 타인의 명')); + // Korean (Outlook) - all ks_c_5601 spellings map to UHC (CP949). Encode + // with iconv to avoid depending on mbstring's UHC support, and cover the + // case-insensitive charset spellings. + $koreanText = '안녕하세요'; + $koreanBytes = iconv('UTF-8', 'CP949', $koreanText); + $koreanCharsets = ['ks_c_5601-1987', 'ks_c_5601-1989', 'KS_C_5601-1987', 'Ks_C_5601-1987', 'cp949', 'windows-949']; + $koreanCases = []; + foreach ($koreanCharsets as $koreanCharset) { + $koreanMimePart = new Horde_Mime_Part(); + $koreanMimePart->setType('text/plain'); + $koreanMimePart->setCharset($koreanCharset); + $koreanMimePart->setContents($koreanBytes); + $koreanCases[] = [$koreanMimePart, $koreanText]; + } // Arabic - not in mb $windowsMimePart = new Horde_Mime_Part(); $windowsMimePart->setType('text/plain'); $windowsMimePart->setCharset('Windows-1256'); $windowsMimePart->setContents(iconv('UTF-8', 'Windows-1256', 'قام زهاء أوراقهم ما,')); - return[ + return array_merge([ [$utfMimePart, '😊'], [$utfMimeStreamPart, '💦'], [$iso88591MimePart, 'Ümlaut'], [$iso2022jpMimePart, '外せ園査リツハワ題'], [$iso88591MimePart_noCharset, 'בה בדף לחבר ממונרכיה, בקר בגרסה ואמנות דת'], - // [$iso106461MimePart, '언론·출판은 타인의 명'], - [$windowsMimePart, 'قام زهاء أوراقهم ما,'] - ]; + ], $koreanCases, [ + [$windowsMimePart, 'قام زهاء أوراقهم ما,'], + ]); + } + + /** + * A part without a charset header must still decode via detection. + */ + public function testConvertWithNullCharsetFallback(): void { + $mimePart = $this->createMock(Horde_Mime_Part::class); + $mimePart->method('getContents') + ->willReturn(mb_convert_encoding('Tëst', 'ISO-8859-1', 'UTF-8')); + $mimePart->method('getCharset') + ->willReturn(null); + + $result = $this->converter->convert($mimePart); + + $this->assertEquals('Tëst', $result); + $this->assertTrue(mb_check_encoding($result, 'UTF-8')); + } + + /** + * Test that an invalid/unknown charset name does not let ValueError bubble up. + * + * When an invalid charset is provided, Converter catches the ValueError + * and falls back to mbstring auto-detection. The result depends on + * mb_detect_order, but the important behavior is that no ValueError escapes. + */ + public function testConvertWithInvalidCharsetDoesNotThrowValueError(): void { + $mimePart = $this->createMock(Horde_Mime_Part::class); + $mimePart->method('getContents') + ->willReturn(mb_convert_encoding('Tëst with spëcial chärs', 'ISO-8859-1', 'UTF-8')); + $mimePart->method('getCharset') + ->willReturn('INVALID-CHARSET-NAME-12345'); + + $thrown = null; + $result = null; + try { + $result = $this->converter->convert($mimePart); + } catch (\ValueError $e) { + $thrown = $e; + } catch (\OCA\Mail\Exception\ServiceException) { + // ServiceException is acceptable (auto-detection failed) + } + + $this->assertNull($thrown, 'ValueError should not bubble up from convert()'); + if ($result !== null) { + $this->assertTrue(mb_check_encoding($result, 'UTF-8'), 'A successful conversion must yield valid UTF-8'); + } } }