search

emersion / go-message

✉️ A streaming Go library for the Internet Message Format and mail messages
MIT License
382 stars 111 forks source link

parseHeaderWithParams: keeps the first parameter and remove duplicate ones #141

Closed iredmail closed 2 years ago

iredmail commented 2 years ago

Some mail header may contain duplicate parameter name, for example:

Content-Type: multipart/alternative; boundary="----RTIR0GV26TDW15Q5CVX979LBL6XN18"; boundary="----RTIR0GV26TDW15Q5CVX979LBL6XN18"

go-message reports mime: duplicate parameter name. This PR keeps the first one and remove duplicate ones like below, so that parsing header works well. Note: it doesn't modify original header.

Content-Type: multipart/alternative; boundary="----RTIR0GV26TDW15Q5CVX979LBL6XN18";

It's actually a Go mime package "issue", there's a proposal to handle it, but not accepted due to "Timed out in state WaitingForInfo": golang/go#28618 Also an open one: golang/go#47602

Since it's not that easy to push Go team to "fix" it, I suggest we use this PR as a workaround and make go-message more reliable.

emersion commented 2 years ago

We can't just do string operations like this, the syntax is much more complicated: https://datatracker.ietf.org/doc/html/rfc2045#section-5.1

Can we send a fix to upstream Go as well? Once that's done, I'm OK to merge a fix downstream to accommodate for older Go versions and avoid blocking on the Go team availability.

iredmail commented 2 years ago

The modified function parseHeaderWithParams() is used to parse header with parameters, and this PR focus on key=value parameters, not cover all supported syntax. Is it ok?

I will try to send a fix to upstream, but not sure when it will get some attention or merged. :(

iredmail commented 2 years ago

Will try to figure out a better workaround.

emersion commented 2 years ago

The modified function parseHeaderWithParams() is used to parse header with parameters, and this PR focus on key=value parameters, not cover all supported syntax. Is it ok?

No, it's not okay, because the function will not work for all Content-Type header fields.

I will try to send a fix to upstream, but not sure when it will get some attention or merged. :(

That's fine, the important part is to send a patch upstream. Then we can backport it to go-message.

If nobody sends a patch upstream, upstream will have this issue forever.