Skip to content

Fix case-insensitive charset detection for ISO-2022-JP - #13641

Open
neconohitomi wants to merge 1 commit into
nextcloud:mainfrom
neconohitomi:fix/iso-2022-jp-charset
Open

Fix case-insensitive charset detection for ISO-2022-JP#13641
neconohitomi wants to merge 1 commit into
nextcloud:mainfrom
neconohitomi:fix/iso-2022-jp-charset

Conversation

@neconohitomi

Copy link
Copy Markdown

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Summary

Fixes ISO-2022-JP email conversion when the charset is provided in lowercase.

Problem

Some emails declare their charset as ISO-2022-JP, but Horde can return the charset as lowercase (iso-2022-jp).

Converter previously checked the charset against mb_list_encodings() using a case-sensitive comparison. As a result, iso-2022-jp was not recognized as an mbstring encoding and the conversion fell back to iconv().

On the affected Alpine/libiconv environment, iconv() fails to convert these ISO-2022-JP messages, resulting in garbled email content.

Fix

Make the charset comparison case-insensitive when checking the available mbstring encodings.

The existing charset normalization and conversion logic are otherwise unchanged.

Tests

Added a regression test covering lowercase iso-2022-jp.

Tested with:

  • ConverterTest: 15 tests, 30 assertions — OK
  • Full Mail unit test suite: 1518 tests, 4378 assertions — OK

Fixes #13472

$normalizedCharset = $this->normalizeCharset($charset);
try {
if (in_array($normalizedCharset, $this->mbEncodings(), true)) {
if (in_array(strtoupper($charset), array_map('strtoupper', $this->mbEncodings()), true)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move array_map(strtoupper) into the mbEncodings function.

$normalizedCharset = $this->normalizeCharset($charset);
try {
if (in_array($normalizedCharset, $this->mbEncodings(), true)) {
if (in_array(strtoupper($charset), array_map('strtoupper', $this->mbEncodings()), true)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strtoupper($charset) should be done in normalizedCharset. At best by altering the map to also use uppercase and flip lowerCharset to upperCharset and return that version as fallback.

@kesselb

kesselb commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Good finding, Thanks for your pr 👍

@kesselb

kesselb commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

The commit message needs a DCO signoff and the Assisted-By trailer.

It's often enough to point your Agent https://github.com/nextcloud/mail/blob/main/AGENTS.md#commit-message-format and https://github.com/nextcloud/mail/pull/13641/checks?check_run_id=101341532634 to resolve it.

@neconohitomi

Copy link
Copy Markdown
Author

The commit message needs a DCO signoff and the Assisted-By trailer.

It's often enough to point your Agent https://github.com/nextcloud/mail/blob/main/AGENTS.md#commit-message-format and https://github.com/nextcloud/mail/pull/13641/checks?check_run_id=101341532634 to resolve it.

Thanks for the review and for pointing out that the current commit is missing the required DCO Signed-off-by and Assisted-by trailers.

I used an AI agent (ChatGPT) while preparing this PR, and this makes me wonder about the following situation. I would appreciate your advice.

According to AGENTS.md, review changes should be submitted as fixup commits without force-pushing. However, the DCO check reports that the existing commit itself is missing the sign-off.

What would be the preferred way to fix the DCO/Assisted-by trailers in this PR while following the review workflow?

I can prepare the required fixup commit if that is the preferred approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ISO-2022-JP emails are garbled in Mail 5.10.12 [Minimal fix available]

2 participants