fix: assemble RFC 2231 multi-segment filename params with non-UTF-8 charsets - #401
fix: assemble RFC 2231 multi-segment filename params with non-UTF-8 charsets#401troclaux wants to merge 1 commit into
Conversation
|
coverage: 86.41% (+0.01%) from 86.4% — troclaux:fix/issue-109 into jhillyerd:main |
…harsets Go's mime.ParseMediaType only supports UTF-8 and US-ASCII in RFC 2231 charset-encoded continuation parameters. When a Content-Disposition header uses a non-ASCII charset (e.g. EUC-KR) split across multiple filename*N*= segments, stdlib discards all but the last successfully decoded segment, producing a truncated filename. Add assembleRFC2231Params in the mediatype package to detect continuation parameters (name*N*= / name*N=), sort by segment number, concatenate the percent-decoded bytes, convert from the declared charset to UTF-8 using enmime's existing coding package, and replace the N-segment params with a single RFC 2231 UTF-8 encoded parameter before passing to mime.ParseMediaType. Fixes jhillyerd#109
| // order, applies charset conversion, and returns a new string where the continuation parameters | ||
| // are replaced by a single decoded parameter. This is needed because Go's standard | ||
| // mime.ParseMediaType only supports UTF-8 and US-ASCII in RFC 2231 charset-encoded parameters. | ||
| func assembleRFC2231Params(s string) string { |
There was a problem hiding this comment.
s param name isn't very descriptive, maybe use ctype to align with our other funcs
|
|
||
| // ParseWithOptions parses media-type with additional options controlling the parsing behavior. | ||
| // rfc2231SegmentRe matches RFC 2231 continuation parameter names such as "filename*0*" or "filename*1". | ||
| var rfc2231SegmentRe = regexp.MustCompile(`(?i)^([a-zA-Z0-9!#$&\-^_.+]+)\*(\d+)(\*?)$`) |
There was a problem hiding this comment.
Specifying (?i) and both a-z & A-Z is redundant
| mtype: "application/pdf", | ||
| params: map[string]string{"name": "key=value"}, | ||
| }, | ||
| { |
There was a problem hiding this comment.
the code coverage report isn't loading, but these tests look like they just cover the happy path. We should make sure we are triggering some of the failure/continue paths and confirming reasonable output. Doesn't need to be in this particular test func.
There was a problem hiding this comment.
Pull request overview
This PR improves enmime’s parsing of RFC 2231 continuation parameters (e.g. filename*0*= / filename*1*=) so multi-segment filenames using non-UTF-8 charsets (like EUC-KR) are correctly reassembled and converted to UTF-8 before delegating to Go’s mime.ParseMediaType.
Changes:
- Added
assembleRFC2231Paramsto detect/sort/concatenate RFC 2231 continuation segments, charset-convert the byte stream, and re-encode as a singlename*=utf-8''...param prior tomime.ParseMediaType. - Added unit tests to cover EUC-KR and US-ASCII multi-segment filename continuations at the
mediatype.Parselevel. - Added an end-to-end regression test + raw fixture to ensure
(*enmime.Part).FileNameis set correctly from a real multipart message.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mediatype/mediatype.go |
Adds RFC 2231 continuation assembly + charset conversion shim before calling stdlib parsing. |
mediatype/mediatype_test.go |
Adds unit coverage for multi-segment RFC 2231 filename* handling (EUC-KR and ASCII). |
part_test.go |
Adds integration regression test verifying Part.FileName is populated correctly from continuation segments. |
testdata/parts/long-filename-euckr.raw |
Adds fixture email reproducing the segmented EUC-KR filename case from #109. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i, seg := range segs { | ||
| val := seg.value | ||
| if i == 0 { | ||
| // Segment 0 may carry a charset''value prefix per RFC 2231. | ||
| if before, after, found := strings.Cut(val, "''"); found { | ||
| charset = before | ||
| val = after | ||
| } | ||
| } | ||
| if seg.encoded { | ||
| decoded, err := url.PathUnescape(val) | ||
| if err != nil { | ||
| failed = true | ||
| break | ||
| } | ||
| rawBytes = append(rawBytes, decoded...) | ||
| } else { | ||
| rawBytes = append(rawBytes, val...) | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure how we should handle non-contiguous segments, enmime tries to be lenient nowdays... but only respecting charset on seg#0 may be a good idea?
| // rfc2231PercentEncode percent-encodes a UTF-8 string for use in an RFC 2231 parameter value. | ||
| // Only unreserved token characters (alphanumeric and !#$&-^_.+~) are left unencoded. |
|
Thanks for sending this. Sorry for the Copilot review, but I don't spend much time reading Go code nowdays, need all the help I can get. :) I think you may also want to run go fix, the enmime codebase has been "fixed" recently so we are using some of the newer range syntaxes etc. |
Summary
Go's
mime.ParseMediaTypeonly supports UTF-8 and US-ASCII in RFC 2231 charset-encoded continuation parameters. When aContent-Dispositionheader uses a non-ASCII charset (e.g.EUC-KR) split across multiplefilename*N*=segments, stdlib discards all but the last successfully decoded segment, producing a truncated filename likep.zipinstead of the full Korean filename.Changes
assembleRFC2231Paramsinmediatype/mediatype.goto detect RFC 2231 continuation parameters (name*N*=/name*N=), sort them by segment number, concatenate the percent-decoded bytes, and convert from the declared charset to UTF-8 using enmime's existingcoding.ConvertToUTF8String.name*=utf-8''...parameter before being passed tomime.ParseMediaType, which stdlib handles correctly.mediatype/mediatype_test.gofor both EUC-KR and US-ASCII multi-segment cases.part_test.goand test datatestdata/parts/long-filename-euckr.rawto verify theFileNamefield is set correctly end-to-end.Testing
go test ./...)개.txtfrom segmentsfilename*0*=euc-kr''%B0%B3+filename*1*=%2E%74%78%74)Fixes #109