From 263f6bb3ded0fb2a2786d103be659e509b830953 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 12:43:01 +0100 Subject: [PATCH 01/18] Refactor SMTP client code for better readability The variable names in the code related to the I/O of the SMTP client have been clarified for improved readability and comprehension. For example, unclear variable names like `d` and `w` have been replaced with more meaningful names like `depth` and `writer`. The same naming improvements have also been applied to function parameters. This update aims to enhance code maintenance and simplify future development processes. --- msg.go | 8 +- msgwriter.go | 282 +++++++++++++++++++++++----------------------- msgwriter_test.go | 8 +- 3 files changed, 151 insertions(+), 147 deletions(-) diff --git a/msg.go b/msg.go index 0efd043..c740450 100644 --- a/msg.go +++ b/msg.go @@ -933,9 +933,9 @@ func (m *Msg) applyMiddlewares(ms *Msg) *Msg { // WriteTo writes the formated Msg into a give io.Writer and satisfies the io.WriteTo interface func (m *Msg) WriteTo(w io.Writer) (int64, error) { - mw := &msgWriter{w: w, c: m.charset, en: m.encoder} + mw := &msgWriter{writer: w, charset: m.charset, encoder: m.encoder} mw.writeMsg(m.applyMiddlewares(m)) - return mw.n, mw.err + return mw.bytesWritten, mw.err } // WriteToSkipMiddleware writes the formated Msg into a give io.Writer and satisfies @@ -950,10 +950,10 @@ func (m *Msg) WriteToSkipMiddleware(w io.Writer, mt MiddlewareType) (int64, erro mwl = append(mwl, m.middlewares[i]) } m.middlewares = mwl - mw := &msgWriter{w: w, c: m.charset, en: m.encoder} + mw := &msgWriter{writer: w, charset: m.charset, encoder: m.encoder} mw.writeMsg(m.applyMiddlewares(m)) m.middlewares = omwl - return mw.n, mw.err + return mw.bytesWritten, mw.err } // Write is an alias method to WriteTo due to compatibility reasons diff --git a/msgwriter.go b/msgwriter.go index b2ff118..696c080 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -35,237 +35,241 @@ const DoubleNewLine = "\r\n\r\n" // msgWriter handles the I/O to the io.WriteCloser of the SMTP client type msgWriter struct { - c Charset - d int8 - en mime.WordEncoder - err error - mpw [3]*multipart.Writer - n int64 - pw io.Writer - w io.Writer + bytesWritten int64 + charset Charset + depth int8 + encoder mime.WordEncoder + err error + multiPartWriter [3]*multipart.Writer + partWriter io.Writer + writer io.Writer } // Write implements the io.Writer interface for msgWriter -func (mw *msgWriter) Write(p []byte) (int, error) { +func (mw *msgWriter) Write(payload []byte) (int, error) { if mw.err != nil { return 0, fmt.Errorf("failed to write due to previous error: %w", mw.err) } var n int - n, mw.err = mw.w.Write(p) - mw.n += int64(n) + n, mw.err = mw.writer.Write(payload) + mw.bytesWritten += int64(n) return n, mw.err } // writeMsg formats the message and sends it to its io.Writer -func (mw *msgWriter) writeMsg(m *Msg) { - m.addDefaultHeader() - m.checkUserAgent() - mw.writeGenHeader(m) - mw.writePreformattedGenHeader(m) +func (mw *msgWriter) writeMsg(msg *Msg) { + msg.addDefaultHeader() + msg.checkUserAgent() + mw.writeGenHeader(msg) + mw.writePreformattedGenHeader(msg) // Set the FROM header (or envelope FROM if FROM is empty) - hf := true - f, ok := m.addrHeader[HeaderFrom] - if !ok || (len(f) == 0 || f == nil) { - f, ok = m.addrHeader[HeaderEnvelopeFrom] - if !ok || (len(f) == 0 || f == nil) { - hf = false + hasFrom := true + from, ok := msg.addrHeader[HeaderFrom] + if !ok || (len(from) == 0 || from == nil) { + from, ok = msg.addrHeader[HeaderEnvelopeFrom] + if !ok || (len(from) == 0 || from == nil) { + hasFrom = false } } - if hf && (len(f) > 0 && f[0] != nil) { - mw.writeHeader(Header(HeaderFrom), f[0].String()) + if hasFrom && (len(from) > 0 && from[0] != nil) { + mw.writeHeader(Header(HeaderFrom), from[0].String()) } // Set the rest of the address headers - for _, t := range []AddrHeader{HeaderTo, HeaderCc} { - if al, ok := m.addrHeader[t]; ok { - var v []string - for _, a := range al { - v = append(v, a.String()) + for _, to := range []AddrHeader{HeaderTo, HeaderCc} { + if addresses, ok := msg.addrHeader[to]; ok { + var val []string + for _, addr := range addresses { + val = append(val, addr.String()) } - mw.writeHeader(Header(t), v...) + mw.writeHeader(Header(to), val...) } } - if m.hasMixed() { - mw.startMP("mixed", m.boundary) + if msg.hasMixed() { + mw.startMP("mixed", msg.boundary) mw.writeString(DoubleNewLine) } - if m.hasRelated() { - mw.startMP("related", m.boundary) + if msg.hasRelated() { + mw.startMP("related", msg.boundary) mw.writeString(DoubleNewLine) } - if m.hasAlt() { - mw.startMP(MIMEAlternative, m.boundary) + if msg.hasAlt() { + mw.startMP(MIMEAlternative, msg.boundary) mw.writeString(DoubleNewLine) } - if m.hasPGPType() { - switch m.pgptype { + if msg.hasPGPType() { + switch msg.pgptype { case PGPEncrypt: - mw.startMP(`encrypted; protocol="application/pgp-encrypted"`, m.boundary) + mw.startMP(`encrypted; protocol="application/pgp-encrypted"`, + msg.boundary) case PGPSignature: - mw.startMP(`signed; protocol="application/pgp-signature";`, m.boundary) + mw.startMP(`signed; protocol="application/pgp-signature";`, + msg.boundary) + default: } mw.writeString(DoubleNewLine) } - for _, p := range m.parts { - if !p.del { - mw.writePart(p, m.charset) + for _, part := range msg.parts { + if !part.del { + mw.writePart(part, msg.charset) } } - if m.hasAlt() { + if msg.hasAlt() { mw.stopMP() } // Add embeds - mw.addFiles(m.embeds, false) - if m.hasRelated() { + mw.addFiles(msg.embeds, false) + if msg.hasRelated() { mw.stopMP() } // Add attachments - mw.addFiles(m.attachments, true) - if m.hasMixed() { + mw.addFiles(msg.attachments, true) + if msg.hasMixed() { mw.stopMP() } } // writeGenHeader writes out all generic headers to the msgWriter -func (mw *msgWriter) writeGenHeader(m *Msg) { - gk := make([]string, 0, len(m.genHeader)) - for k := range m.genHeader { - gk = append(gk, string(k)) +func (mw *msgWriter) writeGenHeader(msg *Msg) { + keys := make([]string, 0, len(msg.genHeader)) + for key := range msg.genHeader { + keys = append(keys, string(key)) } - sort.Strings(gk) - for _, k := range gk { - mw.writeHeader(Header(k), m.genHeader[Header(k)]...) + sort.Strings(keys) + for _, key := range keys { + mw.writeHeader(Header(key), msg.genHeader[Header(key)]...) } } // writePreformatedHeader writes out all preformated generic headers to the msgWriter -func (mw *msgWriter) writePreformattedGenHeader(m *Msg) { - for k, v := range m.preformHeader { - mw.writeString(fmt.Sprintf("%s: %s%s", k, v, SingleNewLine)) +func (mw *msgWriter) writePreformattedGenHeader(msg *Msg) { + for key, val := range msg.preformHeader { + mw.writeString(fmt.Sprintf("%s: %s%s", key, val, SingleNewLine)) } } // startMP writes a multipart beginning -func (mw *msgWriter) startMP(mt MIMEType, b string) { - mp := multipart.NewWriter(mw) - if b != "" { - mw.err = mp.SetBoundary(b) +func (mw *msgWriter) startMP(mimeType MIMEType, boundary string) { + multiPartWriter := multipart.NewWriter(mw) + if boundary != "" { + mw.err = multiPartWriter.SetBoundary(boundary) } - ct := fmt.Sprintf("multipart/%s;\r\n boundary=%s", mt, mp.Boundary()) - mw.mpw[mw.d] = mp + contentType := fmt.Sprintf("multipart/%s;\r\n boundary=%s", mimeType, + multiPartWriter.Boundary()) + mw.multiPartWriter[mw.depth] = multiPartWriter - if mw.d == 0 { - mw.writeString(fmt.Sprintf("%s: %s", HeaderContentType, ct)) + if mw.depth == 0 { + mw.writeString(fmt.Sprintf("%s: %s", HeaderContentType, contentType)) } - if mw.d > 0 { - mw.newPart(map[string][]string{"Content-Type": {ct}}) + if mw.depth > 0 { + mw.newPart(map[string][]string{"Content-Type": {contentType}}) } - mw.d++ + mw.depth++ } // stopMP closes the multipart func (mw *msgWriter) stopMP() { - if mw.d > 0 { - mw.err = mw.mpw[mw.d-1].Close() - mw.d-- + if mw.depth > 0 { + mw.err = mw.multiPartWriter[mw.depth-1].Close() + mw.depth-- } } // addFiles adds the attachments/embeds file content to the mail body -func (mw *msgWriter) addFiles(fl []*File, a bool) { - for _, f := range fl { - e := EncodingB64 - if _, ok := f.getHeader(HeaderContentType); !ok { - mt := mime.TypeByExtension(filepath.Ext(f.Name)) - if mt == "" { - mt = "application/octet-stream" +func (mw *msgWriter) addFiles(files []*File, isAttachment bool) { + for _, file := range files { + encoding := EncodingB64 + if _, ok := file.getHeader(HeaderContentType); !ok { + mimeType := mime.TypeByExtension(filepath.Ext(file.Name)) + if mimeType == "" { + mimeType = "application/octet-stream" } - if f.ContentType != "" { - mt = string(f.ContentType) + if file.ContentType != "" { + mimeType = string(file.ContentType) } - f.setHeader(HeaderContentType, fmt.Sprintf(`%s; name="%s"`, mt, - mw.en.Encode(mw.c.String(), f.Name))) + file.setHeader(HeaderContentType, fmt.Sprintf(`%s; name="%s"`, mimeType, + mw.encoder.Encode(mw.charset.String(), file.Name))) } - if _, ok := f.getHeader(HeaderContentTransferEnc); !ok { - if f.Enc != "" { - e = f.Enc + if _, ok := file.getHeader(HeaderContentTransferEnc); !ok { + if file.Enc != "" { + encoding = file.Enc } - f.setHeader(HeaderContentTransferEnc, string(e)) + file.setHeader(HeaderContentTransferEnc, string(encoding)) } - if f.Desc != "" { - if _, ok := f.getHeader(HeaderContentDescription); !ok { - f.setHeader(HeaderContentDescription, f.Desc) + if file.Desc != "" { + if _, ok := file.getHeader(HeaderContentDescription); !ok { + file.setHeader(HeaderContentDescription, file.Desc) } } - if _, ok := f.getHeader(HeaderContentDisposition); !ok { - d := "inline" - if a { - d = "attachment" + if _, ok := file.getHeader(HeaderContentDisposition); !ok { + disposition := "inline" + if isAttachment { + disposition = "attachment" } - f.setHeader(HeaderContentDisposition, fmt.Sprintf(`%s; filename="%s"`, d, - mw.en.Encode(mw.c.String(), f.Name))) + file.setHeader(HeaderContentDisposition, fmt.Sprintf(`%s; filename="%s"`, + disposition, mw.encoder.Encode(mw.charset.String(), file.Name))) } - if !a { - if _, ok := f.getHeader(HeaderContentID); !ok { - f.setHeader(HeaderContentID, fmt.Sprintf("<%s>", f.Name)) + if !isAttachment { + if _, ok := file.getHeader(HeaderContentID); !ok { + file.setHeader(HeaderContentID, fmt.Sprintf("<%s>", file.Name)) } } - if mw.d == 0 { - for h, v := range f.Header { - mw.writeHeader(Header(h), v...) + if mw.depth == 0 { + for header, val := range file.Header { + mw.writeHeader(Header(header), val...) } mw.writeString(SingleNewLine) } - if mw.d > 0 { - mw.newPart(f.Header) + if mw.depth > 0 { + mw.newPart(file.Header) } if mw.err == nil { - mw.writeBody(f.Writer, e) + mw.writeBody(file.Writer, encoding) } } } // newPart creates a new MIME multipart io.Writer and sets the partwriter to it -func (mw *msgWriter) newPart(h map[string][]string) { - mw.pw, mw.err = mw.mpw[mw.d-1].CreatePart(h) +func (mw *msgWriter) newPart(header map[string][]string) { + mw.partWriter, mw.err = mw.multiPartWriter[mw.depth-1].CreatePart(header) } // writePart writes the corresponding part to the Msg body -func (mw *msgWriter) writePart(p *Part, cs Charset) { - pcs := p.cset - if pcs.String() == "" { - pcs = cs +func (mw *msgWriter) writePart(part *Part, charset Charset) { + partCharset := part.cset + if partCharset.String() == "" { + partCharset = charset } - ct := fmt.Sprintf("%s; charset=%s", p.ctype, pcs) - cte := p.enc.String() - if mw.d == 0 { - mw.writeHeader(HeaderContentType, ct) - mw.writeHeader(HeaderContentTransferEnc, cte) + contentType := fmt.Sprintf("%s; charset=%s", part.ctype, partCharset) + contentTransferEnc := part.enc.String() + if mw.depth == 0 { + mw.writeHeader(HeaderContentType, contentType) + mw.writeHeader(HeaderContentTransferEnc, contentTransferEnc) mw.writeString(SingleNewLine) } - if mw.d > 0 { - mh := textproto.MIMEHeader{} - if p.desc != "" { - mh.Add(string(HeaderContentDescription), p.desc) + if mw.depth > 0 { + mimeHeader := textproto.MIMEHeader{} + if part.desc != "" { + mimeHeader.Add(string(HeaderContentDescription), part.desc) } - mh.Add(string(HeaderContentType), ct) - mh.Add(string(HeaderContentTransferEnc), cte) - mw.newPart(mh) + mimeHeader.Add(string(HeaderContentType), contentType) + mimeHeader.Add(string(HeaderContentTransferEnc), contentTransferEnc) + mw.newPart(mimeHeader) } - mw.writeBody(p.w, p.enc) + mw.writeBody(part.w, part.enc) } // writeString writes a string into the msgWriter's io.Writer interface @@ -274,24 +278,24 @@ func (mw *msgWriter) writeString(s string) { return } var n int - n, mw.err = io.WriteString(mw.w, s) - mw.n += int64(n) + n, mw.err = io.WriteString(mw.writer, s) + mw.bytesWritten += int64(n) } // writeHeader writes a header into the msgWriter's io.Writer -func (mw *msgWriter) writeHeader(k Header, vl ...string) { +func (mw *msgWriter) writeHeader(key Header, values ...string) { wbuf := bytes.Buffer{} cl := MaxHeaderLength - 2 - wbuf.WriteString(string(k)) - cl -= len(k) - if len(vl) == 0 { + wbuf.WriteString(string(key)) + cl -= len(key) + if len(values) == 0 { wbuf.WriteString(":\r\n") return } wbuf.WriteString(": ") cl -= 2 - fs := strings.Join(vl, ", ") + fs := strings.Join(values, ", ") sfs := strings.Split(fs, " ") for i, v := range sfs { if cl-len(v) <= 1 { @@ -318,11 +322,11 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { var ew io.WriteCloser var n int64 var err error - if mw.d == 0 { - w = mw.w + if mw.depth == 0 { + w = mw.writer } - if mw.d > 0 { - w = mw.pw + if mw.depth > 0 { + w = mw.partWriter } wbuf := bytes.Buffer{} lb := Base64LineBreaker{} @@ -342,8 +346,8 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { if err != nil && mw.err == nil { mw.err = fmt.Errorf("bodyWriter io.Copy: %w", err) } - if mw.d == 0 { - mw.n += n + if mw.depth == 0 { + mw.bytesWritten += n } return default: @@ -369,7 +373,7 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { // Since the part writer uses the WriteTo() method, we don't need to add the // bytes twice - if mw.d == 0 { - mw.n += n + if mw.depth == 0 { + mw.bytesWritten += n } } diff --git a/msgwriter_test.go b/msgwriter_test.go index 4f61c6f..e6582fb 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -28,7 +28,7 @@ func (bw *brokenWriter) Write([]byte) (int, error) { // TestMsgWriter_Write tests the WriteTo() method of the msgWriter func TestMsgWriter_Write(t *testing.T) { bw := &brokenWriter{} - mw := &msgWriter{w: bw, c: CharsetUTF8, en: mime.QEncoding} + mw := &msgWriter{writer: bw, charset: CharsetUTF8, encoder: mime.QEncoding} _, err := mw.Write([]byte("test")) if err == nil { t.Errorf("msgWriter WriteTo() with brokenWriter should fail, but didn't") @@ -55,7 +55,7 @@ func TestMsgWriter_writeMsg(t *testing.T) { m.SetBodyString(TypeTextPlain, "This is the body") m.AddAlternativeString(TypeTextHTML, "This is the alternative body") buf := bytes.Buffer{} - mw := &msgWriter{w: &buf, c: CharsetUTF8, en: mime.QEncoding} + mw := &msgWriter{writer: &buf, charset: CharsetUTF8, encoder: mime.QEncoding} mw.writeMsg(m) ms := buf.String() @@ -134,7 +134,7 @@ func TestMsgWriter_writeMsg_PGP(t *testing.T) { m.Subject("This is a subject") m.SetBodyString(TypeTextPlain, "This is the body") buf := bytes.Buffer{} - mw := &msgWriter{w: &buf, c: CharsetUTF8, en: mime.QEncoding} + mw := &msgWriter{writer: &buf, charset: CharsetUTF8, encoder: mime.QEncoding} mw.writeMsg(m) ms := buf.String() if !strings.Contains(ms, `encrypted; protocol="application/pgp-encrypted"`) { @@ -147,7 +147,7 @@ func TestMsgWriter_writeMsg_PGP(t *testing.T) { m.Subject("This is a subject") m.SetBodyString(TypeTextPlain, "This is the body") buf = bytes.Buffer{} - mw = &msgWriter{w: &buf, c: CharsetUTF8, en: mime.QEncoding} + mw = &msgWriter{writer: &buf, charset: CharsetUTF8, encoder: mime.QEncoding} mw.writeMsg(m) ms = buf.String() if !strings.Contains(ms, `signed; protocol="application/pgp-signature"`) { From c126670f7060928c9532f4ee8c0a08d1b010cf73 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 17:38:42 +0100 Subject: [PATCH 02/18] Refactor variable and function names in msgWriter The commit includes changes such as renaming the variables `wbuf` to `buffer`, `cl` to `charLength` and functions `f` to `writeFunc` in msgWriter. This refactoring makes the code easier to read and understand. All the changes are aimed at enhancing code clarity without altering underlying logic or functionality. --- msgwriter.go | 79 ++++++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/msgwriter.go b/msgwriter.go index 696c080..e584a99 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -284,65 +284,66 @@ func (mw *msgWriter) writeString(s string) { // writeHeader writes a header into the msgWriter's io.Writer func (mw *msgWriter) writeHeader(key Header, values ...string) { - wbuf := bytes.Buffer{} - cl := MaxHeaderLength - 2 - wbuf.WriteString(string(key)) - cl -= len(key) + buffer := strings.Builder{} + charLength := MaxHeaderLength - 2 + buffer.WriteString(string(key)) + charLength -= len(key) if len(values) == 0 { - wbuf.WriteString(":\r\n") + buffer.WriteString(":\r\n") return } - wbuf.WriteString(": ") - cl -= 2 + buffer.WriteString(": ") + charLength -= 2 - fs := strings.Join(values, ", ") - sfs := strings.Split(fs, " ") - for i, v := range sfs { - if cl-len(v) <= 1 { - wbuf.WriteString(fmt.Sprintf("%s ", SingleNewLine)) - cl = MaxHeaderLength - 3 + fullValueStr := strings.Join(values, ", ") + words := strings.Split(fullValueStr, " ") + for i, val := range words { + if charLength-len(val) <= 1 { + buffer.WriteString(fmt.Sprintf("%s ", SingleNewLine)) + charLength = MaxHeaderLength - 3 } - wbuf.WriteString(v) - if i < len(sfs)-1 { - wbuf.WriteString(" ") - cl -= 1 + buffer.WriteString(val) + if i < len(words)-1 { + buffer.WriteString(" ") + charLength -= 1 } - cl -= len(v) + charLength -= len(val) } - bufs := wbuf.String() - bufs = strings.ReplaceAll(bufs, fmt.Sprintf(" %s", SingleNewLine), SingleNewLine) - mw.writeString(bufs) + bufferString := buffer.String() + bufferString = strings.ReplaceAll(bufferString, fmt.Sprintf(" %s", SingleNewLine), + SingleNewLine) + mw.writeString(bufferString) mw.writeString("\r\n") } // writeBody writes an io.Reader into an io.Writer using provided Encoding -func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { - var w io.Writer - var ew io.WriteCloser +func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding) { + var writer io.Writer + var encodedWriter io.WriteCloser var n int64 var err error if mw.depth == 0 { - w = mw.writer + writer = mw.writer } if mw.depth > 0 { - w = mw.partWriter + writer = mw.partWriter } - wbuf := bytes.Buffer{} - lb := Base64LineBreaker{} - lb.out = &wbuf + writeBuffer := bytes.Buffer{} + lineBreaker := Base64LineBreaker{} + lineBreaker.out = &writeBuffer - switch e { + switch encoding { case EncodingQP: - ew = quotedprintable.NewWriter(&wbuf) + encodedWriter = quotedprintable.NewWriter(&writeBuffer) case EncodingB64: - ew = base64.NewEncoder(base64.StdEncoding, &lb) + encodedWriter = base64.NewEncoder(base64.StdEncoding, &lineBreaker) case NoEncoding: - _, err = f(&wbuf) + _, err = writeFunc(&writeBuffer) if err != nil { mw.err = fmt.Errorf("bodyWriter function: %w", err) } - n, err = io.Copy(w, &wbuf) + n, err = io.Copy(writer, &writeBuffer) if err != nil && mw.err == nil { mw.err = fmt.Errorf("bodyWriter io.Copy: %w", err) } @@ -351,22 +352,22 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { } return default: - ew = quotedprintable.NewWriter(w) + encodedWriter = quotedprintable.NewWriter(writer) } - _, err = f(ew) + _, err = writeFunc(encodedWriter) if err != nil { mw.err = fmt.Errorf("bodyWriter function: %w", err) } - err = ew.Close() + err = encodedWriter.Close() if err != nil && mw.err == nil { mw.err = fmt.Errorf("bodyWriter close encoded writer: %w", err) } - err = lb.Close() + err = lineBreaker.Close() if err != nil && mw.err == nil { mw.err = fmt.Errorf("bodyWriter close linebreaker: %w", err) } - n, err = io.Copy(w, &wbuf) + n, err = io.Copy(writer, &writeBuffer) if err != nil && mw.err == nil { mw.err = fmt.Errorf("bodyWriter io.Copy: %w", err) } From 4e880ab31c0895c6d05256af69247fe32f9b8392 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 18:26:30 +0100 Subject: [PATCH 03/18] Refactor variable and function names for improved clarity in msg.go The change updates various variable and function names in msg.go to make the code more intuitive. Updated names better capture what they represent or do, improving code readability and maintainability. This refactor does not impact functionality or logic of the code. --- msg.go | 337 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 169 insertions(+), 168 deletions(-) diff --git a/msg.go b/msg.go index c740450..40d828a 100644 --- a/msg.go +++ b/msg.go @@ -126,8 +126,8 @@ const SendmailPath = "/usr/sbin/sendmail" type MsgOption func(*Msg) // NewMsg returns a new Msg pointer -func NewMsg(o ...MsgOption) *Msg { - m := &Msg{ +func NewMsg(opts ...MsgOption) *Msg { + msg := &Msg{ addrHeader: make(map[AddrHeader][]*mail.Address), charset: CharsetUTF8, encoding: EncodingQP, @@ -137,17 +137,17 @@ func NewMsg(o ...MsgOption) *Msg { } // Override defaults with optionally provided MsgOption functions - for _, co := range o { - if co == nil { + for _, option := range opts { + if option == nil { continue } - co(m) + option(msg) } // Set the matcing mime.WordEncoder for the Msg - m.setEncoder() + msg.setEncoder() - return m + return msg } // WithCharset overrides the default message charset @@ -186,9 +186,9 @@ func WithMiddleware(mw Middleware) MsgOption { } // WithPGPType overrides the default PGPType of the message -func WithPGPType(t PGPType) MsgOption { +func WithPGPType(pt PGPType) MsgOption { return func(m *Msg) { - m.pgptype = t + m.pgptype = pt } } @@ -232,20 +232,20 @@ func (m *Msg) Charset() string { // For adding address headers like "To:" or "From", see SetAddrHeader // // Deprecated: This method only exists for compatibility reason. Please use SetGenHeader instead -func (m *Msg) SetHeader(h Header, v ...string) { - m.SetGenHeader(h, v...) +func (m *Msg) SetHeader(header Header, values ...string) { + m.SetGenHeader(header, values...) } // SetGenHeader sets a generic header field of the Msg // For adding address headers like "To:" or "From", see SetAddrHeader -func (m *Msg) SetGenHeader(h Header, v ...string) { +func (m *Msg) SetGenHeader(header Header, values ...string) { if m.genHeader == nil { m.genHeader = make(map[Header][]string) } - for i, hv := range v { - v[i] = m.encodeString(hv) + for i, val := range values { + values[i] = m.encodeString(val) } - m.genHeader[h] = v + m.genHeader[header] = values } // SetHeaderPreformatted sets a generic header field of the Msg which content is @@ -253,8 +253,8 @@ func (m *Msg) SetGenHeader(h Header, v ...string) { // // Deprecated: This method only exists for compatibility reason. Please use // SetGenHeaderPreformatted instead -func (m *Msg) SetHeaderPreformatted(h Header, v string) { - m.SetGenHeaderPreformatted(h, v) +func (m *Msg) SetHeaderPreformatted(header Header, value string) { + m.SetGenHeaderPreformatted(header, value) } // SetGenHeaderPreformatted sets a generic header field of the Msg which content is @@ -268,206 +268,207 @@ func (m *Msg) SetHeaderPreformatted(h Header, v string) { // user is respondible for the formating of the message header, go-mail cannot // guarantee the fully compliance with the RFC 2822. It is recommended to use // SetGenHeader instead. -func (m *Msg) SetGenHeaderPreformatted(h Header, v string) { +func (m *Msg) SetGenHeaderPreformatted(header Header, value string) { if m.preformHeader == nil { m.preformHeader = make(map[Header]string) } - m.preformHeader[h] = v + m.preformHeader[header] = value } // SetAddrHeader sets an address related header field of the Msg -func (m *Msg) SetAddrHeader(h AddrHeader, v ...string) error { +func (m *Msg) SetAddrHeader(header AddrHeader, values ...string) error { if m.addrHeader == nil { m.addrHeader = make(map[AddrHeader][]*mail.Address) } - var al []*mail.Address - for _, av := range v { - a, err := mail.ParseAddress(av) + var addresses []*mail.Address + for _, addrVal := range values { + address, err := mail.ParseAddress(addrVal) if err != nil { - return fmt.Errorf(errParseMailAddr, av, err) + return fmt.Errorf(errParseMailAddr, addrVal, err) } - al = append(al, a) + addresses = append(addresses, address) } - switch h { + switch header { case HeaderFrom: - if len(al) > 0 { - m.addrHeader[h] = []*mail.Address{al[0]} + if len(addresses) > 0 { + m.addrHeader[header] = []*mail.Address{addresses[0]} } default: - m.addrHeader[h] = al + m.addrHeader[header] = addresses } return nil } // SetAddrHeaderIgnoreInvalid sets an address related header field of the Msg and ignores invalid address // in the validation process -func (m *Msg) SetAddrHeaderIgnoreInvalid(h AddrHeader, v ...string) { - var al []*mail.Address - for _, av := range v { - a, err := mail.ParseAddress(m.encodeString(av)) +func (m *Msg) SetAddrHeaderIgnoreInvalid(header AddrHeader, values ...string) { + var addresses []*mail.Address + for _, addrVal := range values { + address, err := mail.ParseAddress(m.encodeString(addrVal)) if err != nil { continue } - al = append(al, a) + addresses = append(addresses, address) } - m.addrHeader[h] = al + m.addrHeader[header] = addresses } // EnvelopeFrom takes and validates a given mail address and sets it as envelope "FROM" // addrHeader of the Msg -func (m *Msg) EnvelopeFrom(f string) error { - return m.SetAddrHeader(HeaderEnvelopeFrom, f) +func (m *Msg) EnvelopeFrom(from string) error { + return m.SetAddrHeader(HeaderEnvelopeFrom, from) } // EnvelopeFromFormat takes a name and address, formats them RFC5322 compliant and stores them as // the envelope FROM address header field -func (m *Msg) EnvelopeFromFormat(n, a string) error { - return m.SetAddrHeader(HeaderEnvelopeFrom, fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) EnvelopeFromFormat(name, addr string) error { + return m.SetAddrHeader(HeaderEnvelopeFrom, fmt.Sprintf(`"%s" <%s>`, name, addr)) } // From takes and validates a given mail address and sets it as "From" genHeader of the Msg -func (m *Msg) From(f string) error { - return m.SetAddrHeader(HeaderFrom, f) +func (m *Msg) From(from string) error { + return m.SetAddrHeader(HeaderFrom, from) } // FromFormat takes a name and address, formats them RFC5322 compliant and stores them as // the From address header field -func (m *Msg) FromFormat(n, a string) error { - return m.SetAddrHeader(HeaderFrom, fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) FromFormat(name, addr string) error { + return m.SetAddrHeader(HeaderFrom, fmt.Sprintf(`"%s" <%s>`, name, addr)) } // To takes and validates a given mail address list sets the To: addresses of the Msg -func (m *Msg) To(t ...string) error { - return m.SetAddrHeader(HeaderTo, t...) +func (m *Msg) To(rcpts ...string) error { + return m.SetAddrHeader(HeaderTo, rcpts...) } // AddTo adds an additional address to the To address header field -func (m *Msg) AddTo(t string) error { - return m.addAddr(HeaderTo, t) +func (m *Msg) AddTo(rcpt string) error { + return m.addAddr(HeaderTo, rcpt) } // AddToFormat takes a name and address, formats them RFC5322 compliant and stores them as // as additional To address header field -func (m *Msg) AddToFormat(n, a string) error { - return m.addAddr(HeaderTo, fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) AddToFormat(name, addr string) error { + return m.addAddr(HeaderTo, fmt.Sprintf(`"%s" <%s>`, name, addr)) } // ToIgnoreInvalid takes and validates a given mail address list sets the To: addresses of the Msg // Any provided address that is not RFC5322 compliant, will be ignored -func (m *Msg) ToIgnoreInvalid(t ...string) { - m.SetAddrHeaderIgnoreInvalid(HeaderTo, t...) +func (m *Msg) ToIgnoreInvalid(rcpts ...string) { + m.SetAddrHeaderIgnoreInvalid(HeaderTo, rcpts...) } // ToFromString takes and validates a given string of comma separted // mail address and sets them as To: addresses of the Msg -func (m *Msg) ToFromString(v string) error { - return m.To(strings.Split(v, ",")...) +func (m *Msg) ToFromString(rcpts string) error { + return m.To(strings.Split(rcpts, ",")...) } // Cc takes and validates a given mail address list sets the Cc: addresses of the Msg -func (m *Msg) Cc(c ...string) error { - return m.SetAddrHeader(HeaderCc, c...) +func (m *Msg) Cc(rcpts ...string) error { + return m.SetAddrHeader(HeaderCc, rcpts...) } // AddCc adds an additional address to the Cc address header field -func (m *Msg) AddCc(t string) error { - return m.addAddr(HeaderCc, t) +func (m *Msg) AddCc(rcpt string) error { + return m.addAddr(HeaderCc, rcpt) } // AddCcFormat takes a name and address, formats them RFC5322 compliant and stores them as // as additional Cc address header field -func (m *Msg) AddCcFormat(n, a string) error { - return m.addAddr(HeaderCc, fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) AddCcFormat(name, addr string) error { + return m.addAddr(HeaderCc, fmt.Sprintf(`"%s" <%s>`, name, addr)) } // CcIgnoreInvalid takes and validates a given mail address list sets the Cc: addresses of the Msg // Any provided address that is not RFC5322 compliant, will be ignored -func (m *Msg) CcIgnoreInvalid(c ...string) { - m.SetAddrHeaderIgnoreInvalid(HeaderCc, c...) +func (m *Msg) CcIgnoreInvalid(rcpts ...string) { + m.SetAddrHeaderIgnoreInvalid(HeaderCc, rcpts...) } // CcFromString takes and validates a given string of comma separted // mail address and sets them as Cc: addresses of the Msg -func (m *Msg) CcFromString(v string) error { - return m.Cc(strings.Split(v, ",")...) +func (m *Msg) CcFromString(rcpts string) error { + return m.Cc(strings.Split(rcpts, ",")...) } // Bcc takes and validates a given mail address list sets the Bcc: addresses of the Msg -func (m *Msg) Bcc(b ...string) error { - return m.SetAddrHeader(HeaderBcc, b...) +func (m *Msg) Bcc(rcpts ...string) error { + return m.SetAddrHeader(HeaderBcc, rcpts...) } // AddBcc adds an additional address to the Bcc address header field -func (m *Msg) AddBcc(t string) error { - return m.addAddr(HeaderBcc, t) +func (m *Msg) AddBcc(rcpt string) error { + return m.addAddr(HeaderBcc, rcpt) } // AddBccFormat takes a name and address, formats them RFC5322 compliant and stores them as // as additional Bcc address header field -func (m *Msg) AddBccFormat(n, a string) error { - return m.addAddr(HeaderBcc, fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) AddBccFormat(name, addr string) error { + return m.addAddr(HeaderBcc, fmt.Sprintf(`"%s" <%s>`, name, addr)) } // BccIgnoreInvalid takes and validates a given mail address list sets the Bcc: addresses of the Msg // Any provided address that is not RFC5322 compliant, will be ignored -func (m *Msg) BccIgnoreInvalid(b ...string) { - m.SetAddrHeaderIgnoreInvalid(HeaderBcc, b...) +func (m *Msg) BccIgnoreInvalid(rcpts ...string) { + m.SetAddrHeaderIgnoreInvalid(HeaderBcc, rcpts...) } // BccFromString takes and validates a given string of comma separted // mail address and sets them as Bcc: addresses of the Msg -func (m *Msg) BccFromString(v string) error { - return m.Bcc(strings.Split(v, ",")...) +func (m *Msg) BccFromString(rcpts string) error { + return m.Bcc(strings.Split(rcpts, ",")...) } // ReplyTo takes and validates a given mail address and sets it as "Reply-To" addrHeader of the Msg -func (m *Msg) ReplyTo(r string) error { - rt, err := mail.ParseAddress(r) +func (m *Msg) ReplyTo(addr string) error { + replyTo, err := mail.ParseAddress(addr) if err != nil { return fmt.Errorf("failed to parse reply-to address: %w", err) } - m.SetGenHeader(HeaderReplyTo, rt.String()) + m.SetGenHeader(HeaderReplyTo, replyTo.String()) return nil } // ReplyToFormat takes a name and address, formats them RFC5322 compliant and stores them as // the Reply-To header field -func (m *Msg) ReplyToFormat(n, a string) error { - return m.ReplyTo(fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) ReplyToFormat(name, addr string) error { + return m.ReplyTo(fmt.Sprintf(`"%s" <%s>`, name, addr)) } // addAddr adds an additional address to the given addrHeader of the Msg -func (m *Msg) addAddr(h AddrHeader, a string) error { - var al []string - for _, ca := range m.addrHeader[h] { - al = append(al, ca.String()) +func (m *Msg) addAddr(header AddrHeader, addr string) error { + var addresses []string + for _, address := range m.addrHeader[header] { + addresses = append(addresses, address.String()) } - al = append(al, a) - return m.SetAddrHeader(h, al...) + addresses = append(addresses, addr) + return m.SetAddrHeader(header, addresses...) } // Subject sets the "Subject" header field of the Msg -func (m *Msg) Subject(s string) { - m.SetGenHeader(HeaderSubject, s) +func (m *Msg) Subject(subj string) { + m.SetGenHeader(HeaderSubject, subj) } // SetMessageID generates a random message id for the mail func (m *Msg) SetMessageID() { - hn, err := os.Hostname() + hostname, err := os.Hostname() if err != nil { - hn = "localhost.localdomain" + hostname = "localhost.localdomain" } - rn, _ := randNum(100000000) - rm, _ := randNum(10000) - rs, _ := randomStringSecure(17) - pid := os.Getpid() * rm - mid := fmt.Sprintf("%d.%d%d.%s@%s", pid, rn, rm, rs, hn) - m.SetMessageIDWithValue(mid) + randNumPrimary, _ := randNum(100000000) + randNumSecondary, _ := randNum(10000) + randString, _ := randomStringSecure(17) + procID := os.Getpid() * randNumSecondary + messageID := fmt.Sprintf("%d.%d%d.%s@%s", procID, randNumPrimary, randNumSecondary, + randString, hostname) + m.SetMessageIDWithValue(messageID) } // SetMessageIDWithValue sets the message id for the mail -func (m *Msg) SetMessageIDWithValue(v string) { - m.SetGenHeader(HeaderMessageID, fmt.Sprintf("<%s>", v)) +func (m *Msg) SetMessageIDWithValue(messageID string) { + m.SetGenHeader(HeaderMessageID, fmt.Sprintf("<%s>", messageID)) } // SetBulk sets the "Precedence: bulk" and "X-Auto-Response-Suppress: All" genHeaders which are @@ -481,35 +482,35 @@ func (m *Msg) SetBulk() { // SetDate sets the Date genHeader field to the current time in a valid format func (m *Msg) SetDate() { - ts := time.Now().Format(time.RFC1123Z) - m.SetGenHeader(HeaderDate, ts) + now := time.Now().Format(time.RFC1123Z) + m.SetGenHeader(HeaderDate, now) } // SetDateWithValue sets the Date genHeader field to the provided time in a valid format -func (m *Msg) SetDateWithValue(t time.Time) { - m.SetGenHeader(HeaderDate, t.Format(time.RFC1123Z)) +func (m *Msg) SetDateWithValue(timeVal time.Time) { + m.SetGenHeader(HeaderDate, timeVal.Format(time.RFC1123Z)) } // SetImportance sets the Msg Importance/Priority header to given Importance -func (m *Msg) SetImportance(i Importance) { - if i == ImportanceNormal { +func (m *Msg) SetImportance(importance Importance) { + if importance == ImportanceNormal { return } - m.SetGenHeader(HeaderImportance, i.String()) - m.SetGenHeader(HeaderPriority, i.NumString()) - m.SetGenHeader(HeaderXPriority, i.XPrioString()) - m.SetGenHeader(HeaderXMSMailPriority, i.NumString()) + m.SetGenHeader(HeaderImportance, importance.String()) + m.SetGenHeader(HeaderPriority, importance.NumString()) + m.SetGenHeader(HeaderXPriority, importance.XPrioString()) + m.SetGenHeader(HeaderXMSMailPriority, importance.NumString()) } // SetOrganization sets the provided string as Organization header for the Msg -func (m *Msg) SetOrganization(o string) { - m.SetGenHeader(HeaderOrganization, o) +func (m *Msg) SetOrganization(org string) { + m.SetGenHeader(HeaderOrganization, org) } // SetUserAgent sets the User-Agent/X-Mailer header for the Msg -func (m *Msg) SetUserAgent(a string) { - m.SetGenHeader(HeaderUserAgent, a) - m.SetGenHeader(HeaderXMailer, a) +func (m *Msg) SetUserAgent(userAgent string) { + m.SetGenHeader(HeaderUserAgent, userAgent) + m.SetGenHeader(HeaderXMailer, userAgent) } // IsDelivered will return true if the Msg has been successfully delivered @@ -521,17 +522,17 @@ func (m *Msg) IsDelivered() bool { // as described in RFC8098. It allows to provide a list recipient addresses. // Address validation is performed // See: https://www.rfc-editor.org/rfc/rfc8098.html -func (m *Msg) RequestMDNTo(t ...string) error { - var tl []string - for _, at := range t { - a, err := mail.ParseAddress(at) +func (m *Msg) RequestMDNTo(rcpts ...string) error { + var addresses []string + for _, addrVal := range rcpts { + address, err := mail.ParseAddress(addrVal) if err != nil { - return fmt.Errorf(errParseMailAddr, at, err) + return fmt.Errorf(errParseMailAddr, addrVal, err) } - tl = append(tl, a.String()) + addresses = append(addresses, address.String()) } if _, ok := m.genHeader[HeaderDispositionNotificationTo]; ok { - m.genHeader[HeaderDispositionNotificationTo] = tl + m.genHeader[HeaderDispositionNotificationTo] = addresses } return nil } @@ -540,77 +541,77 @@ func (m *Msg) RequestMDNTo(t ...string) error { // as described in RFC8098. It allows to provide a recipient address with name and address and will format // accordingly. Address validation is performed // See: https://www.rfc-editor.org/rfc/rfc8098.html -func (m *Msg) RequestMDNToFormat(n, a string) error { - return m.RequestMDNTo(fmt.Sprintf(`%s <%s>`, n, a)) +func (m *Msg) RequestMDNToFormat(name, addr string) error { + return m.RequestMDNTo(fmt.Sprintf(`%s <%s>`, name, addr)) } // RequestMDNAddTo adds an additional recipient to the recipient list of the MDN -func (m *Msg) RequestMDNAddTo(t string) error { - a, err := mail.ParseAddress(t) +func (m *Msg) RequestMDNAddTo(rcpt string) error { + address, err := mail.ParseAddress(rcpt) if err != nil { - return fmt.Errorf(errParseMailAddr, t, err) + return fmt.Errorf(errParseMailAddr, rcpt, err) } - var tl []string - tl = append(tl, m.genHeader[HeaderDispositionNotificationTo]...) - tl = append(tl, a.String()) + var addresses []string + addresses = append(addresses, m.genHeader[HeaderDispositionNotificationTo]...) + addresses = append(addresses, address.String()) if _, ok := m.genHeader[HeaderDispositionNotificationTo]; ok { - m.genHeader[HeaderDispositionNotificationTo] = tl + m.genHeader[HeaderDispositionNotificationTo] = addresses } return nil } // RequestMDNAddToFormat adds an additional formated recipient to the recipient list of the MDN -func (m *Msg) RequestMDNAddToFormat(n, a string) error { - return m.RequestMDNAddTo(fmt.Sprintf(`"%s" <%s>`, n, a)) +func (m *Msg) RequestMDNAddToFormat(name, addr string) error { + return m.RequestMDNAddTo(fmt.Sprintf(`"%s" <%s>`, name, addr)) } // GetSender returns the currently set envelope FROM address. If no envelope FROM is set it will use -// the first mail body FROM address. If ff is true, it will return the full address string including -// the address name, if set -func (m *Msg) GetSender(ff bool) (string, error) { - f, ok := m.addrHeader[HeaderEnvelopeFrom] - if !ok || len(f) == 0 { - f, ok = m.addrHeader[HeaderFrom] - if !ok || len(f) == 0 { +// the first mail body FROM address. If useFullAddr is true, it will return the full address string +// including the address name, if set +func (m *Msg) GetSender(useFullAddr bool) (string, error) { + from, ok := m.addrHeader[HeaderEnvelopeFrom] + if !ok || len(from) == 0 { + from, ok = m.addrHeader[HeaderFrom] + if !ok || len(from) == 0 { return "", ErrNoFromAddress } } - if ff { - return f[0].String(), nil + if useFullAddr { + return from[0].String(), nil } - return f[0].Address, nil + return from[0].Address, nil } // GetRecipients returns a list of the currently set TO/CC/BCC addresses. func (m *Msg) GetRecipients() ([]string, error) { - var rl []string - for _, t := range []AddrHeader{HeaderTo, HeaderCc, HeaderBcc} { - al, ok := m.addrHeader[t] - if !ok || len(al) == 0 { + var rcpts []string + for _, addressType := range []AddrHeader{HeaderTo, HeaderCc, HeaderBcc} { + addresses, ok := m.addrHeader[addressType] + if !ok || len(addresses) == 0 { continue } - for _, r := range al { - rl = append(rl, r.Address) + for _, r := range addresses { + rcpts = append(rcpts, r.Address) } } - if len(rl) <= 0 { - return rl, ErrNoRcptAddresses + if len(rcpts) <= 0 { + return rcpts, ErrNoRcptAddresses } - return rl, nil + return rcpts, nil } // GetAddrHeader returns the content of the requested address header of the Msg -func (m *Msg) GetAddrHeader(h AddrHeader) []*mail.Address { - return m.addrHeader[h] +func (m *Msg) GetAddrHeader(header AddrHeader) []*mail.Address { + return m.addrHeader[header] } // GetAddrHeaderString returns the address string of the requested address header of the Msg -func (m *Msg) GetAddrHeaderString(h AddrHeader) []string { - var al []string - for _, mh := range m.addrHeader[h] { - al = append(al, mh.String()) +func (m *Msg) GetAddrHeaderString(header AddrHeader) []string { + var addresses []string + for _, mh := range m.addrHeader[header] { + addresses = append(addresses, mh.String()) } - return al + return addresses } // GetFrom returns the content of the From address header of the Msg @@ -654,8 +655,8 @@ func (m *Msg) GetBccString() []string { } // GetGenHeader returns the content of the requested generic header of the Msg -func (m *Msg) GetGenHeader(h Header) []string { - return m.genHeader[h] +func (m *Msg) GetGenHeader(header Header) []string { + return m.genHeader[header] } // GetParts returns the message parts of the Msg @@ -669,8 +670,8 @@ func (m *Msg) GetAttachments() []*File { } // SetAttachements sets the attachements of the message. -func (m *Msg) SetAttachements(ff []*File) { - m.attachments = ff +func (m *Msg) SetAttachements(files []*File) { + m.attachments = files } // UnsetAllAttachments unset the attachments of the message. @@ -684,8 +685,8 @@ func (m *Msg) GetEmbeds() []*File { } // SetEmbeds sets the embeds of the message. -func (m *Msg) SetEmbeds(ff []*File) { - m.embeds = ff +func (m *Msg) SetEmbeds(files []*File) { + m.embeds = files } // UnsetAllEmbeds unset the embeds of the message. @@ -700,16 +701,16 @@ func (m *Msg) UnsetAllParts() { } // SetBodyString sets the body of the message. -func (m *Msg) SetBodyString(ct ContentType, b string, o ...PartOption) { - buf := bytes.NewBufferString(b) - w := writeFuncFromBuffer(buf) - m.SetBodyWriter(ct, w, o...) +func (m *Msg) SetBodyString(contentType ContentType, content string, opts ...PartOption) { + buffer := bytes.NewBufferString(content) + writeFunc := writeFuncFromBuffer(buffer) + m.SetBodyWriter(contentType, writeFunc, opts...) } // SetBodyWriter sets the body of the message. -func (m *Msg) SetBodyWriter(ct ContentType, w func(io.Writer) (int64, error), o ...PartOption) { - p := m.newPart(ct, o...) - p.w = w +func (m *Msg) SetBodyWriter(contentType ContentType, writeFunc func(io.Writer) (int64, error), opts ...PartOption) { + p := m.newPart(contentType, opts...) + p.w = writeFunc m.parts = []*Part{p} } From c455b45a3e5b15748a45addd04e6d39f38bdbaa4 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 21:26:55 +0100 Subject: [PATCH 04/18] Refactor variable and function names for improved clarity in msg.go The change updates various variable and function names in msg.go to make the code more intuitive. Updated names better capture what they represent or do, improving code readability and maintainability. This refactor does not impact functionality or logic of the code. --- msg.go | 408 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 211 insertions(+), 197 deletions(-) diff --git a/msg.go b/msg.go index 40d828a..b62e83c 100644 --- a/msg.go +++ b/msg.go @@ -708,7 +708,10 @@ func (m *Msg) SetBodyString(contentType ContentType, content string, opts ...Par } // SetBodyWriter sets the body of the message. -func (m *Msg) SetBodyWriter(contentType ContentType, writeFunc func(io.Writer) (int64, error), opts ...PartOption) { +func (m *Msg) SetBodyWriter( + contentType ContentType, writeFunc func(io.Writer) (int64, error), + opts ...PartOption, +) { p := m.newPart(contentType, opts...) p.w = writeFunc m.parts = []*Part{p} @@ -716,85 +719,88 @@ func (m *Msg) SetBodyWriter(contentType ContentType, writeFunc func(io.Writer) ( // SetBodyHTMLTemplate sets the body of the message from a given html/template.Template pointer // The content type will be set to text/html automatically -func (m *Msg) SetBodyHTMLTemplate(t *ht.Template, d interface{}, o ...PartOption) error { - if t == nil { +func (m *Msg) SetBodyHTMLTemplate(tpl *ht.Template, data interface{}, opts ...PartOption) error { + if tpl == nil { return fmt.Errorf(errTplPointerNil) } - buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + buffer := bytes.Buffer{} + if err := tpl.Execute(&buffer, data); err != nil { return fmt.Errorf(errTplExecuteFailed, err) } - w := writeFuncFromBuffer(&buf) - m.SetBodyWriter(TypeTextHTML, w, o...) + writeFunc := writeFuncFromBuffer(&buffer) + m.SetBodyWriter(TypeTextHTML, writeFunc, opts...) return nil } // SetBodyTextTemplate sets the body of the message from a given text/template.Template pointer // The content type will be set to text/plain automatically -func (m *Msg) SetBodyTextTemplate(t *tt.Template, d interface{}, o ...PartOption) error { - if t == nil { +func (m *Msg) SetBodyTextTemplate(tpl *tt.Template, data interface{}, opts ...PartOption) error { + if tpl == nil { return fmt.Errorf(errTplPointerNil) } buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + if err := tpl.Execute(&buf, data); err != nil { return fmt.Errorf(errTplExecuteFailed, err) } - w := writeFuncFromBuffer(&buf) - m.SetBodyWriter(TypeTextPlain, w, o...) + writeFunc := writeFuncFromBuffer(&buf) + m.SetBodyWriter(TypeTextPlain, writeFunc, opts...) return nil } // AddAlternativeString sets the alternative body of the message. -func (m *Msg) AddAlternativeString(ct ContentType, b string, o ...PartOption) { - buf := bytes.NewBufferString(b) - w := writeFuncFromBuffer(buf) - m.AddAlternativeWriter(ct, w, o...) +func (m *Msg) AddAlternativeString(contentType ContentType, content string, opts ...PartOption) { + buffer := bytes.NewBufferString(content) + writeFunc := writeFuncFromBuffer(buffer) + m.AddAlternativeWriter(contentType, writeFunc, opts...) } // AddAlternativeWriter sets the body of the message. -func (m *Msg) AddAlternativeWriter(ct ContentType, w func(io.Writer) (int64, error), o ...PartOption) { - p := m.newPart(ct, o...) - p.w = w - m.parts = append(m.parts, p) +func (m *Msg) AddAlternativeWriter( + contentType ContentType, writeFunc func(io.Writer) (int64, error), + opts ...PartOption, +) { + part := m.newPart(contentType, opts...) + part.w = writeFunc + m.parts = append(m.parts, part) } // AddAlternativeHTMLTemplate sets the alternative body of the message to a html/template.Template output // The content type will be set to text/html automatically -func (m *Msg) AddAlternativeHTMLTemplate(t *ht.Template, d interface{}, o ...PartOption) error { - if t == nil { +func (m *Msg) AddAlternativeHTMLTemplate(tpl *ht.Template, data interface{}, opts ...PartOption) error { + if tpl == nil { return fmt.Errorf(errTplPointerNil) } - buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + buffer := bytes.Buffer{} + if err := tpl.Execute(&buffer, data); err != nil { return fmt.Errorf(errTplExecuteFailed, err) } - w := writeFuncFromBuffer(&buf) - m.AddAlternativeWriter(TypeTextHTML, w, o...) + writeFunc := writeFuncFromBuffer(&buffer) + m.AddAlternativeWriter(TypeTextHTML, writeFunc, opts...) return nil } // AddAlternativeTextTemplate sets the alternative body of the message to a text/template.Template output // The content type will be set to text/plain automatically -func (m *Msg) AddAlternativeTextTemplate(t *tt.Template, d interface{}, o ...PartOption) error { - if t == nil { +func (m *Msg) AddAlternativeTextTemplate(tpl *tt.Template, data interface{}, opts ...PartOption) error { + if tpl == nil { return fmt.Errorf(errTplPointerNil) } - buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + buffer := bytes.Buffer{} + if err := tpl.Execute(&buffer, data); err != nil { return fmt.Errorf(errTplExecuteFailed, err) } - w := writeFuncFromBuffer(&buf) - m.AddAlternativeWriter(TypeTextPlain, w, o...) + writeFunc := writeFuncFromBuffer(&buffer) + m.AddAlternativeWriter(TypeTextPlain, writeFunc, opts...) return nil } // AttachFile adds an attachment File to the Msg -func (m *Msg) AttachFile(n string, o ...FileOption) { - f := fileFromFS(n) - if f == nil { +func (m *Msg) AttachFile(name string, opts ...FileOption) { + file := fileFromFS(name) + if file == nil { return } - m.attachments = m.appendFile(m.attachments, f, o...) + m.attachments = m.appendFile(m.attachments, file, opts...) } // AttachReader adds an attachment File via io.Reader to the Msg @@ -803,61 +809,65 @@ func (m *Msg) AttachFile(n string, o ...FileOption) { // into memory first, so it can seek through it. Using larger amounts of // data on the io.Reader should be avoided. For such, it is recommended to // either use AttachFile or AttachReadSeeker instead -func (m *Msg) AttachReader(n string, r io.Reader, o ...FileOption) error { - f, err := fileFromReader(n, r) +func (m *Msg) AttachReader(name string, reader io.Reader, opts ...FileOption) error { + file, err := fileFromReader(name, reader) if err != nil { return err } - m.attachments = m.appendFile(m.attachments, f, o...) + m.attachments = m.appendFile(m.attachments, file, opts...) return nil } // AttachReadSeeker adds an attachment File via io.ReadSeeker to the Msg -func (m *Msg) AttachReadSeeker(n string, r io.ReadSeeker, o ...FileOption) { - f := fileFromReadSeeker(n, r) - m.attachments = m.appendFile(m.attachments, f, o...) +func (m *Msg) AttachReadSeeker(name string, reader io.ReadSeeker, opts ...FileOption) { + file := fileFromReadSeeker(name, reader) + m.attachments = m.appendFile(m.attachments, file, opts...) } // AttachHTMLTemplate adds the output of a html/template.Template pointer as File attachment to the Msg -func (m *Msg) AttachHTMLTemplate(n string, t *ht.Template, d interface{}, o ...FileOption) error { - f, err := fileFromHTMLTemplate(n, t, d) +func (m *Msg) AttachHTMLTemplate( + name string, tpl *ht.Template, data interface{}, opts ...FileOption, +) error { + file, err := fileFromHTMLTemplate(name, tpl, data) if err != nil { return fmt.Errorf("failed to attach template: %w", err) } - m.attachments = m.appendFile(m.attachments, f, o...) + m.attachments = m.appendFile(m.attachments, file, opts...) return nil } // AttachTextTemplate adds the output of a text/template.Template pointer as File attachment to the Msg -func (m *Msg) AttachTextTemplate(n string, t *tt.Template, d interface{}, o ...FileOption) error { - f, err := fileFromTextTemplate(n, t, d) +func (m *Msg) AttachTextTemplate( + name string, tpl *tt.Template, data interface{}, opts ...FileOption, +) error { + file, err := fileFromTextTemplate(name, tpl, data) if err != nil { return fmt.Errorf("failed to attach template: %w", err) } - m.attachments = m.appendFile(m.attachments, f, o...) + m.attachments = m.appendFile(m.attachments, file, opts...) return nil } // AttachFromEmbedFS adds an attachment File from an embed.FS to the Msg -func (m *Msg) AttachFromEmbedFS(n string, f *embed.FS, o ...FileOption) error { - if f == nil { +func (m *Msg) AttachFromEmbedFS(name string, fs *embed.FS, opts ...FileOption) error { + if fs == nil { return fmt.Errorf("embed.FS must not be nil") } - ef, err := fileFromEmbedFS(n, f) + file, err := fileFromEmbedFS(name, fs) if err != nil { return err } - m.attachments = m.appendFile(m.attachments, ef, o...) + m.attachments = m.appendFile(m.attachments, file, opts...) return nil } // EmbedFile adds an embedded File to the Msg -func (m *Msg) EmbedFile(n string, o ...FileOption) { - f := fileFromFS(n) - if f == nil { +func (m *Msg) EmbedFile(name string, opts ...FileOption) { + file := fileFromFS(name) + if file == nil { return } - m.embeds = m.appendFile(m.embeds, f, o...) + m.embeds = m.appendFile(m.embeds, file, opts...) } // EmbedReader adds an embedded File from an io.Reader to the Msg @@ -866,51 +876,55 @@ func (m *Msg) EmbedFile(n string, o ...FileOption) { // into memory first, so it can seek through it. Using larger amounts of // data on the io.Reader should be avoided. For such, it is recommended to // either use EmbedFile or EmbedReadSeeker instead -func (m *Msg) EmbedReader(n string, r io.Reader, o ...FileOption) error { - f, err := fileFromReader(n, r) +func (m *Msg) EmbedReader(name string, reader io.Reader, opts ...FileOption) error { + file, err := fileFromReader(name, reader) if err != nil { return err } - m.embeds = m.appendFile(m.embeds, f, o...) + m.embeds = m.appendFile(m.embeds, file, opts...) return nil } // EmbedReadSeeker adds an embedded File from an io.ReadSeeker to the Msg -func (m *Msg) EmbedReadSeeker(n string, r io.ReadSeeker, o ...FileOption) { - f := fileFromReadSeeker(n, r) - m.embeds = m.appendFile(m.embeds, f, o...) +func (m *Msg) EmbedReadSeeker(name string, reader io.ReadSeeker, opts ...FileOption) { + file := fileFromReadSeeker(name, reader) + m.embeds = m.appendFile(m.embeds, file, opts...) } // EmbedHTMLTemplate adds the output of a html/template.Template pointer as embedded File to the Msg -func (m *Msg) EmbedHTMLTemplate(n string, t *ht.Template, d interface{}, o ...FileOption) error { - f, err := fileFromHTMLTemplate(n, t, d) +func (m *Msg) EmbedHTMLTemplate( + name string, tpl *ht.Template, data interface{}, opts ...FileOption, +) error { + file, err := fileFromHTMLTemplate(name, tpl, data) if err != nil { return fmt.Errorf("failed to embed template: %w", err) } - m.embeds = m.appendFile(m.embeds, f, o...) + m.embeds = m.appendFile(m.embeds, file, opts...) return nil } // EmbedTextTemplate adds the output of a text/template.Template pointer as embedded File to the Msg -func (m *Msg) EmbedTextTemplate(n string, t *tt.Template, d interface{}, o ...FileOption) error { - f, err := fileFromTextTemplate(n, t, d) +func (m *Msg) EmbedTextTemplate( + name string, tpl *tt.Template, data interface{}, opts ...FileOption, +) error { + file, err := fileFromTextTemplate(name, tpl, data) if err != nil { return fmt.Errorf("failed to embed template: %w", err) } - m.embeds = m.appendFile(m.embeds, f, o...) + m.embeds = m.appendFile(m.embeds, file, opts...) return nil } // EmbedFromEmbedFS adds an embedded File from an embed.FS to the Msg -func (m *Msg) EmbedFromEmbedFS(n string, f *embed.FS, o ...FileOption) error { - if f == nil { +func (m *Msg) EmbedFromEmbedFS(name string, fs *embed.FS, opts ...FileOption) error { + if fs == nil { return fmt.Errorf("embed.FS must not be nil") } - ef, err := fileFromEmbedFS(n, f) + file, err := fileFromEmbedFS(name, fs) if err != nil { return err } - m.embeds = m.appendFile(m.embeds, ef, o...) + m.embeds = m.appendFile(m.embeds, file, opts...) return nil } @@ -925,73 +939,73 @@ func (m *Msg) Reset() { } // ApplyMiddlewares apply the list of middlewares to a Msg -func (m *Msg) applyMiddlewares(ms *Msg) *Msg { - for _, mw := range m.middlewares { - ms = mw.Handle(ms) +func (m *Msg) applyMiddlewares(msg *Msg) *Msg { + for _, middleware := range m.middlewares { + msg = middleware.Handle(msg) } - return ms + return msg } // WriteTo writes the formated Msg into a give io.Writer and satisfies the io.WriteTo interface -func (m *Msg) WriteTo(w io.Writer) (int64, error) { - mw := &msgWriter{writer: w, charset: m.charset, encoder: m.encoder} +func (m *Msg) WriteTo(writer io.Writer) (int64, error) { + mw := &msgWriter{writer: writer, charset: m.charset, encoder: m.encoder} mw.writeMsg(m.applyMiddlewares(m)) return mw.bytesWritten, mw.err } // WriteToSkipMiddleware writes the formated Msg into a give io.Writer and satisfies // the io.WriteTo interface but will skip the given Middleware -func (m *Msg) WriteToSkipMiddleware(w io.Writer, mt MiddlewareType) (int64, error) { - var omwl, mwl []Middleware - omwl = m.middlewares +func (m *Msg) WriteToSkipMiddleware(writer io.Writer, middleWareType MiddlewareType) (int64, error) { + var origMiddlewares, middlewares []Middleware + origMiddlewares = m.middlewares for i := range m.middlewares { - if m.middlewares[i].Type() == mt { + if m.middlewares[i].Type() == middleWareType { continue } - mwl = append(mwl, m.middlewares[i]) + middlewares = append(middlewares, m.middlewares[i]) } - m.middlewares = mwl - mw := &msgWriter{writer: w, charset: m.charset, encoder: m.encoder} + m.middlewares = middlewares + mw := &msgWriter{writer: writer, charset: m.charset, encoder: m.encoder} mw.writeMsg(m.applyMiddlewares(m)) - m.middlewares = omwl + m.middlewares = origMiddlewares return mw.bytesWritten, mw.err } // Write is an alias method to WriteTo due to compatibility reasons -func (m *Msg) Write(w io.Writer) (int64, error) { - return m.WriteTo(w) +func (m *Msg) Write(writer io.Writer) (int64, error) { + return m.WriteTo(writer) } // appendFile adds a File to the Msg (as attachment or embed) -func (m *Msg) appendFile(c []*File, f *File, o ...FileOption) []*File { +func (m *Msg) appendFile(files []*File, file *File, opts ...FileOption) []*File { // Override defaults with optionally provided FileOption functions - for _, co := range o { - if co == nil { + for _, opt := range opts { + if opt == nil { continue } - co(f) + opt(file) } - if c == nil { - return []*File{f} + if files == nil { + return []*File{file} } - return append(c, f) + return append(files, file) } // WriteToFile stores the Msg as file on disk. It will try to create the given filename // Already existing files will be overwritten -func (m *Msg) WriteToFile(n string) error { - f, err := os.Create(n) +func (m *Msg) WriteToFile(name string) error { + file, err := os.Create(name) if err != nil { return fmt.Errorf("failed to create output file: %w", err) } - defer func() { _ = f.Close() }() - _, err = m.WriteTo(f) + defer func() { _ = file.Close() }() + _, err = m.WriteTo(file) if err != nil { return fmt.Errorf("failed to write to output file: %w", err) } - return f.Close() + return file.Close() } // WriteToSendmail returns WriteToSendmailWithCommand with a default sendmail path @@ -1001,38 +1015,38 @@ func (m *Msg) WriteToSendmail() error { // WriteToSendmailWithCommand returns WriteToSendmailWithContext with a default timeout // of 5 seconds and a given sendmail path -func (m *Msg) WriteToSendmailWithCommand(sp string) error { - tctx, tcfn := context.WithTimeout(context.Background(), time.Second*5) - defer tcfn() - return m.WriteToSendmailWithContext(tctx, sp) +func (m *Msg) WriteToSendmailWithCommand(sendmailPath string) error { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + defer cancel() + return m.WriteToSendmailWithContext(ctx, sendmailPath) } // WriteToSendmailWithContext opens an pipe to the local sendmail binary and tries to send the // mail though that. It takes a context.Context, the path to the sendmail binary and additional // arguments for the sendmail binary as parameters -func (m *Msg) WriteToSendmailWithContext(ctx context.Context, sp string, a ...string) error { - ec := exec.CommandContext(ctx, sp) - ec.Args = append(ec.Args, "-oi", "-t") - ec.Args = append(ec.Args, a...) +func (m *Msg) WriteToSendmailWithContext(ctx context.Context, sendmailPath string, args ...string) error { + cmdCtx := exec.CommandContext(ctx, sendmailPath) + cmdCtx.Args = append(cmdCtx.Args, "-oi", "-t") + cmdCtx.Args = append(cmdCtx.Args, args...) - se, err := ec.StderrPipe() + stdErr, err := cmdCtx.StderrPipe() if err != nil { return fmt.Errorf("failed to set STDERR pipe: %w", err) } - si, err := ec.StdinPipe() + stdIn, err := cmdCtx.StdinPipe() if err != nil { return fmt.Errorf("failed to set STDIN pipe: %w", err) } - if se == nil || si == nil { + if stdErr == nil || stdIn == nil { return fmt.Errorf("received nil for STDERR or STDIN pipe") } // Start the execution and write to STDIN - if err = ec.Start(); err != nil { + if err = cmdCtx.Start(); err != nil { return fmt.Errorf("could not start sendmail execution: %w", err) } - _, err = m.WriteTo(si) + _, err = m.WriteTo(stdIn) if err != nil { if !errors.Is(err, syscall.EPIPE) { return fmt.Errorf("failed to write mail to buffer: %w", err) @@ -1040,20 +1054,20 @@ func (m *Msg) WriteToSendmailWithContext(ctx context.Context, sp string, a ...st } // Close STDIN and wait for completion or cancellation of the sendmail executable - if err = si.Close(); err != nil { + if err = stdIn.Close(); err != nil { return fmt.Errorf("failed to close STDIN pipe: %w", err) } // Read the stderr pipe for possible errors - serr, err := io.ReadAll(se) + sendmailErr, err := io.ReadAll(stdErr) if err != nil { return fmt.Errorf("failed to read STDERR pipe: %w", err) } - if len(serr) > 0 { - return fmt.Errorf("sendmail command failed: %s", string(serr)) + if len(sendmailErr) > 0 { + return fmt.Errorf("sendmail command failed: %s", string(sendmailErr)) } - if err = ec.Wait(); err != nil { + if err = cmdCtx.Wait(); err != nil { return fmt.Errorf("sendmail command execution failed: %w", err) } @@ -1067,24 +1081,24 @@ func (m *Msg) WriteToSendmailWithContext(ctx context.Context, sp string, a ...st // changes will not be reflected in the Reader. You will have to use Msg.UpdateReader // first to update the Reader's buffer with the current Msg content func (m *Msg) NewReader() *Reader { - r := &Reader{} - wbuf := bytes.Buffer{} - _, err := m.Write(&wbuf) + reader := &Reader{} + buffer := bytes.Buffer{} + _, err := m.Write(&buffer) if err != nil { - r.err = fmt.Errorf("failed to write Msg to Reader buffer: %w", err) + reader.err = fmt.Errorf("failed to write Msg to Reader buffer: %w", err) } - r.buf = wbuf.Bytes() - return r + reader.buf = buffer.Bytes() + return reader } // UpdateReader will update a Reader with the content of the given Msg and reset the // Reader position to the start -func (m *Msg) UpdateReader(r *Reader) { - wbuf := bytes.Buffer{} - _, err := m.Write(&wbuf) - r.Reset() - r.buf = wbuf.Bytes() - r.err = err +func (m *Msg) UpdateReader(reader *Reader) { + buffer := bytes.Buffer{} + _, err := m.Write(&buffer) + reader.Reset() + reader.buf = buffer.Bytes() + reader.err = err } // HasSendError returns true if the Msg experienced an error during the message delivery and the @@ -1096,9 +1110,9 @@ func (m *Msg) HasSendError() bool { // SendErrorIsTemp returns true if the Msg experienced an error during the message delivery and the // corresponding error was of temporary nature and should be retried later func (m *Msg) SendErrorIsTemp() bool { - var e *SendError - if errors.As(m.sendError, &e) && e != nil { - return e.isTemp + var err *SendError + if errors.As(m.sendError, &err) && err != nil { + return err.isTemp } return false } @@ -1110,19 +1124,19 @@ func (m *Msg) SendError() error { // encodeString encodes a string based on the configured message encoder and the corresponding // charset for the Msg -func (m *Msg) encodeString(s string) string { - return m.encoder.Encode(string(m.charset), s) +func (m *Msg) encodeString(str string) string { + return m.encoder.Encode(string(m.charset), str) } // hasAlt returns true if the Msg has more than one part func (m *Msg) hasAlt() bool { - c := 0 - for _, p := range m.parts { - if !p.del { - c++ + count := 0 + for _, part := range m.parts { + if !part.del { + count++ } } - return c > 1 && m.pgptype == 0 + return count > 1 && m.pgptype == 0 } // hasMixed returns true if the Msg has mixed parts @@ -1141,19 +1155,19 @@ func (m *Msg) hasPGPType() bool { } // newPart returns a new Part for the Msg -func (m *Msg) newPart(ct ContentType, o ...PartOption) *Part { +func (m *Msg) newPart(contentType ContentType, opts ...PartOption) *Part { p := &Part{ - ctype: ct, + ctype: contentType, cset: m.charset, enc: m.encoding, } // Override defaults with optionally provided MsgOption functions - for _, co := range o { - if co == nil { + for _, opt := range opts { + if opt == nil { continue } - co(p) + opt(p) } return p @@ -1187,118 +1201,118 @@ func (m *Msg) addDefaultHeader() { } // fileFromEmbedFS returns a File pointer from a given file in the provided embed.FS -func fileFromEmbedFS(n string, f *embed.FS) (*File, error) { - _, err := f.Open(n) +func fileFromEmbedFS(name string, fs *embed.FS) (*File, error) { + _, err := fs.Open(name) if err != nil { return nil, fmt.Errorf("failed to open file from embed.FS: %w", err) } return &File{ - Name: filepath.Base(n), + Name: filepath.Base(name), Header: make(map[string][]string), - Writer: func(w io.Writer) (int64, error) { - h, err := f.Open(n) + Writer: func(writer io.Writer) (int64, error) { + file, err := fs.Open(name) if err != nil { return 0, err } - nb, err := io.Copy(w, h) + numBytes, err := io.Copy(writer, file) if err != nil { - _ = h.Close() - return nb, fmt.Errorf("failed to copy file to io.Writer: %w", err) + _ = file.Close() + return numBytes, fmt.Errorf("failed to copy file to io.Writer: %w", err) } - return nb, h.Close() + return numBytes, file.Close() }, }, nil } // fileFromFS returns a File pointer from a given file in the system's file system -func fileFromFS(n string) *File { - _, err := os.Stat(n) +func fileFromFS(name string) *File { + _, err := os.Stat(name) if err != nil { return nil } return &File{ - Name: filepath.Base(n), + Name: filepath.Base(name), Header: make(map[string][]string), - Writer: func(w io.Writer) (int64, error) { - h, err := os.Open(n) + Writer: func(writer io.Writer) (int64, error) { + file, err := os.Open(name) if err != nil { return 0, err } - nb, err := io.Copy(w, h) + numBytes, err := io.Copy(writer, file) if err != nil { - _ = h.Close() - return nb, fmt.Errorf("failed to copy file to io.Writer: %w", err) + _ = file.Close() + return numBytes, fmt.Errorf("failed to copy file to io.Writer: %w", err) } - return nb, h.Close() + return numBytes, file.Close() }, } } // fileFromReader returns a File pointer from a given io.Reader -func fileFromReader(n string, r io.Reader) (*File, error) { - d, err := io.ReadAll(r) +func fileFromReader(name string, reader io.Reader) (*File, error) { + d, err := io.ReadAll(reader) if err != nil { return &File{}, err } - br := bytes.NewReader(d) + byteReader := bytes.NewReader(d) return &File{ - Name: n, + Name: name, Header: make(map[string][]string), - Writer: func(w io.Writer) (int64, error) { - rb, cerr := io.Copy(w, br) - if cerr != nil { - return rb, cerr + Writer: func(writer io.Writer) (int64, error) { + readBytes, copyErr := io.Copy(writer, byteReader) + if copyErr != nil { + return readBytes, copyErr } - _, cerr = br.Seek(0, io.SeekStart) - return rb, cerr + _, copyErr = byteReader.Seek(0, io.SeekStart) + return readBytes, copyErr }, }, nil } // fileFromReadSeeker returns a File pointer from a given io.ReadSeeker -func fileFromReadSeeker(n string, r io.ReadSeeker) *File { +func fileFromReadSeeker(name string, reader io.ReadSeeker) *File { return &File{ - Name: n, + Name: name, Header: make(map[string][]string), - Writer: func(w io.Writer) (int64, error) { - rb, err := io.Copy(w, r) + Writer: func(writer io.Writer) (int64, error) { + readBytes, err := io.Copy(writer, reader) if err != nil { - return rb, err + return readBytes, err } - _, err = r.Seek(0, io.SeekStart) - return rb, err + _, err = reader.Seek(0, io.SeekStart) + return readBytes, err }, } } // fileFromHTMLTemplate returns a File pointer form a given html/template.Template -func fileFromHTMLTemplate(n string, t *ht.Template, d interface{}) (*File, error) { - if t == nil { +func fileFromHTMLTemplate(name string, tpl *ht.Template, data interface{}) (*File, error) { + if tpl == nil { return nil, fmt.Errorf(errTplPointerNil) } - buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + buffer := bytes.Buffer{} + if err := tpl.Execute(&buffer, data); err != nil { return nil, fmt.Errorf(errTplExecuteFailed, err) } - return fileFromReader(n, &buf) + return fileFromReader(name, &buffer) } // fileFromTextTemplate returns a File pointer form a given text/template.Template -func fileFromTextTemplate(n string, t *tt.Template, d interface{}) (*File, error) { - if t == nil { +func fileFromTextTemplate(name string, tpl *tt.Template, data interface{}) (*File, error) { + if tpl == nil { return nil, fmt.Errorf(errTplPointerNil) } - buf := bytes.Buffer{} - if err := t.Execute(&buf, d); err != nil { + buffer := bytes.Buffer{} + if err := tpl.Execute(&buffer, data); err != nil { return nil, fmt.Errorf(errTplExecuteFailed, err) } - return fileFromReader(n, &buf) + return fileFromReader(name, &buffer) } // getEncoder creates a new mime.WordEncoder based on the encoding setting of the message -func getEncoder(e Encoding) mime.WordEncoder { - switch e { +func getEncoder(enc Encoding) mime.WordEncoder { + switch enc { case EncodingQP: return mime.QEncoding case EncodingB64: @@ -1310,10 +1324,10 @@ func getEncoder(e Encoding) mime.WordEncoder { // writeFuncFromBuffer is a common method to convert a byte buffer into a writeFunc as // often required by this library -func writeFuncFromBuffer(buf *bytes.Buffer) func(io.Writer) (int64, error) { - w := func(w io.Writer) (int64, error) { - nb, err := w.Write(buf.Bytes()) - return int64(nb), err +func writeFuncFromBuffer(buffer *bytes.Buffer) func(io.Writer) (int64, error) { + writeFunc := func(w io.Writer) (int64, error) { + numBytes, err := w.Write(buffer.Bytes()) + return int64(numBytes), err } - return w + return writeFunc } From d81dee95a8bfdba32d2fc14bf15542bb6af97ee9 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 21:38:20 +0100 Subject: [PATCH 05/18] Refactor variable and function names for improved clarity in b64linebreaker.go The existing variable and function names in the file have been updated for better readability. This change aids in code comprehension without affecting its functionality or logic. With accurately depicted names, the maintainability of the code is enhanced. --- b64linebreaker.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/b64linebreaker.go b/b64linebreaker.go index 5174984..f5fb967 100644 --- a/b64linebreaker.go +++ b/b64linebreaker.go @@ -20,39 +20,39 @@ type Base64LineBreaker struct { out io.Writer } -var nl = []byte(SingleNewLine) +var newlineBytes = []byte(SingleNewLine) // Write writes the data stream and inserts a SingleNewLine when the maximum // line length is reached -func (l *Base64LineBreaker) Write(b []byte) (n int, err error) { +func (l *Base64LineBreaker) Write(data []byte) (numBytes int, err error) { if l.out == nil { err = fmt.Errorf(ErrNoOutWriter) return } - if l.used+len(b) < MaxBodyLength { - copy(l.line[l.used:], b) - l.used += len(b) - return len(b), nil + if l.used+len(data) < MaxBodyLength { + copy(l.line[l.used:], data) + l.used += len(data) + return len(data), nil } - n, err = l.out.Write(l.line[0:l.used]) + numBytes, err = l.out.Write(l.line[0:l.used]) if err != nil { return } excess := MaxBodyLength - l.used l.used = 0 - n, err = l.out.Write(b[0:excess]) + numBytes, err = l.out.Write(data[0:excess]) if err != nil { return } - n, err = l.out.Write(nl) + numBytes, err = l.out.Write(newlineBytes) if err != nil { return } - return l.Write(b[excess:]) + return l.Write(data[excess:]) } // Close closes the Base64LineBreaker and writes any access data that is still @@ -63,7 +63,7 @@ func (l *Base64LineBreaker) Close() (err error) { if err != nil { return } - _, err = l.out.Write(nl) + _, err = l.out.Write(newlineBytes) } return From 12a145f612dcf273732705ea41efc501ee6db75e Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 21:46:48 +0100 Subject: [PATCH 06/18] Fix typos in comments in tls.go The comments within the tls.go file had multiple typographical errors which could have caused misunderstandings. These typos have been corrected to improve the clarity of the comments and maintain code understanding. --- tls.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tls.go b/tls.go index e8ccc67..bb7ad14 100644 --- a/tls.go +++ b/tls.go @@ -8,17 +8,17 @@ package mail type TLSPolicy int const ( - // TLSMandatory requires that the connection cto the server is + // TLSMandatory requires that the connection to the server is // encrypting using STARTTLS. If the server does not support STARTTLS // the connection will be terminated with an error TLSMandatory TLSPolicy = iota - // TLSOpportunistic tries cto establish an encrypted connection via the + // TLSOpportunistic tries to establish an encrypted connection via the // STARTTLS protocol. If the server does not support this, it will fall - // back cto non-encrypted plaintext transmission + // back to non-encrypted plaintext transmission TLSOpportunistic - // NoTLS forces the transaction cto be not encrypted + // NoTLS forces the transaction to be not encrypted NoTLS ) From e1098091c347f9a814adf250f83910b3d70edd88 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 24 Feb 2024 22:06:18 +0100 Subject: [PATCH 07/18] Refactor variable names in SMTP client code The commit introduces improved variable names in the SMTP client code that are more expressive and better describe the function they perform. This is combined with corrections to some code comments. It's aimed to increase code readability and maintainability. Updates are made across the client_119.go and client.go files to ensure consistent naming conventions. --- client.go | 284 ++++++++++++++++++++++++------------------------- client_119.go | 16 +-- client_120.go | 16 +-- client_test.go | 66 ++++++------ 4 files changed, 191 insertions(+), 191 deletions(-) diff --git a/client.go b/client.go index 2fbfe56..2432a73 100644 --- a/client.go +++ b/client.go @@ -87,11 +87,11 @@ type DialContextFunc func(ctx context.Context, network, address string) (net.Con // Client is the SMTP client struct type Client struct { - // co is the net.Conn that the smtp.Client is based on - co net.Conn + // connection is the net.Conn that the smtp.Client is based on + connection net.Conn // Timeout for the SMTP server connection - cto time.Duration + connTimeout time.Duration // dsn indicates that we want to use DSN for the Client dsn bool @@ -102,8 +102,8 @@ type Client struct { // dsnrntype defines the DSNRcptNotifyOption in case DSN is enabled dsnrntype []string - // enc indicates if a Client connection is encrypted or not - enc bool + // isEncrypted indicates if a Client connection is encrypted or not + isEncrypted bool // noNoop indicates the Noop is to be skipped noNoop bool @@ -121,17 +121,17 @@ type Client struct { port int fallbackPort int - // sa is a pointer to smtp.Auth - sa smtp.Auth + // smtpAuth is a pointer to smtp.Auth + smtpAuth smtp.Auth - // satype represents the authentication type for SMTP AUTH - satype SMTPAuthType + // smtpAuthType represents the authentication type for SMTP AUTH + smtpAuthType SMTPAuthType - // sc is the smtp.Client that is set up when using the Dial*() methods - sc *smtp.Client + // smtpClient is the smtp.Client that is set up when using the Dial*() methods + smtpClient *smtp.Client // Use SSL for the connection - ssl bool + useSSL bool // tlspolicy sets the client to use the provided TLSPolicy for the STARTTLS protocol tlspolicy TLSPolicy @@ -142,11 +142,11 @@ type Client struct { // user is the SMTP AUTH username user string - // dl enables the debug logging on the SMTP client - dl bool + // useDebugLog enables the debug logging on the SMTP client + useDebugLog bool - // l is a logger that implements the log.Logger interface - l log.Logger + // logger is a logger that implements the log.Logger interface + logger log.Logger // dialContextFunc is a custom DialContext function to dial target SMTP server dialContextFunc DialContextFunc @@ -198,13 +198,13 @@ var ( ) // NewClient returns a new Session client object -func NewClient(h string, o ...Option) (*Client, error) { +func NewClient(host string, opts ...Option) (*Client, error) { c := &Client{ - cto: DefaultTimeout, - host: h, - port: DefaultPort, - tlsconfig: &tls.Config{ServerName: h, MinVersion: DefaultTLSMinVersion}, - tlspolicy: DefaultTLSPolicy, + connTimeout: DefaultTimeout, + host: host, + port: DefaultPort, + tlsconfig: &tls.Config{ServerName: host, MinVersion: DefaultTLSMinVersion}, + tlspolicy: DefaultTLSPolicy, } // Set default HELO/EHLO hostname @@ -213,11 +213,11 @@ func NewClient(h string, o ...Option) (*Client, error) { } // Override defaults with optionally provided Option functions - for _, co := range o { - if co == nil { + for _, opt := range opts { + if opt == nil { continue } - if err := co(c); err != nil { + if err := opt(c); err != nil { return c, fmt.Errorf("failed to apply option: %w", err) } } @@ -231,23 +231,23 @@ func NewClient(h string, o ...Option) (*Client, error) { } // WithPort overrides the default connection port -func WithPort(p int) Option { +func WithPort(port int) Option { return func(c *Client) error { - if p < 1 || p > 65535 { + if port < 1 || port > 65535 { return ErrInvalidPort } - c.port = p + c.port = port return nil } } // WithTimeout overrides the default connection timeout -func WithTimeout(t time.Duration) Option { +func WithTimeout(timeout time.Duration) Option { return func(c *Client) error { - if t <= 0 { + if timeout <= 0 { return ErrInvalidTimeout } - c.cto = t + c.connTimeout = timeout return nil } } @@ -257,7 +257,7 @@ func WithTimeout(t time.Duration) Option { // Deprecated: use WithSSLPort instead. func WithSSL() Option { return func(c *Client) error { - c.ssl = true + c.useSSL = true return nil } } @@ -267,9 +267,9 @@ func WithSSL() Option { // // When the SSL connection fails and fallback is set to true, // the client will attempt to connect on port 25 using plaintext. -func WithSSLPort(fb bool) Option { +func WithSSLPort(fallback bool) Option { return func(c *Client) error { - c.SetSSLPort(true, fb) + c.SetSSLPort(true, fallback) return nil } } @@ -278,26 +278,26 @@ func WithSSLPort(fb bool) Option { // to StdErr func WithDebugLog() Option { return func(c *Client) error { - c.dl = true + c.useDebugLog = true return nil } } // WithLogger overrides the default log.Logger that is used for debug logging -func WithLogger(l log.Logger) Option { +func WithLogger(logger log.Logger) Option { return func(c *Client) error { - c.l = l + c.logger = logger return nil } } // WithHELO tells the client to use the provided string as HELO/EHLO greeting host -func WithHELO(h string) Option { +func WithHELO(helo string) Option { return func(c *Client) error { - if h == "" { + if helo == "" { return ErrInvalidHELO } - c.helo = h + c.helo = helo return nil } } @@ -305,9 +305,9 @@ func WithHELO(h string) Option { // WithTLSPolicy tells the client to use the provided TLSPolicy // // Deprecated: use WithTLSPortPolicy instead. -func WithTLSPolicy(p TLSPolicy) Option { +func WithTLSPolicy(policy TLSPolicy) Option { return func(c *Client) error { - c.tlspolicy = p + c.tlspolicy = policy return nil } } @@ -319,52 +319,52 @@ func WithTLSPolicy(p TLSPolicy) Option { // If the connection fails with TLSOpportunistic, // a plaintext connection is attempted on port 25 as a fallback. // NoTLS will allways use port 25. -func WithTLSPortPolicy(p TLSPolicy) Option { +func WithTLSPortPolicy(policy TLSPolicy) Option { return func(c *Client) error { - c.SetTLSPortPolicy(p) + c.SetTLSPortPolicy(policy) return nil } } // WithTLSConfig tells the client to use the provided *tls.Config -func WithTLSConfig(co *tls.Config) Option { +func WithTLSConfig(tlsconfig *tls.Config) Option { return func(c *Client) error { - if co == nil { + if tlsconfig == nil { return ErrInvalidTLSConfig } - c.tlsconfig = co + c.tlsconfig = tlsconfig return nil } } // WithSMTPAuth tells the client to use the provided SMTPAuthType for authentication -func WithSMTPAuth(t SMTPAuthType) Option { +func WithSMTPAuth(authtype SMTPAuthType) Option { return func(c *Client) error { - c.satype = t + c.smtpAuthType = authtype return nil } } // WithSMTPAuthCustom tells the client to use the provided smtp.Auth for SMTP authentication -func WithSMTPAuthCustom(a smtp.Auth) Option { +func WithSMTPAuthCustom(smtpAuth smtp.Auth) Option { return func(c *Client) error { - c.sa = a + c.smtpAuth = smtpAuth return nil } } // WithUsername tells the client to use the provided string as username for authentication -func WithUsername(u string) Option { +func WithUsername(username string) Option { return func(c *Client) error { - c.user = u + c.user = username return nil } } // WithPassword tells the client to use the provided string as password/secret for authentication -func WithPassword(p string) Option { +func WithPassword(password string) Option { return func(c *Client) error { - c.pass = p + c.pass = password return nil } } @@ -386,9 +386,9 @@ func WithDSN() Option { // as described in the RFC 1891 and set the MAIL FROM Return option type to the // given DSNMailReturnOption // See: https://www.rfc-editor.org/rfc/rfc1891 -func WithDSNMailReturnType(mro DSNMailReturnOption) Option { +func WithDSNMailReturnType(option DSNMailReturnOption) Option { return func(c *Client) error { - switch mro { + switch option { case DSNMailReturnHeadersOnly: case DSNMailReturnFull: default: @@ -396,7 +396,7 @@ func WithDSNMailReturnType(mro DSNMailReturnOption) Option { } c.dsn = true - c.dsnmrtype = mro + c.dsnmrtype = option return nil } } @@ -404,13 +404,13 @@ func WithDSNMailReturnType(mro DSNMailReturnOption) Option { // WithDSNRcptNotifyType enables the Client to request DSNs as described in the RFC 1891 // and sets the RCPT TO notify options to the given list of DSNRcptNotifyOption // See: https://www.rfc-editor.org/rfc/rfc1891 -func WithDSNRcptNotifyType(rno ...DSNRcptNotifyOption) Option { +func WithDSNRcptNotifyType(opts ...DSNRcptNotifyOption) Option { return func(c *Client) error { - var rnol []string + var rcptOpts []string var ns, nns bool - if len(rno) > 0 { - for _, crno := range rno { - switch crno { + if len(opts) > 0 { + for _, opt := range opts { + switch opt { case DSNRcptNotifyNever: ns = true case DSNRcptNotifySuccess: @@ -422,7 +422,7 @@ func WithDSNRcptNotifyType(rno ...DSNRcptNotifyOption) Option { default: return ErrInvalidDSNRcptNotifyOption } - rnol = append(rnol, string(crno)) + rcptOpts = append(rcptOpts, string(opt)) } } if ns && nns { @@ -430,7 +430,7 @@ func WithDSNRcptNotifyType(rno ...DSNRcptNotifyOption) Option { } c.dsn = true - c.dsnrntype = rnol + c.dsnrntype = rcptOpts return nil } } @@ -445,9 +445,9 @@ func WithoutNoop() Option { } // WithDialContextFunc overrides the default DialContext for connecting SMTP server -func WithDialContextFunc(f DialContextFunc) Option { +func WithDialContextFunc(dialCtxFunc DialContextFunc) Option { return func(c *Client) error { - c.dialContextFunc = f + c.dialContextFunc = dialCtxFunc return nil } } @@ -463,8 +463,8 @@ func (c *Client) ServerAddr() string { } // SetTLSPolicy overrides the current TLSPolicy with the given TLSPolicy value -func (c *Client) SetTLSPolicy(p TLSPolicy) { - c.tlspolicy = p +func (c *Client) SetTLSPolicy(policy TLSPolicy) { + c.tlspolicy = policy } // SetTLSPortPolicy overrides the current TLSPolicy with the given TLSPolicy @@ -474,22 +474,22 @@ func (c *Client) SetTLSPolicy(p TLSPolicy) { // If the connection fails with TLSOpportunistic, a plaintext connection is // attempted on port 25 as a fallback. // NoTLS will allways use port 25. -func (c *Client) SetTLSPortPolicy(p TLSPolicy) { +func (c *Client) SetTLSPortPolicy(policy TLSPolicy) { c.port = DefaultPortTLS - if p == TLSOpportunistic { + if policy == TLSOpportunistic { c.fallbackPort = DefaultPort } - if p == NoTLS { + if policy == NoTLS { c.port = DefaultPort } - c.tlspolicy = p + c.tlspolicy = policy } // SetSSL tells the Client wether to use SSL or not -func (c *Client) SetSSL(s bool) { - c.ssl = s +func (c *Client) SetSSL(ssl bool) { + c.useSSL = ssl } // SetSSLPort tells the Client wether or not to use SSL and fallback. @@ -499,124 +499,124 @@ func (c *Client) SetSSL(s bool) { // Port 25 is used when SSL is unset (false). // When the SSL connection fails and fb is set to true, // the client will attempt to connect on port 25 using plaintext. -func (c *Client) SetSSLPort(ssl bool, fb bool) { +func (c *Client) SetSSLPort(ssl bool, fallback bool) { c.port = DefaultPort if ssl { c.port = DefaultPortSSL } c.fallbackPort = 0 - if fb { + if fallback { c.fallbackPort = DefaultPort } - c.ssl = ssl + c.useSSL = ssl } // SetDebugLog tells the Client whether debug logging is enabled or not -func (c *Client) SetDebugLog(v bool) { - c.dl = v - if c.sc != nil { - c.sc.SetDebugLog(v) +func (c *Client) SetDebugLog(val bool) { + c.useDebugLog = val + if c.smtpClient != nil { + c.smtpClient.SetDebugLog(val) } } // SetLogger tells the Client which log.Logger to use -func (c *Client) SetLogger(l log.Logger) { - c.l = l - if c.sc != nil { - c.sc.SetLogger(l) +func (c *Client) SetLogger(logger log.Logger) { + c.logger = logger + if c.smtpClient != nil { + c.smtpClient.SetLogger(logger) } } // SetTLSConfig overrides the current *tls.Config with the given *tls.Config value -func (c *Client) SetTLSConfig(co *tls.Config) error { - if co == nil { +func (c *Client) SetTLSConfig(tlsconfig *tls.Config) error { + if tlsconfig == nil { return ErrInvalidTLSConfig } - c.tlsconfig = co + c.tlsconfig = tlsconfig return nil } // SetUsername overrides the current username string with the given value -func (c *Client) SetUsername(u string) { - c.user = u +func (c *Client) SetUsername(username string) { + c.user = username } // SetPassword overrides the current password string with the given value -func (c *Client) SetPassword(p string) { - c.pass = p +func (c *Client) SetPassword(password string) { + c.pass = password } // SetSMTPAuth overrides the current SMTP AUTH type setting with the given value -func (c *Client) SetSMTPAuth(a SMTPAuthType) { - c.satype = a +func (c *Client) SetSMTPAuth(authtype SMTPAuthType) { + c.smtpAuthType = authtype } // SetSMTPAuthCustom overrides the current SMTP AUTH setting with the given custom smtp.Auth -func (c *Client) SetSMTPAuthCustom(sa smtp.Auth) { - c.sa = sa +func (c *Client) SetSMTPAuthCustom(smtpAuth smtp.Auth) { + c.smtpAuth = smtpAuth } // setDefaultHelo retrieves the current hostname and sets it as HELO/EHLO hostname func (c *Client) setDefaultHelo() error { - hn, err := os.Hostname() + hostname, err := os.Hostname() if err != nil { - return fmt.Errorf("failed cto read local hostname: %w", err) + return fmt.Errorf("failed to read local hostname: %w", err) } - c.helo = hn + c.helo = hostname return nil } -// DialWithContext establishes a connection cto the SMTP server with a given context.Context -func (c *Client) DialWithContext(pc context.Context) error { - ctx, cfn := context.WithDeadline(pc, time.Now().Add(c.cto)) - defer cfn() +// DialWithContext establishes a connection to the SMTP server with a given context.Context +func (c *Client) DialWithContext(dialCtx context.Context) error { + ctx, cancel := context.WithDeadline(dialCtx, time.Now().Add(c.connTimeout)) + defer cancel() if c.dialContextFunc == nil { - nd := net.Dialer{} - c.dialContextFunc = nd.DialContext + netDialer := net.Dialer{} + c.dialContextFunc = netDialer.DialContext - if c.ssl { - td := tls.Dialer{NetDialer: &nd, Config: c.tlsconfig} - c.enc = true - c.dialContextFunc = td.DialContext + if c.useSSL { + tlsDialer := tls.Dialer{NetDialer: &netDialer, Config: c.tlsconfig} + c.isEncrypted = true + c.dialContextFunc = tlsDialer.DialContext } } var err error - c.co, err = c.dialContextFunc(ctx, "tcp", c.ServerAddr()) + c.connection, err = c.dialContextFunc(ctx, "tcp", c.ServerAddr()) if err != nil && c.fallbackPort != 0 { // TODO: should we somehow log or append the previous error? - c.co, err = c.dialContextFunc(ctx, "tcp", c.serverFallbackAddr()) + c.connection, err = c.dialContextFunc(ctx, "tcp", c.serverFallbackAddr()) } if err != nil { return err } - sc, err := smtp.NewClient(c.co, c.host) + client, err := smtp.NewClient(c.connection, c.host) if err != nil { return err } - if sc == nil { + if client == nil { return fmt.Errorf("SMTP client is nil") } - c.sc = sc + c.smtpClient = client - if c.l != nil { - c.sc.SetLogger(c.l) + if c.logger != nil { + c.smtpClient.SetLogger(c.logger) } - if c.dl { - c.sc.SetDebugLog(true) + if c.useDebugLog { + c.smtpClient.SetDebugLog(true) } - if err := c.sc.Hello(c.helo); err != nil { + if err = c.smtpClient.Hello(c.helo); err != nil { return err } - if err := c.tls(); err != nil { + if err = c.tls(); err != nil { return err } - if err := c.auth(); err != nil { + if err = c.auth(); err != nil { return err } @@ -628,7 +628,7 @@ func (c *Client) Close() error { if err := c.checkConn(); err != nil { return err } - if err := c.sc.Quit(); err != nil { + if err := c.smtpClient.Quit(); err != nil { return fmt.Errorf("failed to close SMTP client: %w", err) } @@ -640,7 +640,7 @@ func (c *Client) Reset() error { if err := c.checkConn(); err != nil { return err } - if err := c.sc.Reset(); err != nil { + if err := c.smtpClient.Reset(); err != nil { return fmt.Errorf("failed to send RSET to SMTP client: %w", err) } @@ -672,17 +672,17 @@ func (c *Client) DialAndSendWithContext(ctx context.Context, ml ...*Msg) error { // checkConn makes sure that a required server connection is available and extends the // connection deadline func (c *Client) checkConn() error { - if c.co == nil { + if c.connection == nil { return ErrNoActiveConnection } if !c.noNoop { - if err := c.sc.Noop(); err != nil { + if err := c.smtpClient.Noop(); err != nil { return ErrNoActiveConnection } } - if err := c.co.SetDeadline(time.Now().Add(c.cto)); err != nil { + if err := c.connection.SetDeadline(time.Now().Add(c.connTimeout)); err != nil { return ErrDeadlineExtendFailed } return nil @@ -696,12 +696,12 @@ func (c *Client) serverFallbackAddr() string { // tls tries to make sure that the STARTTLS requirements are satisfied func (c *Client) tls() error { - if c.co == nil { + if c.connection == nil { return ErrNoActiveConnection } - if !c.ssl && c.tlspolicy != NoTLS { + if !c.useSSL && c.tlspolicy != NoTLS { est := false - st, _ := c.sc.Extension("STARTTLS") + st, _ := c.smtpClient.Extension("STARTTLS") if c.tlspolicy == TLSMandatory { est = true if !st { @@ -715,11 +715,11 @@ func (c *Client) tls() error { } } if est { - if err := c.sc.StartTLS(c.tlsconfig); err != nil { + if err := c.smtpClient.StartTLS(c.tlsconfig); err != nil { return err } } - _, c.enc = c.sc.TLSConnectionState() + _, c.isEncrypted = c.smtpClient.TLSConnectionState() } return nil } @@ -729,40 +729,40 @@ func (c *Client) auth() error { if err := c.checkConn(); err != nil { return fmt.Errorf("failed to authenticate: %w", err) } - if c.sa == nil && c.satype != "" { - sa, sat := c.sc.Extension("AUTH") + if c.smtpAuth == nil && c.smtpAuthType != "" { + sa, sat := c.smtpClient.Extension("AUTH") if !sa { return fmt.Errorf("server does not support SMTP AUTH") } - switch c.satype { + switch c.smtpAuthType { case SMTPAuthPlain: if !strings.Contains(sat, string(SMTPAuthPlain)) { return ErrPlainAuthNotSupported } - c.sa = smtp.PlainAuth("", c.user, c.pass, c.host) + c.smtpAuth = smtp.PlainAuth("", c.user, c.pass, c.host) case SMTPAuthLogin: if !strings.Contains(sat, string(SMTPAuthLogin)) { return ErrLoginAuthNotSupported } - c.sa = smtp.LoginAuth(c.user, c.pass, c.host) + c.smtpAuth = smtp.LoginAuth(c.user, c.pass, c.host) case SMTPAuthCramMD5: if !strings.Contains(sat, string(SMTPAuthCramMD5)) { return ErrCramMD5AuthNotSupported } - c.sa = smtp.CRAMMD5Auth(c.user, c.pass) + c.smtpAuth = smtp.CRAMMD5Auth(c.user, c.pass) case SMTPAuthXOAUTH2: if !strings.Contains(sat, string(SMTPAuthXOAUTH2)) { return ErrXOauth2AuthNotSupported } - c.sa = smtp.XOAuth2Auth(c.user, c.pass) + c.smtpAuth = smtp.XOAuth2Auth(c.user, c.pass) default: - return fmt.Errorf("unsupported SMTP AUTH type %q", c.satype) + return fmt.Errorf("unsupported SMTP AUTH type %q", c.smtpAuthType) } } - if c.sa != nil { - if err := c.sc.Auth(c.sa); err != nil { + if c.smtpAuth != nil { + if err := c.smtpClient.Auth(c.smtpAuth); err != nil { return fmt.Errorf("SMTP AUTH failed: %w", err) } } diff --git a/client_119.go b/client_119.go index df09e18..060914c 100644 --- a/client_119.go +++ b/client_119.go @@ -18,7 +18,7 @@ func (c *Client) Send(ml ...*Msg) error { for _, m := range ml { m.sendError = nil if m.encoding == NoEncoding { - if ok, _ := c.sc.Extension("8BITMIME"); !ok { + if ok, _ := c.smtpClient.Extension("8BITMIME"); !ok { se := &SendError{Reason: ErrNoUnencoded, isTemp: false} m.sendError = se errs = append(errs, se) @@ -42,12 +42,12 @@ func (c *Client) Send(ml ...*Msg) error { if c.dsn { if c.dsnmrtype != "" { - c.sc.SetDSNMailReturnOption(string(c.dsnmrtype)) + c.smtpClient.SetDSNMailReturnOption(string(c.dsnmrtype)) } } - if err := c.sc.Mail(f); err != nil { + if err := c.smtpClient.Mail(f); err != nil { se := &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} - if reserr := c.sc.Reset(); reserr != nil { + if reserr := c.smtpClient.Reset(); reserr != nil { se.errlist = append(se.errlist, reserr) } m.sendError = se @@ -59,9 +59,9 @@ func (c *Client) Send(ml ...*Msg) error { rse.errlist = make([]error, 0) rse.rcpt = make([]string, 0) rno := strings.Join(c.dsnrntype, ",") - c.sc.SetDSNRcptNotifyOption(rno) + c.smtpClient.SetDSNRcptNotifyOption(rno) for _, r := range rl { - if err := c.sc.Rcpt(r); err != nil { + if err := c.smtpClient.Rcpt(r); err != nil { rse.Reason = ErrSMTPRcptTo rse.errlist = append(rse.errlist, err) rse.rcpt = append(rse.rcpt, r) @@ -70,14 +70,14 @@ func (c *Client) Send(ml ...*Msg) error { } } if failed { - if reserr := c.sc.Reset(); reserr != nil { + if reserr := c.smtpClient.Reset(); reserr != nil { rse.errlist = append(rse.errlist, err) } m.sendError = rse errs = append(errs, rse) continue } - w, err := c.sc.Data() + w, err := c.smtpClient.Data() if err != nil { se := &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} m.sendError = se diff --git a/client_120.go b/client_120.go index 1c0a5be..d6ee0a9 100644 --- a/client_120.go +++ b/client_120.go @@ -21,7 +21,7 @@ func (c *Client) Send(ml ...*Msg) (rerr error) { for _, m := range ml { m.sendError = nil if m.encoding == NoEncoding { - if ok, _ := c.sc.Extension("8BITMIME"); !ok { + if ok, _ := c.smtpClient.Extension("8BITMIME"); !ok { m.sendError = &SendError{Reason: ErrNoUnencoded, isTemp: false} rerr = errors.Join(rerr, m.sendError) continue @@ -42,13 +42,13 @@ func (c *Client) Send(ml ...*Msg) (rerr error) { if c.dsn { if c.dsnmrtype != "" { - c.sc.SetDSNMailReturnOption(string(c.dsnmrtype)) + c.smtpClient.SetDSNMailReturnOption(string(c.dsnmrtype)) } } - if err := c.sc.Mail(f); err != nil { + if err := c.smtpClient.Mail(f); err != nil { m.sendError = &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} rerr = errors.Join(rerr, m.sendError) - if reserr := c.sc.Reset(); reserr != nil { + if reserr := c.smtpClient.Reset(); reserr != nil { rerr = errors.Join(rerr, reserr) } continue @@ -58,9 +58,9 @@ func (c *Client) Send(ml ...*Msg) (rerr error) { rse.errlist = make([]error, 0) rse.rcpt = make([]string, 0) rno := strings.Join(c.dsnrntype, ",") - c.sc.SetDSNRcptNotifyOption(rno) + c.smtpClient.SetDSNRcptNotifyOption(rno) for _, r := range rl { - if err := c.sc.Rcpt(r); err != nil { + if err := c.smtpClient.Rcpt(r); err != nil { rse.Reason = ErrSMTPRcptTo rse.errlist = append(rse.errlist, err) rse.rcpt = append(rse.rcpt, r) @@ -69,14 +69,14 @@ func (c *Client) Send(ml ...*Msg) (rerr error) { } } if failed { - if reserr := c.sc.Reset(); reserr != nil { + if reserr := c.smtpClient.Reset(); reserr != nil { rerr = errors.Join(rerr, reserr) } m.sendError = rse rerr = errors.Join(rerr, m.sendError) continue } - w, err := c.sc.Data() + w, err := c.smtpClient.Data() if err != nil { m.sendError = &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} rerr = errors.Join(rerr, m.sendError) diff --git a/client_test.go b/client_test.go index 6cceb01..0ad309a 100644 --- a/client_test.go +++ b/client_test.go @@ -49,9 +49,9 @@ func TestNewClient(t *testing.T) { if c.host != tt.host { t.Errorf("failed to create new client. Host expected: %s, got: %s", host, c.host) } - if c.cto != DefaultTimeout { + if c.connTimeout != DefaultTimeout { t.Errorf("failed to create new client. Timeout expected: %s, got: %s", DefaultTimeout.String(), - c.cto.String()) + c.connTimeout.String()) } if c.port != DefaultPort { t.Errorf("failed to create new client. Port expected: %d, got: %d", DefaultPort, c.port) @@ -205,8 +205,8 @@ func TestWithTimeout(t *testing.T) { t.Errorf("failed to create new client: %s", err) return } - if c.cto != tt.want { - t.Errorf("failed to set custom timeout. Want: %d, got: %d", tt.want, c.cto) + if c.connTimeout != tt.want { + t.Errorf("failed to set custom timeout. Want: %d, got: %d", tt.want, c.connTimeout) } }) } @@ -345,8 +345,8 @@ func TestSetSSL(t *testing.T) { return } c.SetSSL(tt.value) - if c.ssl != tt.value { - t.Errorf("failed to set SSL setting. Got: %t, want: %t", c.ssl, tt.value) + if c.useSSL != tt.value { + t.Errorf("failed to set SSL setting. Got: %t, want: %t", c.useSSL, tt.value) } }) } @@ -374,8 +374,8 @@ func TestClient_SetSSLPort(t *testing.T) { return } c.SetSSLPort(tt.value, tt.fb) - if c.ssl != tt.value { - t.Errorf("failed to set SSL setting. Got: %t, want: %t", c.ssl, tt.value) + if c.useSSL != tt.value { + t.Errorf("failed to set SSL setting. Got: %t, want: %t", c.useSSL, tt.value) } if c.port != tt.port { t.Errorf("failed to set SSLPort, wanted port: %d, got: %d", c.port, tt.port) @@ -460,8 +460,8 @@ func TestSetSMTPAuth(t *testing.T) { return } c.SetSMTPAuth(tt.value) - if string(c.satype) != tt.want { - t.Errorf("failed to set SMTP auth type. Expected %s, got: %s", tt.want, string(c.satype)) + if string(c.smtpAuthType) != tt.want { + t.Errorf("failed to set SMTP auth type. Expected %s, got: %s", tt.want, string(c.smtpAuthType)) } }) } @@ -590,10 +590,10 @@ func TestSetSMTPAuthCustom(t *testing.T) { return } c.SetSMTPAuthCustom(tt.value) - if c.sa == nil { + if c.smtpAuth == nil { t.Errorf("failed to set custom SMTP auth method. SMTP Auth method is empty") } - p, _, err := c.sa.Start(&si) + p, _, err := c.smtpAuth.Start(&si) if err != nil { t.Errorf("SMTP Auth Start() method returned error: %s", err) } @@ -615,10 +615,10 @@ func TestClient_DialWithContext(t *testing.T) { t.Errorf("failed to dial with context: %s", err) return } - if c.co == nil { + if c.connection == nil { t.Errorf("DialWithContext didn't fail but no connection found.") } - if c.sc == nil { + if c.smtpClient == nil { t.Errorf("DialWithContext didn't fail but no SMTP client found.") } if err := c.Close(); err != nil { @@ -640,10 +640,10 @@ func TestClient_DialWithContext_Fallback(t *testing.T) { t.Errorf("failed to dial with context: %s", err) return } - if c.co == nil { + if c.connection == nil { t.Errorf("DialWithContext didn't fail but no connection found.") } - if c.sc == nil { + if c.smtpClient == nil { t.Errorf("DialWithContext didn't fail but no SMTP client found.") } if err := c.Close(); err != nil { @@ -670,10 +670,10 @@ func TestClient_DialWithContext_Debug(t *testing.T) { t.Errorf("failed to dial with context: %s", err) return } - if c.co == nil { + if c.connection == nil { t.Errorf("DialWithContext didn't fail but no connection found.") } - if c.sc == nil { + if c.smtpClient == nil { t.Errorf("DialWithContext didn't fail but no SMTP client found.") } c.SetDebugLog(true) @@ -694,10 +694,10 @@ func TestClient_DialWithContext_Debug_custom(t *testing.T) { t.Errorf("failed to dial with context: %s", err) return } - if c.co == nil { + if c.connection == nil { t.Errorf("DialWithContext didn't fail but no connection found.") } - if c.sc == nil { + if c.smtpClient == nil { t.Errorf("DialWithContext didn't fail but no SMTP client found.") } c.SetDebugLog(true) @@ -714,7 +714,7 @@ func TestClient_DialWithContextInvalidHost(t *testing.T) { if err != nil { t.Skipf("failed to create test client: %s. Skipping tests", err) } - c.co = nil + c.connection = nil c.host = "invalid.addr" ctx := context.Background() if err := c.DialWithContext(ctx); err == nil { @@ -730,7 +730,7 @@ func TestClient_DialWithContextInvalidHELO(t *testing.T) { if err != nil { t.Skipf("failed to create test client: %s. Skipping tests", err) } - c.co = nil + c.connection = nil c.helo = "" ctx := context.Background() if err := c.DialWithContext(ctx); err == nil { @@ -762,7 +762,7 @@ func TestClient_checkConn(t *testing.T) { if err != nil { t.Skipf("failed to create test client: %s. Skipping tests", err) } - c.co = nil + c.connection = nil if err := c.checkConn(); err == nil { t.Errorf("connCheck() should fail but succeeded") } @@ -799,10 +799,10 @@ func TestClient_DialWithContextOptions(t *testing.T) { return } if !tt.sf { - if c.co == nil && !tt.sf { + if c.connection == nil && !tt.sf { t.Errorf("DialWithContext didn't fail but no connection found.") } - if c.sc == nil && !tt.sf { + if c.smtpClient == nil && !tt.sf { t.Errorf("DialWithContext didn't fail but no SMTP client found.") } if err := c.Reset(); err != nil { @@ -1002,16 +1002,16 @@ func TestClient_DialSendCloseBroken(t *testing.T) { return } if tt.closestart { - _ = c.sc.Close() - _ = c.co.Close() + _ = c.smtpClient.Close() + _ = c.connection.Close() } if err := c.Send(m); err != nil && !tt.sf { t.Errorf("Send() failed: %s", err) return } if tt.closeearly { - _ = c.sc.Close() - _ = c.co.Close() + _ = c.smtpClient.Close() + _ = c.connection.Close() } if err := c.Close(); err != nil && !tt.sf { t.Errorf("Close() failed: %s", err) @@ -1062,16 +1062,16 @@ func TestClient_DialSendCloseBrokenWithDSN(t *testing.T) { return } if tt.closestart { - _ = c.sc.Close() - _ = c.co.Close() + _ = c.smtpClient.Close() + _ = c.connection.Close() } if err := c.Send(m); err != nil && !tt.sf { t.Errorf("Send() failed: %s", err) return } if tt.closeearly { - _ = c.sc.Close() - _ = c.co.Close() + _ = c.smtpClient.Close() + _ = c.connection.Close() } if err := c.Close(); err != nil && !tt.sf { t.Errorf("Close() failed: %s", err) From f18335fa1d855073d838e8ff04ad382ab1a0df34 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sun, 25 Feb 2024 17:48:53 +0100 Subject: [PATCH 08/18] Refactor variable names in SMTP client code Improved variable names in the SMTP client code to make them more expressive and descriptive. Also corrected several code comments to enhance clarity. These changes are intended to increase code readability and maintainability and have been implemented throughout the client_119.go and client.go files for consistency in naming conventions. --- client.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/client.go b/client.go index 2432a73..9595a8a 100644 --- a/client.go +++ b/client.go @@ -649,18 +649,18 @@ func (c *Client) Reset() error { // DialAndSend establishes a connection to the SMTP server with a // default context.Background and sends the mail -func (c *Client) DialAndSend(ml ...*Msg) error { +func (c *Client) DialAndSend(messages ...*Msg) error { ctx := context.Background() - return c.DialAndSendWithContext(ctx, ml...) + return c.DialAndSendWithContext(ctx, messages...) } // DialAndSendWithContext establishes a connection to the SMTP server with a // custom context and sends the mail -func (c *Client) DialAndSendWithContext(ctx context.Context, ml ...*Msg) error { +func (c *Client) DialAndSendWithContext(ctx context.Context, messages ...*Msg) error { if err := c.DialWithContext(ctx); err != nil { return fmt.Errorf("dial failed: %w", err) } - if err := c.Send(ml...); err != nil { + if err := c.Send(messages...); err != nil { return fmt.Errorf("send failed: %w", err) } if err := c.Close(); err != nil { @@ -700,21 +700,21 @@ func (c *Client) tls() error { return ErrNoActiveConnection } if !c.useSSL && c.tlspolicy != NoTLS { - est := false - st, _ := c.smtpClient.Extension("STARTTLS") + hasStartTLS := false + extension, _ := c.smtpClient.Extension("STARTTLS") if c.tlspolicy == TLSMandatory { - est = true - if !st { + hasStartTLS = true + if !extension { return fmt.Errorf("STARTTLS mode set to: %q, but target host does not support STARTTLS", c.tlspolicy) } } if c.tlspolicy == TLSOpportunistic { - if st { - est = true + if extension { + hasStartTLS = true } } - if est { + if hasStartTLS { if err := c.smtpClient.StartTLS(c.tlsconfig); err != nil { return err } @@ -730,29 +730,29 @@ func (c *Client) auth() error { return fmt.Errorf("failed to authenticate: %w", err) } if c.smtpAuth == nil && c.smtpAuthType != "" { - sa, sat := c.smtpClient.Extension("AUTH") - if !sa { + hasSMTPAuth, smtpAuthType := c.smtpClient.Extension("AUTH") + if !hasSMTPAuth { return fmt.Errorf("server does not support SMTP AUTH") } switch c.smtpAuthType { case SMTPAuthPlain: - if !strings.Contains(sat, string(SMTPAuthPlain)) { + if !strings.Contains(smtpAuthType, string(SMTPAuthPlain)) { return ErrPlainAuthNotSupported } c.smtpAuth = smtp.PlainAuth("", c.user, c.pass, c.host) case SMTPAuthLogin: - if !strings.Contains(sat, string(SMTPAuthLogin)) { + if !strings.Contains(smtpAuthType, string(SMTPAuthLogin)) { return ErrLoginAuthNotSupported } c.smtpAuth = smtp.LoginAuth(c.user, c.pass, c.host) case SMTPAuthCramMD5: - if !strings.Contains(sat, string(SMTPAuthCramMD5)) { + if !strings.Contains(smtpAuthType, string(SMTPAuthCramMD5)) { return ErrCramMD5AuthNotSupported } c.smtpAuth = smtp.CRAMMD5Auth(c.user, c.pass) case SMTPAuthXOAUTH2: - if !strings.Contains(sat, string(SMTPAuthXOAUTH2)) { + if !strings.Contains(smtpAuthType, string(SMTPAuthXOAUTH2)) { return ErrXOauth2AuthNotSupported } c.smtpAuth = smtp.XOAuth2Auth(c.user, c.pass) From b2f735854dc8efdbed2671a57df23cc8e9588295 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 26 Feb 2024 00:56:29 +0100 Subject: [PATCH 09/18] Refactor variable names for readability in reader.go Variable names in reader.go have been changed to be more expressive for improved readability. 'buf' and 'off' were renamed to 'buffer' and 'offset' respectively throughout the file. The changes also include corresponding adjustments in code comments and related test cases, contributing to consistent and understandable code. --- msg.go | 4 ++-- reader.go | 22 +++++++++++----------- reader_test.go | 4 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/msg.go b/msg.go index b62e83c..09012ed 100644 --- a/msg.go +++ b/msg.go @@ -1087,7 +1087,7 @@ func (m *Msg) NewReader() *Reader { if err != nil { reader.err = fmt.Errorf("failed to write Msg to Reader buffer: %w", err) } - reader.buf = buffer.Bytes() + reader.buffer = buffer.Bytes() return reader } @@ -1097,7 +1097,7 @@ func (m *Msg) UpdateReader(reader *Reader) { buffer := bytes.Buffer{} _, err := m.Write(&buffer) reader.Reset() - reader.buf = buffer.Bytes() + reader.buffer = buffer.Bytes() reader.err = err } diff --git a/reader.go b/reader.go index acde0a2..3c44f4c 100644 --- a/reader.go +++ b/reader.go @@ -10,9 +10,9 @@ import ( // Reader is a type that implements the io.Reader interface for a Msg type Reader struct { - buf []byte // contents are the bytes buf[off : len(buf)] - off int // read at &buf[off], write at &buf[len(buf)] - err error // initialization error + buffer []byte // contents are the bytes buffer[offset : len(buffer)] + offset int // read at &buffer[offset], write at &buffer[len(buffer)] + err error // initialization error } // Error returns an error if the Reader err field is not nil @@ -21,28 +21,28 @@ func (r *Reader) Error() error { } // Read reads the length of p of the Msg buffer to satisfy the io.Reader interface -func (r *Reader) Read(p []byte) (n int, err error) { +func (r *Reader) Read(payload []byte) (n int, err error) { if r.err != nil { return 0, r.err } - if r.empty() || r.buf == nil { + if r.empty() || r.buffer == nil { r.Reset() - if len(p) == 0 { + if len(payload) == 0 { return 0, nil } return 0, io.EOF } - n = copy(p, r.buf[r.off:]) - r.off += n + n = copy(payload, r.buffer[r.offset:]) + r.offset += n return n, err } // Reset resets the Reader buffer to be empty, but it retains the underlying storage // for use by future writes. func (r *Reader) Reset() { - r.buf = r.buf[:0] - r.off = 0 + r.buffer = r.buffer[:0] + r.offset = 0 } // empty reports whether the unread portion of the Reader buffer is empty. -func (r *Reader) empty() bool { return len(r.buf) <= r.off } +func (r *Reader) empty() bool { return len(r.buffer) <= r.offset } diff --git a/reader_test.go b/reader_test.go index c1c198b..fb71af7 100644 --- a/reader_test.go +++ b/reader_test.go @@ -65,7 +65,7 @@ func TestReader_Read_error(t *testing.T) { // TestReader_Read_empty tests the Reader.Read method with an empty buffer func TestReader_Read_empty(t *testing.T) { - r := Reader{buf: []byte{}} + r := Reader{buffer: []byte{}} p := make([]byte, 1) p[0] = 'a' _, err := r.Read(p) @@ -76,7 +76,7 @@ func TestReader_Read_empty(t *testing.T) { // TestReader_Read_nil tests the Reader.Read method with a nil buffer func TestReader_Read_nil(t *testing.T) { - r := Reader{buf: nil, off: -10} + r := Reader{buffer: nil, offset: -10} p := make([]byte, 0) _, err := r.Read(p) if err != nil && !errors.Is(err, io.EOF) { From 180f6f3a630f03f456322bba41b5a039382aba33 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 26 Feb 2024 21:03:20 +0100 Subject: [PATCH 10/18] Add a new test for no default user agent in msg_test.go A new test function named "TestNewMsgWithNoDefaultUserAgent" has been added in `msg_test.go` file. This function is meant to test 'NewMsg' function with 'WithNoDefaultUserAgent' parameter. The addition is devised to enhance the test coverage and ensure the noDefaultUserAgent field is functioning correctly. --- msg_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/msg_test.go b/msg_test.go index a84967d..6b71e60 100644 --- a/msg_test.go +++ b/msg_test.go @@ -3211,3 +3211,11 @@ func TestMsg_checkUserAgent(t *testing.T) { }) } } + +// TestNewMsgWithMIMEVersion tests WithMIMEVersion and Msg.SetMIMEVersion +func TestNewMsgWithNoDefaultUserAgent(t *testing.T) { + m := NewMsg(WithNoDefaultUserAgent()) + if m.noDefaultUserAgent != true { + t.Errorf("WithNoDefaultUserAgent() failed. Expected: %t, got: %t", true, false) + } +} From 21d7b367bdc3821cf69b21e14d9b9fd1e5f4bfee Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 26 Feb 2024 21:05:52 +0100 Subject: [PATCH 11/18] Update user agent test in msg_test.go Updated the test "check default user agent" in `msg_test.go` to reflect dynamic versioning. The wantUserAgent field now uses fmt.Sprintf to combine the go-mail version dynamically, improving the accuracy of testing. --- msg_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/msg_test.go b/msg_test.go index 6b71e60..af668f3 100644 --- a/msg_test.go +++ b/msg_test.go @@ -3174,7 +3174,7 @@ func TestMsg_checkUserAgent(t *testing.T) { { name: "check default user agent", noDefaultUserAgent: false, - wantUserAgent: "go-mail v0.4.1 // https://github.com/wneessen/go-mail", + wantUserAgent: fmt.Sprintf("go-mail v%s // https://github.com/wneessen/go-mail", VERSION), sf: false, }, { From d85c12220af1f2966a27e29f173398f4a6caade3 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:10:49 +0100 Subject: [PATCH 12/18] Refactor variable names in client_119.go Updated variable names in the client_119.go file to enhance readability and clarity. This observes best practices for naming conventions in Go, producing cleaner code that's easier to maintain and troubleshoot. Changes primarily consist of replacing abbreviations with full descriptive names. --- client_119.go | 124 +++++++++++++++++++++++++------------------------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/client_119.go b/client_119.go index 060914c..52e4b3f 100644 --- a/client_119.go +++ b/client_119.go @@ -10,33 +10,33 @@ package mail import "strings" // Send sends out the mail message -func (c *Client) Send(ml ...*Msg) error { +func (c *Client) Send(messages ...*Msg) error { if cerr := c.checkConn(); cerr != nil { return &SendError{Reason: ErrConnCheck, errlist: []error{cerr}, isTemp: isTempError(cerr)} } var errs []*SendError - for _, m := range ml { - m.sendError = nil - if m.encoding == NoEncoding { + for _, message := range messages { + message.sendError = nil + if message.encoding == NoEncoding { if ok, _ := c.smtpClient.Extension("8BITMIME"); !ok { - se := &SendError{Reason: ErrNoUnencoded, isTemp: false} - m.sendError = se - errs = append(errs, se) + sendErr := &SendError{Reason: ErrNoUnencoded, isTemp: false} + message.sendError = sendErr + errs = append(errs, sendErr) continue } } - f, err := m.GetSender(false) + from, err := message.GetSender(false) if err != nil { - se := &SendError{Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + sendErr := &SendError{Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } - rl, err := m.GetRecipients() + rcpts, err := message.GetRecipients() if err != nil { - se := &SendError{Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + sendErr := &SendError{Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } @@ -45,88 +45,88 @@ func (c *Client) Send(ml ...*Msg) error { c.smtpClient.SetDSNMailReturnOption(string(c.dsnmrtype)) } } - if err := c.smtpClient.Mail(f); err != nil { - se := &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} - if reserr := c.smtpClient.Reset(); reserr != nil { - se.errlist = append(se.errlist, reserr) + if err = c.smtpClient.Mail(from); err != nil { + sendErr := &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} + if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { + sendErr.errlist = append(sendErr.errlist, resetSendErr) } - m.sendError = se - errs = append(errs, se) + message.sendError = sendErr + errs = append(errs, sendErr) continue } failed := false - rse := &SendError{} - rse.errlist = make([]error, 0) - rse.rcpt = make([]string, 0) - rno := strings.Join(c.dsnrntype, ",") - c.smtpClient.SetDSNRcptNotifyOption(rno) - for _, r := range rl { - if err := c.smtpClient.Rcpt(r); err != nil { - rse.Reason = ErrSMTPRcptTo - rse.errlist = append(rse.errlist, err) - rse.rcpt = append(rse.rcpt, r) - rse.isTemp = isTempError(err) + rcptSendErr := &SendError{} + rcptSendErr.errlist = make([]error, 0) + rcptSendErr.rcpt = make([]string, 0) + rcptNotifyOpt := strings.Join(c.dsnrntype, ",") + c.smtpClient.SetDSNRcptNotifyOption(rcptNotifyOpt) + for _, rcpt := range rcpts { + if err = c.smtpClient.Rcpt(rcpt); err != nil { + rcptSendErr.Reason = ErrSMTPRcptTo + rcptSendErr.errlist = append(rcptSendErr.errlist, err) + rcptSendErr.rcpt = append(rcptSendErr.rcpt, rcpt) + rcptSendErr.isTemp = isTempError(err) failed = true } } if failed { - if reserr := c.smtpClient.Reset(); reserr != nil { - rse.errlist = append(rse.errlist, err) + if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { + rcptSendErr.errlist = append(rcptSendErr.errlist, err) } - m.sendError = rse - errs = append(errs, rse) + message.sendError = rcptSendErr + errs = append(errs, rcptSendErr) continue } - w, err := c.smtpClient.Data() + writer, err := c.smtpClient.Data() if err != nil { - se := &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + sendErr := &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } - _, err = m.WriteTo(w) + _, err = message.WriteTo(writer) if err != nil { - se := &SendError{Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + sendErr := &SendError{Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } - m.isDelivered = true + message.isDelivered = true - if err := w.Close(); err != nil { - se := &SendError{Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + if err = writer.Close(); err != nil { + sendErr := &SendError{Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } - if err := c.Reset(); err != nil { - se := &SendError{Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + if err = c.Reset(); err != nil { + sendErr := &SendError{Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } - if err := c.checkConn(); err != nil { - se := &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} - m.sendError = se - errs = append(errs, se) + if err = c.checkConn(); err != nil { + sendErr := &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} + message.sendError = sendErr + errs = append(errs, sendErr) continue } } if len(errs) > 0 { if len(errs) > 1 { - re := &SendError{Reason: ErrAmbiguous} + returnErr := &SendError{Reason: ErrAmbiguous} for i := range errs { - re.errlist = append(re.errlist, errs[i].errlist...) - re.rcpt = append(re.rcpt, errs[i].rcpt...) + returnErr.errlist = append(returnErr.errlist, errs[i].errlist...) + returnErr.rcpt = append(returnErr.rcpt, errs[i].rcpt...) } // We assume that the isTemp flag from the last error we received should be the // indicator for the returned isTemp flag as well - re.isTemp = errs[len(errs)-1].isTemp + returnErr.isTemp = errs[len(errs)-1].isTemp - return re + return returnErr } return errs[0] } From 40ea4fbfb3e9c9c08548b2d9e63bf6ac45db89bf Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:14:33 +0100 Subject: [PATCH 13/18] Refactor variable names in client_120.go Updated variable names in the client_120.go file to enhance readability and clarity. This observes best practices for naming conventions in Go, producing cleaner code that's easier to maintain and troubleshoot. Changes primarily consist of replacing abbreviations with full descriptive names. --- client_120.go | 98 +++++++++++++++++++++++++-------------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/client_120.go b/client_120.go index d6ee0a9..ed80fee 100644 --- a/client_120.go +++ b/client_120.go @@ -13,30 +13,30 @@ import ( ) // Send sends out the mail message -func (c *Client) Send(ml ...*Msg) (rerr error) { +func (c *Client) Send(messages ...*Msg) (returnErr error) { if err := c.checkConn(); err != nil { - rerr = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} return } - for _, m := range ml { - m.sendError = nil - if m.encoding == NoEncoding { + for _, message := range messages { + message.sendError = nil + if message.encoding == NoEncoding { if ok, _ := c.smtpClient.Extension("8BITMIME"); !ok { - m.sendError = &SendError{Reason: ErrNoUnencoded, isTemp: false} - rerr = errors.Join(rerr, m.sendError) + message.sendError = &SendError{Reason: ErrNoUnencoded, isTemp: false} + returnErr = errors.Join(returnErr, message.sendError) continue } } - f, err := m.GetSender(false) + from, err := message.GetSender(false) if err != nil { - m.sendError = &SendError{Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + message.sendError = &SendError{Reason: ErrGetSender, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } - rl, err := m.GetRecipients() + rcpts, err := message.GetRecipients() if err != nil { - m.sendError = &SendError{Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + message.sendError = &SendError{Reason: ErrGetRcpts, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } @@ -45,65 +45,65 @@ func (c *Client) Send(ml ...*Msg) (rerr error) { c.smtpClient.SetDSNMailReturnOption(string(c.dsnmrtype)) } } - if err := c.smtpClient.Mail(f); err != nil { - m.sendError = &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) - if reserr := c.smtpClient.Reset(); reserr != nil { - rerr = errors.Join(rerr, reserr) + if err = c.smtpClient.Mail(from); err != nil { + message.sendError = &SendError{Reason: ErrSMTPMailFrom, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) + if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { + returnErr = errors.Join(returnErr, resetSendErr) } continue } failed := false - rse := &SendError{} - rse.errlist = make([]error, 0) - rse.rcpt = make([]string, 0) - rno := strings.Join(c.dsnrntype, ",") - c.smtpClient.SetDSNRcptNotifyOption(rno) - for _, r := range rl { - if err := c.smtpClient.Rcpt(r); err != nil { - rse.Reason = ErrSMTPRcptTo - rse.errlist = append(rse.errlist, err) - rse.rcpt = append(rse.rcpt, r) - rse.isTemp = isTempError(err) + rcptSendErr := &SendError{} + rcptSendErr.errlist = make([]error, 0) + rcptSendErr.rcpt = make([]string, 0) + rcptNotifyOpt := strings.Join(c.dsnrntype, ",") + c.smtpClient.SetDSNRcptNotifyOption(rcptNotifyOpt) + for _, rcpt := range rcpts { + if err = c.smtpClient.Rcpt(rcpt); err != nil { + rcptSendErr.Reason = ErrSMTPRcptTo + rcptSendErr.errlist = append(rcptSendErr.errlist, err) + rcptSendErr.rcpt = append(rcptSendErr.rcpt, rcpt) + rcptSendErr.isTemp = isTempError(err) failed = true } } if failed { - if reserr := c.smtpClient.Reset(); reserr != nil { - rerr = errors.Join(rerr, reserr) + if resetSendErr := c.smtpClient.Reset(); resetSendErr != nil { + returnErr = errors.Join(returnErr, resetSendErr) } - m.sendError = rse - rerr = errors.Join(rerr, m.sendError) + message.sendError = rcptSendErr + returnErr = errors.Join(returnErr, message.sendError) continue } - w, err := c.smtpClient.Data() + writer, err := c.smtpClient.Data() if err != nil { - m.sendError = &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + message.sendError = &SendError{Reason: ErrSMTPData, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } - _, err = m.WriteTo(w) + _, err = message.WriteTo(writer) if err != nil { - m.sendError = &SendError{Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + message.sendError = &SendError{Reason: ErrWriteContent, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } - m.isDelivered = true + message.isDelivered = true - if err := w.Close(); err != nil { - m.sendError = &SendError{Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + if err = writer.Close(); err != nil { + message.sendError = &SendError{Reason: ErrSMTPDataClose, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } - if err := c.Reset(); err != nil { - m.sendError = &SendError{Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + if err = c.Reset(); err != nil { + message.sendError = &SendError{Reason: ErrSMTPReset, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) continue } - if err := c.checkConn(); err != nil { - m.sendError = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} - rerr = errors.Join(rerr, m.sendError) + if err = c.checkConn(); err != nil { + message.sendError = &SendError{Reason: ErrConnCheck, errlist: []error{err}, isTemp: isTempError(err)} + returnErr = errors.Join(returnErr, message.sendError) } } From 19a3cf61edc2932ced82838411b46743d284a0f1 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:21:28 +0100 Subject: [PATCH 14/18] Refactor variable names for code readability Updated variable names in multiple files to enhance code readability and maintainability by replacing abbreviations with full descriptive names. This ensures adherence to the best practices of naming conventions in Go. --- file.go | 26 ++++++++++----------- msg.go | 12 +++++----- msg_test.go | 4 ++-- msgwriter.go | 14 +++++------ part.go | 66 ++++++++++++++++++++++++++-------------------------- part_test.go | 34 +++++++++++++-------------- 6 files changed, 78 insertions(+), 78 deletions(-) diff --git a/file.go b/file.go index 5a4921c..1ad2d5a 100644 --- a/file.go +++ b/file.go @@ -23,17 +23,17 @@ type File struct { } // WithFileName sets the filename of the File -func WithFileName(n string) FileOption { +func WithFileName(name string) FileOption { return func(f *File) { - f.Name = n + f.Name = name } } // WithFileDescription sets an optional file description of the File that will be // added as Content-Description part -func WithFileDescription(d string) FileOption { +func WithFileDescription(description string) FileOption { return func(f *File) { - f.Desc = d + f.Desc = description } } @@ -41,12 +41,12 @@ func WithFileDescription(d string) FileOption { // Base64 encoding but there might be exceptions, where this might come handy. // Please note that quoted-printable should never be used for attachments/embeds. If this // is provided as argument, the function will automatically override back to Base64 -func WithFileEncoding(e Encoding) FileOption { +func WithFileEncoding(encoding Encoding) FileOption { return func(f *File) { - if e == EncodingQP { + if encoding == EncodingQP { return } - f.Enc = e + f.Enc = encoding } } @@ -56,19 +56,19 @@ func WithFileEncoding(e Encoding) FileOption { // could not be guessed. In some cases, however, it might be needed to force // this to a specific type. For such situations this override method can // be used -func WithFileContentType(t ContentType) FileOption { +func WithFileContentType(contentType ContentType) FileOption { return func(f *File) { - f.ContentType = t + f.ContentType = contentType } } // setHeader sets header fields to a File -func (f *File) setHeader(h Header, v string) { - f.Header.Set(string(h), v) +func (f *File) setHeader(header Header, value string) { + f.Header.Set(string(header), value) } // getHeader return header fields of a File -func (f *File) getHeader(h Header) (string, bool) { - v := f.Header.Get(string(h)) +func (f *File) getHeader(header Header) (string, bool) { + v := f.Header.Get(string(header)) return v, v != "" } diff --git a/msg.go b/msg.go index eeb01cf..ff3f018 100644 --- a/msg.go +++ b/msg.go @@ -723,7 +723,7 @@ func (m *Msg) SetBodyWriter( opts ...PartOption, ) { p := m.newPart(contentType, opts...) - p.w = writeFunc + p.writeFunc = writeFunc m.parts = []*Part{p} } @@ -770,7 +770,7 @@ func (m *Msg) AddAlternativeWriter( opts ...PartOption, ) { part := m.newPart(contentType, opts...) - part.w = writeFunc + part.writeFunc = writeFunc m.parts = append(m.parts, part) } @@ -1142,7 +1142,7 @@ func (m *Msg) encodeString(str string) string { func (m *Msg) hasAlt() bool { count := 0 for _, part := range m.parts { - if !part.del { + if !part.isDeleted { count++ } } @@ -1167,9 +1167,9 @@ func (m *Msg) hasPGPType() bool { // newPart returns a new Part for the Msg func (m *Msg) newPart(contentType ContentType, opts ...PartOption) *Part { p := &Part{ - ctype: contentType, - cset: m.charset, - enc: m.encoding, + contentType: contentType, + charset: m.charset, + encoding: m.encoding, } // Override defaults with optionally provided MsgOption functions diff --git a/msg_test.go b/msg_test.go index af668f3..7bcf06b 100644 --- a/msg_test.go +++ b/msg_test.go @@ -1251,7 +1251,7 @@ func TestMsg_SetBodyString(t *testing.T) { } part := m.parts[0] res := bytes.Buffer{} - if _, err := part.w(&res); err != nil && !tt.sf { + if _, err := part.writeFunc(&res); err != nil && !tt.sf { t.Errorf("WriteFunc of part failed: %s", err) } if res.String() != tt.want { @@ -1286,7 +1286,7 @@ func TestMsg_AddAlternativeString(t *testing.T) { } apart := m.parts[1] res := bytes.Buffer{} - if _, err := apart.w(&res); err != nil && !tt.sf { + if _, err := apart.writeFunc(&res); err != nil && !tt.sf { t.Errorf("WriteFunc of part failed: %s", err) } if res.String() != tt.want { diff --git a/msgwriter.go b/msgwriter.go index e584a99..08073db 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -114,7 +114,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } for _, part := range msg.parts { - if !part.del { + if !part.isDeleted { mw.writePart(part, msg.charset) } } @@ -249,12 +249,12 @@ func (mw *msgWriter) newPart(header map[string][]string) { // writePart writes the corresponding part to the Msg body func (mw *msgWriter) writePart(part *Part, charset Charset) { - partCharset := part.cset + partCharset := part.charset if partCharset.String() == "" { partCharset = charset } - contentType := fmt.Sprintf("%s; charset=%s", part.ctype, partCharset) - contentTransferEnc := part.enc.String() + contentType := fmt.Sprintf("%s; charset=%s", part.contentType, partCharset) + contentTransferEnc := part.encoding.String() if mw.depth == 0 { mw.writeHeader(HeaderContentType, contentType) mw.writeHeader(HeaderContentTransferEnc, contentTransferEnc) @@ -262,14 +262,14 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { } if mw.depth > 0 { mimeHeader := textproto.MIMEHeader{} - if part.desc != "" { - mimeHeader.Add(string(HeaderContentDescription), part.desc) + if part.description != "" { + mimeHeader.Add(string(HeaderContentDescription), part.description) } mimeHeader.Add(string(HeaderContentType), contentType) mimeHeader.Add(string(HeaderContentTransferEnc), contentTransferEnc) mw.newPart(mimeHeader) } - mw.writeBody(part.w, part.enc) + mw.writeBody(part.writeFunc, part.encoding) } // writeString writes a string into the msgWriter's io.Writer interface diff --git a/part.go b/part.go index 78e8aed..7c76b7d 100644 --- a/part.go +++ b/part.go @@ -14,18 +14,18 @@ type PartOption func(*Part) // Part is a part of the Msg type Part struct { - ctype ContentType - cset Charset - desc string - enc Encoding - del bool - w func(io.Writer) (int64, error) + contentType ContentType + charset Charset + description string + encoding Encoding + isDeleted bool + writeFunc func(io.Writer) (int64, error) } // GetContent executes the WriteFunc of the Part and returns the content as byte slice func (p *Part) GetContent() ([]byte, error) { var b bytes.Buffer - if _, err := p.w(&b); err != nil { + if _, err := p.writeFunc(&b); err != nil { return nil, err } return b.Bytes(), nil @@ -33,83 +33,83 @@ func (p *Part) GetContent() ([]byte, error) { // GetCharset returns the currently set Charset of the Part func (p *Part) GetCharset() Charset { - return p.cset + return p.charset } // GetContentType returns the currently set ContentType of the Part func (p *Part) GetContentType() ContentType { - return p.ctype + return p.contentType } // GetEncoding returns the currently set Encoding of the Part func (p *Part) GetEncoding() Encoding { - return p.enc + return p.encoding } // GetWriteFunc returns the currently set WriterFunc of the Part func (p *Part) GetWriteFunc() func(io.Writer) (int64, error) { - return p.w + return p.writeFunc } // GetDescription returns the currently set Content-Description of the Part func (p *Part) GetDescription() string { - return p.desc + return p.description } // SetContent overrides the content of the Part with the given string -func (p *Part) SetContent(c string) { - buf := bytes.NewBufferString(c) - p.w = writeFuncFromBuffer(buf) +func (p *Part) SetContent(content string) { + buffer := bytes.NewBufferString(content) + p.writeFunc = writeFuncFromBuffer(buffer) } // SetContentType overrides the ContentType of the Part -func (p *Part) SetContentType(c ContentType) { - p.ctype = c +func (p *Part) SetContentType(contentType ContentType) { + p.contentType = contentType } // SetCharset overrides the Charset of the Part -func (p *Part) SetCharset(c Charset) { - p.cset = c +func (p *Part) SetCharset(charset Charset) { + p.charset = charset } // SetEncoding creates a new mime.WordEncoder based on the encoding setting of the message -func (p *Part) SetEncoding(e Encoding) { - p.enc = e +func (p *Part) SetEncoding(encoding Encoding) { + p.encoding = encoding } // SetDescription overrides the Content-Description of the Part -func (p *Part) SetDescription(d string) { - p.desc = d +func (p *Part) SetDescription(description string) { + p.description = description } // SetWriteFunc overrides the WriteFunc of the Part -func (p *Part) SetWriteFunc(w func(io.Writer) (int64, error)) { - p.w = w +func (p *Part) SetWriteFunc(writeFunc func(io.Writer) (int64, error)) { + p.writeFunc = writeFunc } // Delete removes the current part from the parts list of the Msg by setting the -// del flag to true. The msgWriter will skip it then +// isDeleted flag to true. The msgWriter will skip it then func (p *Part) Delete() { - p.del = true + p.isDeleted = true } // WithPartCharset overrides the default Part charset -func WithPartCharset(c Charset) PartOption { +func WithPartCharset(charset Charset) PartOption { return func(p *Part) { - p.cset = c + p.charset = charset } } // WithPartEncoding overrides the default Part encoding -func WithPartEncoding(e Encoding) PartOption { +func WithPartEncoding(encoding Encoding) PartOption { return func(p *Part) { - p.enc = e + p.encoding = encoding } } // WithPartContentDescription overrides the default Part Content-Description -func WithPartContentDescription(d string) PartOption { +func WithPartContentDescription(description string) PartOption { return func(p *Part) { - p.desc = d + p.description = description } } diff --git a/part_test.go b/part_test.go index 276fc69..a196986 100644 --- a/part_test.go +++ b/part_test.go @@ -31,15 +31,15 @@ func TestPartEncoding(t *testing.T) { t.Errorf("newPart() WithPartEncoding() failed: no part returned") return } - if part.enc.String() != tt.want { + if part.encoding.String() != tt.want { t.Errorf("newPart() WithPartEncoding() failed: expected encoding: %s, got: %s", tt.want, - part.enc.String()) + part.encoding.String()) } - part.enc = "" + part.encoding = "" part.SetEncoding(tt.enc) - if part.enc.String() != tt.want { + if part.encoding.String() != tt.want { t.Errorf("newPart() SetEncoding() failed: expected encoding: %s, got: %s", tt.want, - part.enc.String()) + part.encoding.String()) } }) } @@ -64,9 +64,9 @@ func TestWithPartCharset(t *testing.T) { t.Errorf("newPart() WithPartCharset() failed: no part returned") return } - if part.cset.String() != tt.want { + if part.charset.String() != tt.want { t.Errorf("newPart() WithPartCharset() failed: expected charset: %s, got: %s", - tt.want, part.cset.String()) + tt.want, part.charset.String()) } }) } @@ -89,14 +89,14 @@ func TestPart_WithPartContentDescription(t *testing.T) { t.Errorf("newPart() WithPartContentDescription() failed: no part returned") return } - if part.desc != tt.desc { + if part.description != tt.desc { t.Errorf("newPart() WithPartContentDescription() failed: expected: %s, got: %s", tt.desc, - part.desc) + part.description) } - part.desc = "" + part.description = "" part.SetDescription(tt.desc) - if part.desc != tt.desc { - t.Errorf("newPart() SetDescription() failed: expected: %s, got: %s", tt.desc, part.desc) + if part.description != tt.desc { + t.Errorf("newPart() SetDescription() failed: expected: %s, got: %s", tt.desc, part.description) } }) } @@ -236,7 +236,7 @@ func TestPart_GetContentBroken(t *testing.T) { t.Errorf("failed: %s", err) return } - pl[0].w = func(io.Writer) (int64, error) { + pl[0].writeFunc = func(io.Writer) (int64, error) { return 0, fmt.Errorf("broken") } _, err = pl[0].GetContent() @@ -314,8 +314,8 @@ func TestPart_SetDescription(t *testing.T) { t.Errorf("Part.GetDescription failed. Expected empty description but got: %s", pd) } pl[0].SetDescription(d) - if pl[0].desc != d { - t.Errorf("Part.SetDescription failed. Expected desc to be: %s, got: %s", d, pd) + if pl[0].description != d { + t.Errorf("Part.SetDescription failed. Expected description to be: %s, got: %s", d, pd) } pd = pl[0].GetDescription() if pd != d { @@ -334,8 +334,8 @@ func TestPart_Delete(t *testing.T) { return } pl[0].Delete() - if !pl[0].del { - t.Errorf("Delete failed. Expected: %t, got: %t", true, pl[0].del) + if !pl[0].isDeleted { + t.Errorf("Delete failed. Expected: %t, got: %t", true, pl[0].isDeleted) } } From 7de4f5053ffe46a9089058a99186450e4e9a90af Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:30:09 +0100 Subject: [PATCH 15/18] Refactor variable names for better clarity Adjusted variable names in several functions and improved code readability. Full, descriptive names have replaced abbreviations to make the codes self-explanatory. The change is aimed at enhancing code maintenance and adhering to Go's best practices for naming conventions. --- random.go | 63 +++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/random.go b/random.go index 27305db..ed663f2 100644 --- a/random.go +++ b/random.go @@ -22,54 +22,53 @@ const ( letterIdxMax = 63 / letterIdxBits // # of letter indices fitting in 63 bits ) -// randomStringSecure returns a random, n long string of characters. The character set is based -// on the s (special chars) and h (human readable) boolean arguments. This method uses the +// randomStringSecure returns a random, string of length characters. This method uses the // crypto/random package and therfore is cryptographically secure -func randomStringSecure(n int) (string, error) { - rs := strings.Builder{} - rs.Grow(n) - crl := len(cr) +func randomStringSecure(length int) (string, error) { + randString := strings.Builder{} + randString.Grow(length) + charRangeLength := len(cr) - rp := make([]byte, 8) - _, err := rand.Read(rp) + randPool := make([]byte, 8) + _, err := rand.Read(randPool) if err != nil { - return rs.String(), err + return randString.String(), err } - for i, c, r := n-1, binary.BigEndian.Uint64(rp), letterIdxMax; i >= 0; { - if r == 0 { - _, err := rand.Read(rp) + for idx, char, rest := length-1, binary.BigEndian.Uint64(randPool), letterIdxMax; idx >= 0; { + if rest == 0 { + _, err = rand.Read(randPool) if err != nil { - return rs.String(), err + return randString.String(), err } - c, r = binary.BigEndian.Uint64(rp), letterIdxMax + char, rest = binary.BigEndian.Uint64(randPool), letterIdxMax } - if idx := int(c & letterIdxMask); idx < crl { - rs.WriteByte(cr[idx]) - i-- + if idx := int(char & letterIdxMask); idx < charRangeLength { + randString.WriteByte(cr[idx]) + idx-- } - c >>= letterIdxBits - r-- + char >>= letterIdxBits + rest-- } - return rs.String(), nil + return randString.String(), nil } -// randNum returns a random number with a maximum value of n -func randNum(n int) (int, error) { - if n <= 0 { - return 0, fmt.Errorf("provided number is <= 0: %d", n) +// randNum returns a random number with a maximum value of length +func randNum(length int) (int, error) { + if length <= 0 { + return 0, fmt.Errorf("provided number is <= 0: %d", length) } - mbi := big.NewInt(int64(n)) - if !mbi.IsUint64() { - return 0, fmt.Errorf("big.NewInt() generation returned negative value: %d", mbi) + length64 := big.NewInt(int64(length)) + if !length64.IsUint64() { + return 0, fmt.Errorf("big.NewInt() generation returned negative value: %d", length64) } - rn64, err := rand.Int(rand.Reader, mbi) + randNum64, err := rand.Int(rand.Reader, length64) if err != nil { return 0, err } - rn := int(rn64.Int64()) - if rn < 0 { - return 0, fmt.Errorf("generated random number does not fit as int64: %d", rn64) + randomNum := int(randNum64.Int64()) + if randomNum < 0 { + return 0, fmt.Errorf("generated random number does not fit as int64: %d", randNum64) } - return rn, nil + return randomNum, nil } From 077cf47973bc667c4e1b85c9a7642048117ae41b Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:32:12 +0100 Subject: [PATCH 16/18] Refactor variable names for better clarity Changed variable names for more clarity in senderror.go. This change has converted abbreviated variable names into meaningful and self-explanatory variables. This effort improves code readability and maintainability following Go's best practices. --- senderror.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/senderror.go b/senderror.go index 34037d1..dfe7502 100644 --- a/senderror.go +++ b/senderror.go @@ -71,34 +71,34 @@ func (e *SendError) Error() string { return "unknown reason" } - var em strings.Builder - em.WriteString(e.Reason.String()) + var errMessage strings.Builder + errMessage.WriteString(e.Reason.String()) if len(e.errlist) > 0 { - em.WriteRune(':') + errMessage.WriteRune(':') for i := range e.errlist { - em.WriteRune(' ') - em.WriteString(e.errlist[i].Error()) + errMessage.WriteRune(' ') + errMessage.WriteString(e.errlist[i].Error()) if i != len(e.errlist)-1 { - em.WriteString(", ") + errMessage.WriteString(", ") } } } if len(e.rcpt) > 0 { - em.WriteString(", affected recipient(s): ") + errMessage.WriteString(", affected recipient(s): ") for i := range e.rcpt { - em.WriteString(e.rcpt[i]) + errMessage.WriteString(e.rcpt[i]) if i != len(e.rcpt)-1 { - em.WriteString(", ") + errMessage.WriteString(", ") } } } - return em.String() + return errMessage.String() } // Is implements the errors.Is functionality and compares the SendErrReason -func (e *SendError) Is(et error) bool { +func (e *SendError) Is(errType error) bool { var t *SendError - if errors.As(et, &t) && t != nil { + if errors.As(errType, &t) && t != nil { return e.Reason == t.Reason && e.isTemp == t.isTemp } return false @@ -143,6 +143,6 @@ func (r SendErrReason) String() string { // isTempError checks the given SMTP error and returns true if the given error is of temporary nature // and should be retried -func isTempError(e error) bool { - return e.Error()[0] == '4' +func isTempError(err error) bool { + return err.Error()[0] == '4' } From 6c9b875f580064e84b5939601a238e17c074612a Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:35:37 +0100 Subject: [PATCH 17/18] Refactor variable names in log files Updated variable names from abbreviations to full names for better code readability in the log and test log files. This modification provides a clearer understanding of variable roles and improves code maintainability in adherence to Go's best practices. --- log/jsonlog.go | 66 ++++++++++++++++++++++----------------------- log/jsonlog_test.go | 14 +++++----- log/stdlog.go | 46 +++++++++++++++---------------- log/stdlog_test.go | 12 ++++----- 4 files changed, 69 insertions(+), 69 deletions(-) diff --git a/log/jsonlog.go b/log/jsonlog.go index c0b517e..5e47880 100644 --- a/log/jsonlog.go +++ b/log/jsonlog.go @@ -15,68 +15,68 @@ import ( // JSONlog is the default structured JSON logger that satisfies the Logger interface type JSONlog struct { - l Level - log *slog.Logger + level Level + log *slog.Logger } // NewJSON returns a new JSONlog type that satisfies the Logger interface -func NewJSON(o io.Writer, l Level) *JSONlog { - lo := slog.HandlerOptions{} - switch l { +func NewJSON(output io.Writer, level Level) *JSONlog { + logOpts := slog.HandlerOptions{} + switch level { case LevelDebug: - lo.Level = slog.LevelDebug + logOpts.Level = slog.LevelDebug case LevelInfo: - lo.Level = slog.LevelInfo + logOpts.Level = slog.LevelInfo case LevelWarn: - lo.Level = slog.LevelWarn + logOpts.Level = slog.LevelWarn case LevelError: - lo.Level = slog.LevelError + logOpts.Level = slog.LevelError default: - lo.Level = slog.LevelDebug + logOpts.Level = slog.LevelDebug } - lh := slog.NewJSONHandler(o, &lo) + logHandler := slog.NewJSONHandler(output, &logOpts) return &JSONlog{ - l: l, - log: slog.New(lh), + level: level, + log: slog.New(logHandler), } } // Debugf logs a debug message via the structured JSON logger -func (l *JSONlog) Debugf(lo Log) { - if l.l >= LevelDebug { +func (l *JSONlog) Debugf(log Log) { + if l.level >= LevelDebug { l.log.WithGroup(DirString).With( - slog.String(DirFromString, lo.directionFrom()), - slog.String(DirToString, lo.directionTo()), - ).Debug(fmt.Sprintf(lo.Format, lo.Messages...)) + slog.String(DirFromString, log.directionFrom()), + slog.String(DirToString, log.directionTo()), + ).Debug(fmt.Sprintf(log.Format, log.Messages...)) } } // Infof logs a info message via the structured JSON logger -func (l *JSONlog) Infof(lo Log) { - if l.l >= LevelInfo { +func (l *JSONlog) Infof(log Log) { + if l.level >= LevelInfo { l.log.WithGroup(DirString).With( - slog.String(DirFromString, lo.directionFrom()), - slog.String(DirToString, lo.directionTo()), - ).Info(fmt.Sprintf(lo.Format, lo.Messages...)) + slog.String(DirFromString, log.directionFrom()), + slog.String(DirToString, log.directionTo()), + ).Info(fmt.Sprintf(log.Format, log.Messages...)) } } // Warnf logs a warn message via the structured JSON logger -func (l *JSONlog) Warnf(lo Log) { - if l.l >= LevelWarn { +func (l *JSONlog) Warnf(log Log) { + if l.level >= LevelWarn { l.log.WithGroup(DirString).With( - slog.String(DirFromString, lo.directionFrom()), - slog.String(DirToString, lo.directionTo()), - ).Warn(fmt.Sprintf(lo.Format, lo.Messages...)) + slog.String(DirFromString, log.directionFrom()), + slog.String(DirToString, log.directionTo()), + ).Warn(fmt.Sprintf(log.Format, log.Messages...)) } } // Errorf logs a warn message via the structured JSON logger -func (l *JSONlog) Errorf(lo Log) { - if l.l >= LevelError { +func (l *JSONlog) Errorf(log Log) { + if l.level >= LevelError { l.log.WithGroup(DirString).With( - slog.String(DirFromString, lo.directionFrom()), - slog.String(DirToString, lo.directionTo()), - ).Error(fmt.Sprintf(lo.Format, lo.Messages...)) + slog.String(DirFromString, log.directionFrom()), + slog.String(DirToString, log.directionTo()), + ).Error(fmt.Sprintf(log.Format, log.Messages...)) } } diff --git a/log/jsonlog_test.go b/log/jsonlog_test.go index 1f32317..b14242d 100644 --- a/log/jsonlog_test.go +++ b/log/jsonlog_test.go @@ -30,8 +30,8 @@ type jsonDir struct { func TestNewJSON(t *testing.T) { var b bytes.Buffer l := NewJSON(&b, LevelDebug) - if l.l != LevelDebug { - t.Error("Expected level to be LevelDebug, got ", l.l) + if l.level != LevelDebug { + t.Error("Expected level to be LevelDebug, got ", l.level) } if l.log == nil { t.Error("logger not initialized") @@ -81,7 +81,7 @@ func TestJSONDebugf(t *testing.T) { } b.Reset() - l.l = LevelInfo + l.level = LevelInfo l.Debugf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Debug message was not expected to be logged") @@ -131,7 +131,7 @@ func TestJSONDebugf_WithDefault(t *testing.T) { } b.Reset() - l.l = LevelInfo + l.level = LevelInfo l.Debugf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Debug message was not expected to be logged") @@ -181,7 +181,7 @@ func TestJSONInfof(t *testing.T) { } b.Reset() - l.l = LevelWarn + l.level = LevelWarn l.Infof(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Info message was not expected to be logged") @@ -231,7 +231,7 @@ func TestJSONWarnf(t *testing.T) { } b.Reset() - l.l = LevelError + l.level = LevelError l.Warnf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Warn message was not expected to be logged") @@ -281,7 +281,7 @@ func TestJSONErrorf(t *testing.T) { } b.Reset() - l.l = -99 + l.level = -99 l.Errorf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Error message was not expected to be logged") diff --git a/log/stdlog.go b/log/stdlog.go index 2ac2430..64c4bbf 100644 --- a/log/stdlog.go +++ b/log/stdlog.go @@ -12,7 +12,7 @@ import ( // Stdlog is the default logger that satisfies the Logger interface type Stdlog struct { - l Level + level Level err *log.Logger warn *log.Logger info *log.Logger @@ -24,45 +24,45 @@ type Stdlog struct { const CallDepth = 2 // New returns a new Stdlog type that satisfies the Logger interface -func New(o io.Writer, l Level) *Stdlog { +func New(output io.Writer, level Level) *Stdlog { lf := log.Lmsgprefix | log.LstdFlags return &Stdlog{ - l: l, - err: log.New(o, "ERROR: ", lf), - warn: log.New(o, " WARN: ", lf), - info: log.New(o, " INFO: ", lf), - debug: log.New(o, "DEBUG: ", lf), + level: level, + err: log.New(output, "ERROR: ", lf), + warn: log.New(output, " WARN: ", lf), + info: log.New(output, " INFO: ", lf), + debug: log.New(output, "DEBUG: ", lf), } } // Debugf performs a Printf() on the debug logger -func (l *Stdlog) Debugf(lo Log) { - if l.l >= LevelDebug { - f := fmt.Sprintf("%s %s", lo.directionPrefix(), lo.Format) - _ = l.debug.Output(CallDepth, fmt.Sprintf(f, lo.Messages...)) +func (l *Stdlog) Debugf(log Log) { + if l.level >= LevelDebug { + format := fmt.Sprintf("%s %s", log.directionPrefix(), log.Format) + _ = l.debug.Output(CallDepth, fmt.Sprintf(format, log.Messages...)) } } // Infof performs a Printf() on the info logger -func (l *Stdlog) Infof(lo Log) { - if l.l >= LevelInfo { - f := fmt.Sprintf("%s %s", lo.directionPrefix(), lo.Format) - _ = l.info.Output(CallDepth, fmt.Sprintf(f, lo.Messages...)) +func (l *Stdlog) Infof(log Log) { + if l.level >= LevelInfo { + format := fmt.Sprintf("%s %s", log.directionPrefix(), log.Format) + _ = l.info.Output(CallDepth, fmt.Sprintf(format, log.Messages...)) } } // Warnf performs a Printf() on the warn logger -func (l *Stdlog) Warnf(lo Log) { - if l.l >= LevelWarn { - f := fmt.Sprintf("%s %s", lo.directionPrefix(), lo.Format) - _ = l.warn.Output(CallDepth, fmt.Sprintf(f, lo.Messages...)) +func (l *Stdlog) Warnf(log Log) { + if l.level >= LevelWarn { + format := fmt.Sprintf("%s %s", log.directionPrefix(), log.Format) + _ = l.warn.Output(CallDepth, fmt.Sprintf(format, log.Messages...)) } } // Errorf performs a Printf() on the error logger -func (l *Stdlog) Errorf(lo Log) { - if l.l >= LevelError { - f := fmt.Sprintf("%s %s", lo.directionPrefix(), lo.Format) - _ = l.err.Output(CallDepth, fmt.Sprintf(f, lo.Messages...)) +func (l *Stdlog) Errorf(log Log) { + if l.level >= LevelError { + format := fmt.Sprintf("%s %s", log.directionPrefix(), log.Format) + _ = l.err.Output(CallDepth, fmt.Sprintf(format, log.Messages...)) } } diff --git a/log/stdlog_test.go b/log/stdlog_test.go index d8e781e..8667667 100644 --- a/log/stdlog_test.go +++ b/log/stdlog_test.go @@ -13,8 +13,8 @@ import ( func TestNew(t *testing.T) { var b bytes.Buffer l := New(&b, LevelDebug) - if l.l != LevelDebug { - t.Error("Expected level to be LevelDebug, got ", l.l) + if l.level != LevelDebug { + t.Error("Expected level to be LevelDebug, got ", l.level) } if l.err == nil || l.warn == nil || l.info == nil || l.debug == nil { t.Error("Loggers not initialized") @@ -37,7 +37,7 @@ func TestDebugf(t *testing.T) { } b.Reset() - l.l = LevelInfo + l.level = LevelInfo l.Debugf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Debug message was not expected to be logged") @@ -60,7 +60,7 @@ func TestInfof(t *testing.T) { } b.Reset() - l.l = LevelWarn + l.level = LevelWarn l.Infof(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Info message was not expected to be logged") @@ -83,7 +83,7 @@ func TestWarnf(t *testing.T) { } b.Reset() - l.l = LevelError + l.level = LevelError l.Warnf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Warn message was not expected to be logged") @@ -106,7 +106,7 @@ func TestErrorf(t *testing.T) { } b.Reset() - l.l = LevelError - 1 + l.level = LevelError - 1 l.Errorf(Log{Direction: DirServerToClient, Format: "test %s", Messages: []interface{}{"foo"}}) if b.String() != "" { t.Error("Error message was not expected to be logged") From 514e68be5410aa6f6e8ed1ba982af11ea1c342ec Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 27 Feb 2024 11:43:05 +0100 Subject: [PATCH 18/18] Update variable name in random.go Changed the abbreviation 'idx' to 'i' in the 'random.go' file. This change makes the variable roles much clearer and aligns the naming with Go's best practice, enhancing readability and maintainability. --- random.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/random.go b/random.go index ed663f2..831b118 100644 --- a/random.go +++ b/random.go @@ -42,8 +42,8 @@ func randomStringSecure(length int) (string, error) { } char, rest = binary.BigEndian.Uint64(randPool), letterIdxMax } - if idx := int(char & letterIdxMask); idx < charRangeLength { - randString.WriteByte(cr[idx]) + if i := int(char & letterIdxMask); i < charRangeLength { + randString.WriteByte(cr[i]) idx-- } char >>= letterIdxBits