From 1c385d5c9b2b80d8fc36f28fb9c89159c3c437df Mon Sep 17 00:00:00 2001 From: Romain Le Jeune Date: Fri, 3 Nov 2023 08:55:01 +0000 Subject: [PATCH] fix(GODT-3087): Exclude attachment content-disposition part when determining... --- pkg/message/parser.go | 10 +- pkg/message/parser/handler.go | 6 +- pkg/message/parser/part.go | 10 +- .../testdata/forwarding_html_attachment.eml | 86 +++++++++ pkg/message/parser/walker.go | 29 ++- pkg/message/parser/walker_test.go | 21 +++ tests/features/smtp/send/html.feature | 167 ++++++++++++++++++ 7 files changed, 321 insertions(+), 8 deletions(-) create mode 100644 pkg/message/parser/testdata/forwarding_html_attachment.eml diff --git a/pkg/message/parser.go b/pkg/message/parser.go index ded02bb6..fce95a81 100644 --- a/pkg/message/parser.go +++ b/pkg/message/parser.go @@ -147,7 +147,7 @@ func parse(p *parser.Parser, allowInvalidAddressLists bool) (Message, error) { m.PlainBody = Body(plainBody) m.MIMEBody = MIMEBody(mimeBody) - mimeType, err := determineMIMEType(p) + mimeType, err := determineBodyMIMEType(p) if err != nil { return Message{}, errors.Wrap(err, "failed to get mime type") } @@ -313,7 +313,7 @@ func collectBodyParts(p *parser.Parser, preferredContentType string) (parser.Par return bestChoice(childParts, preferredContentType), nil }). RegisterRule("text/plain", func(p *parser.Part, visit parser.Visit) (interface{}, error) { - disp, _, err := p.Header.ContentDisposition() + disp, _, err := p.ContentDisposition() if err != nil { disp = "" } @@ -325,7 +325,7 @@ func collectBodyParts(p *parser.Parser, preferredContentType string) (parser.Par return parser.Parts{p}, nil }). RegisterRule("text/html", func(p *parser.Part, visit parser.Visit) (interface{}, error) { - disp, _, err := p.Header.ContentDisposition() + disp, _, err := p.ContentDisposition() if err != nil { disp = "" } @@ -405,7 +405,7 @@ func allPartsHaveContentType(parts parser.Parts, contentType string) bool { return true } -func determineMIMEType(p *parser.Parser) (string, error) { +func determineBodyMIMEType(p *parser.Parser) (string, error) { var isHTML bool w := p.NewWalker(). @@ -414,7 +414,7 @@ func determineMIMEType(p *parser.Parser) (string, error) { return }) - if err := w.Walk(); err != nil { + if err := w.WalkSkipAttachment(); err != nil { return "", err } diff --git a/pkg/message/parser/handler.go b/pkg/message/parser/handler.go index ba566a16..4f719541 100644 --- a/pkg/message/parser/handler.go +++ b/pkg/message/parser/handler.go @@ -32,6 +32,10 @@ func (h *handler) matchPart(p *Part) bool { return h.matchType(p) || h.matchDisp(p) } +func (h *handler) matchPartSkipAttachment(p *Part) bool { + return !p.isAttachment() && h.matchPart(p) +} + func (h *handler) matchType(p *Part) bool { if h.typeRegExp == nil { return false @@ -50,7 +54,7 @@ func (h *handler) matchDisp(p *Part) bool { return false } - disp, _, err := p.Header.ContentDisposition() + disp, _, err := p.ContentDisposition() if err != nil { disp = "" } diff --git a/pkg/message/parser/part.go b/pkg/message/parser/part.go index 1f93bd01..7382a28c 100644 --- a/pkg/message/parser/part.go +++ b/pkg/message/parser/part.go @@ -54,7 +54,7 @@ func (p *Part) ContentType() (string, map[string]string, error) { } func (p *Part) ContentDisposition() (string, map[string]string, error) { - return p.Header.ContentDisposition() + return pmmime.ParseMediaType(p.Header.Get("Content-Disposition")) } func (p *Part) HasContentID() bool { @@ -209,6 +209,14 @@ func (p *Part) isMultipartMixedOrRelated() bool { return t == "multipart/mixed" || t == "multipart/related" } +func (p *Part) isAttachment() bool { + disp, _, err := p.ContentDisposition() + if err != nil { + disp = "" + } + return disp == "attachment" +} + func getContentHeaders(header message.Header) message.Header { var res message.Header diff --git a/pkg/message/parser/testdata/forwarding_html_attachment.eml b/pkg/message/parser/testdata/forwarding_html_attachment.eml new file mode 100644 index 00000000..a26ec6c7 --- /dev/null +++ b/pkg/message/parser/testdata/forwarding_html_attachment.eml @@ -0,0 +1,86 @@ +Content-Type: multipart/mixed; boundary="------------MQ01Z9UM8OaR9z39TvzDfdIq" +Subject: Fwd: Reply to this message, it has various attachments. +References: +To: <[user:user2]@[domain]> +From: <[user:user]@[domain]> +In-Reply-To: +X-Forwarded-Message-Id: + +This is a multi-part message in MIME format. +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: text/plain; charset=UTF-8; format=flowed +Content-Transfer-Encoding: 7bit + +Forwarding a message with various attachments in it! + + + +-------- Forwarded Message -------- +Subject: Reply to this message, it has various attachments. +Date: Thu, 26 Oct 2023 10:41:55 +0000 +From: Gjorgji Testing +Reply-To: Gjorgji Testing +To: Gjorgji Test v3 + + + + +For real! + +*Gjorgji Testing +TesASID +* +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: text/html; charset=UTF-8; name="index.html" +Content-Disposition: attachment; filename="index.html" +Content-Transfer-Encoding: base64 + +IDwhRE9DVFlQRSBodG1sPg0KPGh0bWw+DQo8aGVhZD4NCjx0aXRsZT5QYWdlIFRpdGxlPC90 +aXRsZT4NCjwvaGVhZD4NCjxib2R5Pg0KDQo8aDE+TXkgRmlyc3QgSGVhZGluZzwvaDE+DQo8 +cD5NeSBmaXJzdCBwYXJhZ3JhcGguPC9wPg0KDQo8L2JvZHk+DQo8L2h0bWw+IA== +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: text/xml; charset=UTF-8; name="testxml.xml" +Content-Disposition: attachment; filename="testxml.xml" +Content-Transfer-Encoding: base64 + +PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz4KPCFET0NUWVBFIHN1aXRl +IFNZU1RFTSAiaHR0cDovL3Rlc3RuZy5vcmcvdGVzdG5nLTEuMC5kdGQiID4KCjxzdWl0ZSBu +YW1lPSJBZmZpbGlhdGUgTmV0d29ya3MiPgoKICAgIDx0ZXN0IG5hbWU9IkFmZmlsaWF0ZSBO +ZXR3b3JrcyIgZW5hYmxlZD0idHJ1ZSI+CiAgICAgICAgPGNsYXNzZXM+CiAgICAgICAgICAg +IDxjbGFzcyBuYW1lPSJjb20uY2xpY2tvdXQuYXBpdGVzdGluZy5hZmZOZXR3b3Jrcy5Bd2lu +VUtUZXN0Ii8+CiAgICAgICAgPC9jbGFzc2VzPgogICAgPC90ZXN0PgoKPC9zdWl0ZT4= +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: application/pdf; name="test.pdf" +Content-Disposition: attachment; filename="test.pdf" +Content-Transfer-Encoding: base64 + +JVBERi0xLjUKJeLjz9MKNyAwIG9iago8PAovVHlwZSAvRm9udERlc2NyaXB0b3IKL0ZvbnRO +MjM0NAolJUVPRgo= +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet; + name="test.xlsx" +Content-Disposition: attachment; filename="test.xlsx" +Content-Transfer-Encoding: base64 + +UEsDBBQABgAIAAAAIQBi7p1oXgEAAJAEAAATAAgCW0NvbnRlbnRfVHlwZXNdLnhtbCCiBAIo +UQIAABEAAAAAAAAAAAAAAAAARBcAAGRvY1Byb3BzL2NvcmUueG1sUEsBAi0AFAAGAAgAAAAh +AGFJCRCJAQAAEQMAABAAAAAAAAAAAAAAAAAAvBkAAGRvY1Byb3BzL2FwcC54bWxQSwUGAAAA +AAoACgCAAgAAexwAAAAA +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document; + name="test.docx" +Content-Disposition: attachment; filename="test.docx" +Content-Transfer-Encoding: base64 + +UEsDBBQABgAIAAAAIQDfpNJsWgEAACAFAAATAAgCW0NvbnRlbnRfVHlwZXNdLnhtbCCiBAIo +cHAueG1sUEsBAi0AFAAGAAgAAAAhABA0tG9uAQAA4QIAABEAAAAAAAAAAAAAAAAA2xsAAGRv +Y1Byb3BzL2NvcmUueG1sUEsBAi0AFAAGAAgAAAAhAJ/mlBIqCwAAU3AAAA8AAAAAAAAAAAAA +AAAAgB4AAHdvcmQvc3R5bGVzLnhtbFBLBQYAAAAACwALAMECAADXKQAAAAA= +--------------MQ01Z9UM8OaR9z39TvzDfdIq +Content-Type: text/plain; charset=UTF-8; name="text file.txt" +Content-Disposition: attachment; filename="text file.txt" +Content-Transfer-Encoding: base64 + +dGV4dCBmaWxl + +--------------MQ01Z9UM8OaR9z39TvzDfdIq-- diff --git a/pkg/message/parser/walker.go b/pkg/message/parser/walker.go index 232cb0e3..3d489dd8 100644 --- a/pkg/message/parser/walker.go +++ b/pkg/message/parser/walker.go @@ -33,9 +33,12 @@ func newWalker(root *Part) *Walker { } } -func (w *Walker) Walk() (err error) { +func (w *Walker) Walk() error { return w.walkOverPart(w.root) } +func (w *Walker) WalkSkipAttachment() error { + return w.walkOverPartSkipAttachment(w.root) +} func (w *Walker) walkOverPart(p *Part) error { if err := w.getHandlerFunc(p)(p); err != nil { @@ -51,6 +54,20 @@ func (w *Walker) walkOverPart(p *Part) error { return nil } +func (w *Walker) walkOverPartSkipAttachment(p *Part) error { + if err := w.getHandlerFuncSkipAttachment(p)(p); err != nil { + return err + } + + for _, child := range p.children { + if err := w.walkOverPartSkipAttachment(child); err != nil { + return err + } + } + + return nil +} + // RegisterDefaultHandler registers a handler that will be called on every part // that doesn't match a registered content type/disposition handler. func (w *Walker) RegisterDefaultHandler(fn HandlerFunc) *Walker { @@ -91,3 +108,13 @@ func (w *Walker) getHandlerFunc(p *Part) HandlerFunc { return w.defaultHandler } + +func (w *Walker) getHandlerFuncSkipAttachment(p *Part) HandlerFunc { + for _, handler := range w.handlers { + if handler.matchPartSkipAttachment(p) { + return handler.fn + } + } + + return w.defaultHandler +} diff --git a/pkg/message/parser/walker_test.go b/pkg/message/parser/walker_test.go index a159805f..692339b9 100644 --- a/pkg/message/parser/walker_test.go +++ b/pkg/message/parser/walker_test.go @@ -60,6 +60,27 @@ func TestWalkerTypeHandler(t *testing.T) { }, html) } +func TestWalkerTypeHandler_excludingAttachment(t *testing.T) { + p := newTestParser(t, "forwarding_html_attachment.eml") + + html := [][]byte{} + plain := [][]byte{} + + walker := p.NewWalker(). + RegisterContentTypeHandler("text/html", func(p *Part) (err error) { + html = append(html, p.Body) + return + }). + RegisterContentTypeHandler("text/plain", func(p *Part) (err error) { + plain = append(plain, p.Body) + return + }) + + assert.NoError(t, walker.WalkSkipAttachment()) + assert.Equal(t, 1, len(plain)) + assert.Equal(t, 0, len(html)) +} + func TestWalkerDispositionHandler(t *testing.T) { p := newTestParser(t, "text_html_octet_attachment.eml") diff --git a/tests/features/smtp/send/html.feature b/tests/features/smtp/send/html.feature index 663b61f9..7016f41a 100644 --- a/tests/features/smtp/send/html.feature +++ b/tests/features/smtp/send/html.feature @@ -480,3 +480,170 @@ Feature: SMTP sending of plain messages } } """ + +Scenario: Forward a message containing various attachments + When SMTP client "1" sends the following message from "[user:user]@[domain]" to "[user:user2]@[domain]": + """ + Content-Type: multipart/mixed; boundary="------------MQ01Z9UM8OaR9z39TvzDfdIq" + Subject: Fwd: Reply to this message, it has various attachments. + References: + To: <[user:user2]@[domain]> + From: <[user:user]@[domain]> + In-Reply-To: + X-Forwarded-Message-Id: + + This is a multi-part message in MIME format. + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: text/plain; charset=UTF-8; format=flowed + Content-Transfer-Encoding: 7bit + + Forwarding a message with various attachments in it! + + + + -------- Forwarded Message -------- + Subject: Reply to this message, it has various attachments. + Date: Thu, 26 Oct 2023 10:41:55 +0000 + From: Gjorgji Testing + Reply-To: Gjorgji Testing + To: Gjorgji Test v3 + + + + + For real! + + *Gjorgji Testing + TesASID + * + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: text/html; charset=UTF-8; name="index.html" + Content-Disposition: attachment; filename="index.html" + Content-Transfer-Encoding: base64 + + IDwhRE9DVFlQRSBodG1sPg0KPGh0bWw+DQo8aGVhZD4NCjx0aXRsZT5QYWdlIFRpdGxlPC90 + aXRsZT4NCjwvaGVhZD4NCjxib2R5Pg0KDQo8aDE+TXkgRmlyc3QgSGVhZGluZzwvaDE+DQo8 + cD5NeSBmaXJzdCBwYXJhZ3JhcGguPC9wPg0KDQo8L2JvZHk+DQo8L2h0bWw+IA== + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: text/xml; charset=UTF-8; name="testxml.xml" + Content-Disposition: attachment; filename="testxml.xml" + Content-Transfer-Encoding: base64 + + PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iVVRGLTgiPz4KPCFET0NUWVBFIHN1aXRl + IFNZU1RFTSAiaHR0cDovL3Rlc3RuZy5vcmcvdGVzdG5nLTEuMC5kdGQiID4KCjxzdWl0ZSBu + YW1lPSJBZmZpbGlhdGUgTmV0d29ya3MiPgoKICAgIDx0ZXN0IG5hbWU9IkFmZmlsaWF0ZSBO + ZXR3b3JrcyIgZW5hYmxlZD0idHJ1ZSI+CiAgICAgICAgPGNsYXNzZXM+CiAgICAgICAgICAg + IDxjbGFzcyBuYW1lPSJjb20uY2xpY2tvdXQuYXBpdGVzdGluZy5hZmZOZXR3b3Jrcy5Bd2lu + VUtUZXN0Ii8+CiAgICAgICAgPC9jbGFzc2VzPgogICAgPC90ZXN0PgoKPC9zdWl0ZT4= + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: application/pdf; name="test.pdf" + Content-Disposition: attachment; filename="test.pdf" + Content-Transfer-Encoding: base64 + + JVBERi0xLjUKJeLjz9MKNyAwIG9iago8PAovVHlwZSAvRm9udERlc2NyaXB0b3IKL0ZvbnRO + MjM0NAolJUVPRgo= + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet; + name="test.xlsx" + Content-Disposition: attachment; filename="test.xlsx" + Content-Transfer-Encoding: base64 + + UEsDBBQABgAIAAAAIQBi7p1oXgEAAJAEAAATAAgCW0NvbnRlbnRfVHlwZXNdLnhtbCCiBAIo + UQIAABEAAAAAAAAAAAAAAAAARBcAAGRvY1Byb3BzL2NvcmUueG1sUEsBAi0AFAAGAAgAAAAh + AGFJCRCJAQAAEQMAABAAAAAAAAAAAAAAAAAAvBkAAGRvY1Byb3BzL2FwcC54bWxQSwUGAAAA + AAoACgCAAgAAexwAAAAA + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document; + name="test.docx" + Content-Disposition: attachment; filename="test.docx" + Content-Transfer-Encoding: base64 + + UEsDBBQABgAIAAAAIQDfpNJsWgEAACAFAAATAAgCW0NvbnRlbnRfVHlwZXNdLnhtbCCiBAIo + cHAueG1sUEsBAi0AFAAGAAgAAAAhABA0tG9uAQAA4QIAABEAAAAAAAAAAAAAAAAA2xsAAGRv + Y1Byb3BzL2NvcmUueG1sUEsBAi0AFAAGAAgAAAAhAJ/mlBIqCwAAU3AAAA8AAAAAAAAAAAAA + AAAAgB4AAHdvcmQvc3R5bGVzLnhtbFBLBQYAAAAACwALAMECAADXKQAAAAA= + --------------MQ01Z9UM8OaR9z39TvzDfdIq + Content-Type: text/plain; charset=UTF-8; name="text file.txt" + Content-Disposition: attachment; filename="text file.txt" + Content-Transfer-Encoding: base64 + + dGV4dCBmaWxl + + --------------MQ01Z9UM8OaR9z39TvzDfdIq-- + + """ + Then it succeeds + When user "[user:user]" connects and authenticates IMAP client "1" + Then IMAP client "1" eventually sees the following messages in "Sent": + | from | to | subject | X-Forwarded-Message-Id | + | [user:user]@[domain] | [user:user2]@[domain] | Fwd: Reply to this message, it has various attachments. | something@protonmail.ch | + And IMAP client "1" eventually sees 1 messages in "Sent" + When the user logs in with username "[user:user2]" and password "password" + And user "[user:user2]" connects and authenticates IMAP client "2" + And user "[user:user2]" finishes syncing + And it succeeds + Then IMAP client "2" eventually sees the following messages in "Inbox": + | from | to | subject | X-Forwarded-Message-Id | + | [user:user]@[domain] | [user:user2]@[domain] | Fwd: Reply to this message, it has various attachments. | something@protonmail.ch | + Then IMAP client "2" eventually sees the following message in "Inbox" with this structure: + """ + { + "from": "[user:user]@[domain]", + "to": "[user:user2]@[domain]", + "subject": "Fwd: Reply to this message, it has various attachments.", + "content": { + "content-type": "multipart/mixed", + "sections":[ + { + "content-type": "text/plain", + "content-type-charset": "utf-8", + "transfer-encoding": "quoted-printable", + "body-is": "Forwarding a message with various attachments in it!\r\n\r\n\r\n\r\n-------- Forwarded Message --------\r\nSubject: \tReply to this message, it has various attachments.\r\nDate: \tThu, 26 Oct 2023 10:41:55 +0000\r\nFrom: \tGjorgji Testing \r\nReply-To: \tGjorgji Testing \r\nTo: \tGjorgji Test v3 \r\n\r\n\r\n\r\n\r\nFor real!\r\n\r\n*Gjorgji Testing\r\nTesASID \r\n*" + }, + { + "content-type": "text/html", + "content-type-name": "index.html", + "content-disposition": "attachment", + "content-disposition-filename": "index.html", + "transfer-encoding": "base64" + }, + { + "content-type": "application/vnd.openxmlformats-officedocument.wordprocessingml.document", + "content-type-name": "test.docx", + "content-disposition": "attachment", + "content-disposition-filename": "test.docx", + "transfer-encoding": "base64" + }, + { + "content-type": "application/pdf", + "content-type-name": "test.pdf", + "content-disposition": "attachment", + "content-disposition-filename": "test.pdf", + "transfer-encoding": "base64" + }, + { + "content-type": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "content-type-name": "test.xlsx", + "content-disposition": "attachment", + "content-disposition-filename": "test.xlsx", + "transfer-encoding": "base64" + }, + { + "content-type": "text/xml", + "content-type-name": "testxml.xml", + "content-disposition": "attachment", + "content-disposition-filename": "testxml.xml", + "transfer-encoding": "base64" + }, + { + "content-type": "text/plain", + "content-type-name": "text file.txt", + "content-disposition": "attachment", + "content-disposition-filename": "text file.txt", + "transfer-encoding": "base64", + "body-is": "dGV4dCBmaWxl" + } + ] + } + } + """ \ No newline at end of file