Skip to content

Fix decoding of multi-segment RFC2231 extended attachment filenames - #10268

Merged
alecpl merged 1 commit into
roundcube:masterfrom
MiMoHo:held-mime-rfc2231
Jul 25, 2026
Merged

Fix decoding of multi-segment RFC2231 extended attachment filenames#10268
alecpl merged 1 commit into
roundcube:masterfrom
MiMoHo:held-mime-rfc2231

Conversation

@MiMoHo

@MiMoHo MiMoHo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

An attachment whose filename is split across multiple RFC2231 extended (percent-encoded) parameters is decoded only in its first segment; the continuation segments remain percent-encoded, producing a corrupted filename.

Example header:

Content-Disposition: attachment;
 filename*0*=UTF-8''%e2%82%ac;
 filename*1*=%e2%82%ac

was decoded to €%e2%82%ac instead of €€.

Root cause

rcube_mime_decode::parseHeaderValue() (program/lib/Roundcube/rcube_mime_decode.php:295-307) applied rawurldecode() only when a segment carried the charset'lang' prefix (matched at line 299), which is present only on the first segment (filename*0*). Continuation segments (filename*1*, filename*2*, …) have no such prefix and fell through with their value appended raw, still percent-encoded.

Fix

The parameter regex now captures the trailing * that marks an extended (percent-encoded) parameter. Continuation segments of an extended parameter are rawurldecode()'d before concatenation. Plain (non-extended) continuations (filename*0=, no trailing star) are still treated as literal and are not decoded, per RFC2231.

Testing

Added tests/Framework/MimeDecodeTest.php cases:

  • test_decode_rfc2231_extended_continuation: multi-segment filename*0*/filename*1* assembles to the fully decoded value (fails before the fix, passes after).
  • test_decode_rfc2231_plain_continuation: plain filename*0/filename*1 continuation stays literal (%20 not decoded).

Full MimeDecodeTest passes (3 tests, 14 assertions); phpstan level 4 on the changed file reports no errors.

@alecpl

alecpl commented Jul 16, 2026

Copy link
Copy Markdown
Member

@MiMoHo adding an entry to the Changelog is problematic because it's a conflict whenever any other entry is added there. So, better is to not include Changelog entry in a PR.

@MiMoHo
MiMoHo force-pushed the held-mime-rfc2231 branch from e451017 to f3ec5f5 Compare July 16, 2026 08:13
@MiMoHo

MiMoHo commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Good point — thanks. I've dropped the Changelog entry from this PR and from my other open PRs, and I'll leave the Changelog additions to you on merge going forward.

@alecpl alecpl added this to the 1.7.3 milestone Jul 25, 2026
@alecpl
alecpl merged commit 87d4c73 into roundcube:master Jul 25, 2026
17 checks passed
alecpl pushed a commit that referenced this pull request Jul 25, 2026
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.

2 participants