diff --git a/lib/IMAP/Charset/Converter.php b/lib/IMAP/Charset/Converter.php index f46b5642aa..f36750c464 100644 --- a/lib/IMAP/Charset/Converter.php +++ b/lib/IMAP/Charset/Converter.php @@ -11,11 +11,53 @@ use Horde_Mime_Part; use OCA\Mail\Exception\ServiceException; +use ValueError; use function in_array; use function is_string; class Converter { + /** + * Map of unsupported charset names to their mbstring equivalents. + * Keys must be lowercase for case-insensitive lookup. + * + * @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', + ]; + + /** + * Normalize charset names for mbstring compatibility. + * + * Maps unsupported charset names to their mbstring equivalents. + * Notably, handles Korean encodings used by Outlook: + * - ks_c_5601-1987 and ks_c_5601-1989 are mapped to UHC (Windows-949/CP949) + * + * Charset tokens are case-insensitive in email headers (RFC 2045), + * so we normalize to lowercase for lookup. + */ + 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 +79,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 +100,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..5720968e9c 100644 --- a/tests/Unit/IMAP/Charset/ConverterTest.php +++ b/tests/Unit/IMAP/Charset/ConverterTest.php @@ -60,27 +60,90 @@ 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']; + $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, 'قام زهاء أوراقهم ما,'], + ]); + } + + /** + * Test that conversion succeeds when no charset is specified in the MIME header. + * + * This tests the code path where $charset is null. The Converter should: + * 1. Use mb_detect_encoding() to detect the source encoding + * 2. Use the detected charset for conversion to UTF-8 + * + * Without detection, conversion would fail or produce garbled output. + */ + public function testConvertWithNullCharsetFallback(): void { + // Create a mock that returns null for getCharset() to test the null charset path + $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); + + // Should complete without ValueError and return the correctly converted text + $result = $this->converter->convert($mimePart); + + // Verify actual conversion correctness, not just UTF-8 validity + $this->assertEquals('Tëst', $result); + // Also verify it's valid UTF-8 + $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'); + } } }