Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in validHeaderFieldByte(): characters disallowed in http header field are allowed in email header field #282

Closed
iredmail opened this issue Mar 4, 2023 · 7 comments · Fixed by #283
Assignees

Comments

@iredmail
Copy link
Contributor

iredmail commented Mar 4, 2023

Dear @jhillyerd

There's a critical bug in validHeaderFieldByte() (and more places).
validHeaderFieldByte() checks valid characters for http header, not for email header field:
https://github.com/jhillyerd/enmime/blob/master/header.go#L257

According to RFC 5322 "Internet Message Format", section "2.2 Header fields":

   A field name MUST be composed of printable US-ASCII characters (i.e.,
   characters that have values between 33 and 126, inclusive), except
   colon.

Here's full list of printable US-ASCII characters:
https://www.ascii-code.com/characters/printable-characters

For example, characters /, *, [, ] are allowed in email header field, but it won't pass current validHeaderFieldByte(). This causes parse error like this:

[E] Malformed Header: Header name "X-AAID/CID: 0/238704" contains invalid character '/'

I believe there're more bugs caused by treating email header field as http header field.

@iredmail iredmail changed the title Bug in validHeaderFieldByte(): characters disallowed in http/1.1 are allowed in email header field Bug in validHeaderFieldByte(): characters disallowed in http header field are allowed in email header field Mar 4, 2023
@iredmail
Copy link
Contributor Author

iredmail commented Mar 4, 2023

Reported to upstream too: golang/go#58862

@iredmail
Copy link
Contributor Author

iredmail commented Mar 6, 2023

hi @jhillyerd Any thoughts?

@jhillyerd
Copy link
Owner

jhillyerd commented Mar 6, 2023

My thought is this is a bummer, I was planning on releasing enmime 1.0 next week. 😭

This isn't the first time something like this has happened, as you noted, Go is only focused on the HTTP use-case. It looks like the team put your issue into the backlog, so I think we will need to maintain an internal fork of the header parsing to insulate enmime from this and future changes. Hopefully we can maintain compatibility with textproto.MIMEHeader, if not, now is the time to break it.

I'll look into it this week, but for now I'd recommend sticking with Go 1.19

@jhillyerd jhillyerd self-assigned this Mar 6, 2023
@iredmail
Copy link
Contributor Author

iredmail commented Mar 7, 2023

Do you accept an internal "net/textproto" package with this issue fixed? I already worked out a patch and ready to send for your review.

@jhillyerd
Copy link
Owner

Sure, send a PR!

@iredmail
Copy link
Contributor Author

iredmail commented Mar 7, 2023

hi @jhillyerd

PR is ready, please help review: #283

@iredmail
Copy link
Contributor Author

iredmail commented Mar 7, 2023

There's some issue left: all methods of MIMEHeader (e.g. Add, Set, Get) still uses CanonicalMIMEHeaderKey (for http header) instead of CanonicalEmailMIMEHeaderKey (for email header). is it acceptable to introduce a new type for email header? e.g. type EmailMIMEHeader map[][]string.

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

Successfully merging a pull request may close this issue.

2 participants