这是indexloc提供的服务，不要输入任何密码
Skip to content

mail: add Header.MessageID and Header.MsgIDList #70

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 6, 2020
Merged

mail: add Header.MessageID and Header.MsgIDList #70

merged 3 commits into from
Feb 6, 2020

Conversation

@emersion
Copy link
Owner

@emersion emersion commented Jan 23, 2020 •
edited
Loading

Those are defined in RFC 5322 section 3.6.4. We need to provide helpers
because the fields may contain CFWS.

We just use mail.ParseAddress and mail.ParseAddressList to avoid
building a msg-id parser. Maybe it's desirable to add a msg-id parser
upstream in net/mail.

Message identifiers are returned without the angle brackets, because the
RFC says:

Semantically, the angle bracket characters are not part of the
msg-id; the msg-id is what is contained between the two angle bracket
characters.

Also, it's easier for the caller to add back the angle brackets if
needed rather than stripping them.

MsgIDList isn't the best name ever. The RFC defines the message-id token
for the Message-ID header field and the msg-id token for message
identifiers, hence this name. I wonder if we can come up with something
better.

Maybe we should add Header.SetMessageID and Header.SetMsgIDList. Maybe
we should remove our existing GenerateMessageID function, and add it to
Header.

References: #49

@codecov-io
Copy link

codecov-io commented Jan 23, 2020 •
edited
Loading

Codecov Report

Merging #70 into master will decrease coverage by 0.68%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
- Coverage   62.54%   61.85%   -0.69%     
==========================================
  Files          15       15              
  Lines         865      991     +126     
==========================================
+ Hits          541      613      +72     
- Misses        279      316      +37     
- Partials       45       62      +17
Impacted Files Coverage Δ
mail/header.go 61.97% <57.14%> (-38.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40e7be6...5a92841. Read the comment docs.

@emersion emersion changed the title mail: add Header.MessageID and Header.MsgIDList [WIP] mail: add Header.MessageID and Header.MsgIDList Jan 23, 2020
@emersion
Copy link
Owner Author

emersion commented Jan 23, 2020

TODO: References and In-Reply-To don't use commas to separate msg-id tokens.

@emersion emersion mentioned this pull request Jan 23, 2020
Open
@foxcpp
Copy link
Collaborator

foxcpp commented Jan 23, 2020

For reference, relevant ABNF fragments from RFC 5322.

   message-id      =   "Message-ID:" msg-id CRLF
   in-reply-to     =   "In-Reply-To:" 1*msg-id CRLF

   msg-id          =   [CFWS] "<" id-left "@" id-right ">" [CFWS]
   id-left         =   dot-atom-text / obs-id-left
   id-right        =   dot-atom-text / no-fold-literal / obs-id-right

   from            =   "From:" mailbox-list CRLF
   sender          =   "Sender:" mailbox CRLF

   mailbox         =   name-addr / addr-spec
   name-addr       =   [display-name] angle-addr
   angle-addr      =   [CFWS] "<" addr-spec ">" [CFWS] /
                       obs-angle-addr

   local-part      =   dot-atom / quoted-string / obs-local-part
   domain          =   dot-atom / domain-literal / obs-domain

   dtext           =   %d33-90 /          ; Printable US-ASCII
                       %d94-126 /         ;  characters not including
                       obs-dtext          ;  "[", "]", or "\"
   domain-literal  =   [CFWS] "[" *([FWS] dtext) [FWS] "]" [CFWS]

   atext           =   ALPHA / DIGIT /    ; Printable US-ASCII
                       "!" / "#" /        ;  characters not including
                       "$" / "%" /        ;  specials.  Used for atoms.
                       "&" / "'" /
                       "*" / "+" /
                       "-" / "/" /
                       "=" / "?" /
                       "^" / "_" /
                       "`" / "{" /
                       "|" / "}" /
                       "~"

   atom            =   [CFWS] 1*atext [CFWS]
   dot-atom-text   =   1*atext *("." 1*atext)
   dot-atom        =   [CFWS] dot-atom-text [CFWS]

@emersion emersion changed the title [WIP] mail: add Header.MessageID and Header.MsgIDList mail: add Header.MessageID and Header.MsgIDList Jan 29, 2020
@emersion
Copy link
Owner Author

emersion commented Jan 29, 2020 •
edited
Loading

Added a headerParser which should follow the RFC's ABNF, except obs-*. It should be easy to integrate the new parser functions in the stdlib if upstream wants it.

@emersion
mail: add headerParser
9c3c48d 
Mostly imported from the standard library's net/mail.addrParser [1], with
parseNoFoldLiteral and parseMsgID added.

[1]: https://tip.golang.org/src/net/mail/message.go
@emersion
mail: add Header.MessageID and Header.MsgIDList
01d6987 
Those are defined in RFC 5322 section 3.6.4. We need to provide helpers
because the fields may contain CFWS.

We use headerParser introduced in the previous commit.

Message identifiers are returned without the angle brackets, because the
RFC says:

> Semantically, the angle bracket characters are not part of the
> msg-id; the msg-id is what is contained between the two angle bracket
> characters.

Also, it's easier for the caller to add back the angle brackets if
needed rather than stripping them.

MsgIDList isn't the best name ever. The RFC defines the message-id token
for the Message-ID header field and the msg-id token for message
identifiers, hence this name. I wonder if we can come up with something
better.

Maybe we should add Header.SetMessageID and Header.SetMsgIDList. Maybe
we should remove our existing GenerateMessageID function, and add it to
Header.

[1]: https://tools.ietf.org/html/rfc5322#section-3.6.4

References: #49
@emersion
mail: add tests for Header.{MessageID,MsgIDList}
5a92841
foxcpp
foxcpp approved these changes Feb 5, 2020
View reviewed changes
Copy link
Collaborator

@foxcpp foxcpp left a comment

Choose a reason for hiding this comment

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

LGTM. I would add some tests with malformed input, though, but not a big deal.

@foxcpp
Copy link
Collaborator

foxcpp commented Feb 5, 2020 •
edited
Loading

MsgIDList isn't the best name ever. The RFC defines the message-id token
for the Message-ID header field and the msg-id token for message
identifiers, hence this name. I wonder if we can come up with something
better.

MsgIDs? MessageIDs? I guess MsgIDList is fine.

Maybe we should add Header.SetMessageID and Header.SetMsgIDList. Maybe
we should remove our existing GenerateMessageID function, and add it to
Header.

Would be a useful "convenience" function indeed.

@emersion emersion merged commit a41879c into master Feb 6, 2020
@emersion emersion deleted the msg-id branch February 6, 2020 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@foxcpp foxcpp foxcpp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@emersion @codecov-io @foxcpp