From d0566f7f6ff3d503a6d1cbcb5167b8f7996fafac Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sun, 19 Apr 2026 12:59:04 -0700 Subject: [PATCH 1/4] fix: SMTPS support, sync.Pool copy, printf-vet warnings; align toolchain with echo email: stdlib smtp.SendMail speaks plain + STARTTLS only; port 465 (Resend, SendGrid, etc.) hangs on the handshake. Detect port 465 and dial TLS directly via tls.DialWithDialer. Expose an optional TLSConfig for callers needing a custom root pool or ServerName, and DialTimeout so misconfigured hosts fail fast. Public Send API is unchanged. New tests cover plain dial, timeout, and invalid addresses. random: sync.Pool must not be copied after first use; New was copying from a local var into the Random struct. Construct the pool directly on the struct literal to satisfy go vet and prevent mutex corruption under concurrent use. log: the internal (*Logger).log method overloaded three modes (Sprint, Sprintf, JSON) via a format-or-sentinel string, which go vet flagged with 14 "call has arguments but no formatting directives" warnings. Split into log(level, message, jsonBody) and logJSON(level, j). The new logJSON wrapper adds one stack frame, so log bumps runtime.Caller's skip when jsonBody is true to keep the reported file:line on the user's call site. TestCallerFile guards against future skip drift. toolchain: bump go directive to 1.23.0 and CI matrix to 1.23-1.26 to match labstack/echo. Refresh direct deps: testify 1.8.4 -> 1.11.1, go-colorable 0.1.13 -> 0.1.14, go-isatty 0.0.20 -> 0.0.21. Pin x/sys at v0.29.0 (latest that doesn't require Go 1.25+). Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/gommon.yml | 6 +- email/email.go | 74 ++++++++++++--- email/email_test.go | 86 +++++++++++++++++ go.mod | 10 +- go.sum | 19 ++-- log/log.go | 178 ++++++++++++++++++----------------- log/log_test.go | 19 ++++ random/random.go | 13 ++- 8 files changed, 281 insertions(+), 124 deletions(-) diff --git a/.github/workflows/gommon.yml b/.github/workflows/gommon.yml index 108a0fa..f31cf71 100644 --- a/.github/workflows/gommon.yml +++ b/.github/workflows/gommon.yml @@ -23,7 +23,7 @@ on: env: # run static analysis only with the latest Go version - LATEST_GO_VERSION: "1.21" + LATEST_GO_VERSION: "1.26" jobs: test: @@ -31,8 +31,8 @@ jobs: matrix: os: [ubuntu-latest, macos-latest, windows-latest] # Each major Go release is supported until there are two newer major releases. https://golang.org/doc/devel/release.html#policy - # Echo tests with last four major releases - go: ["1.18","1.19","1.20","1.21"] + # Matches the go directive floor in go.mod. + go: ["1.23","1.24","1.25","1.26"] name: ${{ matrix.os }} @ Go ${{ matrix.go }} runs-on: ${{ matrix.os }} steps: diff --git a/email/email.go b/email/email.go index ea0383c..f48ea6e 100644 --- a/email/email.go +++ b/email/email.go @@ -14,9 +14,16 @@ import ( type ( Email struct { - Auth smtp.Auth - Header map[string]string - Template *template.Template + Auth smtp.Auth + Header map[string]string + Template *template.Template + // TLSConfig, when non-nil, is used for both implicit TLS (SMTPS) + // and STARTTLS. Callers that need a custom root pool or a + // specific server name should set this. + TLSConfig *tls.Config + // DialTimeout caps how long the initial connection waits. + // Zero means no caller-imposed timeout. + DialTimeout time.Duration smtpAddress string } @@ -124,22 +131,14 @@ func (e *Email) Send(m *Message) (err error) { m.buffer.WriteString(m.boundary) m.buffer.WriteString("--") - // Dial - c, err := smtp.Dial(e.smtpAddress) + // Dial. Port 465 is SMTPS (implicit TLS) per IANA; everything else + // connects plaintext and upgrades via STARTTLS when advertised. + c, err := e.dial() if err != nil { return } defer c.Quit() - // Check if TLS is required - if ok, _ := c.Extension("STARTTLS"); ok { - host, _, _ := net.SplitHostPort(e.smtpAddress) - config := &tls.Config{ServerName: host} - if err = c.StartTLS(config); err != nil { - return err - } - } - // Authenticate if e.Auth != nil { if err = c.Auth(e.Auth); err != nil { @@ -172,3 +171,50 @@ func (e *Email) Send(m *Message) (err error) { _, err = m.buffer.WriteTo(wc) return } + +func (e *Email) dial() (*smtp.Client, error) { + host, port, err := net.SplitHostPort(e.smtpAddress) + if err != nil { + return nil, err + } + + tlsConfig := e.TLSConfig + if tlsConfig == nil { + tlsConfig = &tls.Config{ServerName: host} + } else if tlsConfig.ServerName == "" { + tlsConfig = tlsConfig.Clone() + tlsConfig.ServerName = host + } + + dialer := &net.Dialer{Timeout: e.DialTimeout} + + if port == "465" { + conn, err := tls.DialWithDialer(dialer, "tcp", e.smtpAddress, tlsConfig) + if err != nil { + return nil, err + } + c, err := smtp.NewClient(conn, host) + if err != nil { + conn.Close() + return nil, err + } + return c, nil + } + + conn, err := dialer.Dial("tcp", e.smtpAddress) + if err != nil { + return nil, err + } + c, err := smtp.NewClient(conn, host) + if err != nil { + conn.Close() + return nil, err + } + if ok, _ := c.Extension("STARTTLS"); ok { + if err := c.StartTLS(tlsConfig); err != nil { + c.Close() + return nil, err + } + } + return c, nil +} diff --git a/email/email_test.go b/email/email_test.go index 8774a6e..9eb653b 100644 --- a/email/email_test.go +++ b/email/email_test.go @@ -1 +1,87 @@ package email + +import ( + "bufio" + "net" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +// TestDial_PlainAccepts verifies non-465 ports dial plaintext and +// complete the SMTP handshake against a minimal fake server. +func TestDial_PlainAccepts(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + assert.NoError(t, err) + defer ln.Close() + + done := runFakeSMTP(ln) + + e := New(ln.Addr().String()) + e.DialTimeout = 2 * time.Second + + c, err := e.dial() + assert.NoError(t, err) + if c != nil { + _ = c.Quit() + } + <-done +} + +// TestDial_Timeout verifies DialTimeout caps dial wait on unreachable +// hosts. Uses 203.0.113.1 (TEST-NET-3, RFC 5737) which is unroutable. +func TestDial_Timeout(t *testing.T) { + e := New("203.0.113.1:465") + e.DialTimeout = 150 * time.Millisecond + + start := time.Now() + _, err := e.dial() + elapsed := time.Since(start) + + assert.Error(t, err) + assert.Less(t, elapsed, 2*time.Second) +} + +// TestDial_InvalidAddress verifies malformed addresses fail fast. +func TestDial_InvalidAddress(t *testing.T) { + e := New("not-a-valid-address") + _, err := e.dial() + assert.Error(t, err) +} + +// runFakeSMTP accepts one connection and drives a minimal SMTP dialog. +// Returns a channel that closes when the connection is done. +func runFakeSMTP(ln net.Listener) <-chan struct{} { + done := make(chan struct{}) + go func() { + defer close(done) + conn, err := ln.Accept() + if err != nil { + return + } + defer conn.Close() + conn.SetDeadline(time.Now().Add(5 * time.Second)) + + _, _ = conn.Write([]byte("220 fake.local ESMTP ready\r\n")) + r := bufio.NewReader(conn) + for { + line, err := r.ReadString('\n') + if err != nil { + return + } + upper := strings.ToUpper(strings.TrimSpace(line)) + switch { + case strings.HasPrefix(upper, "EHLO"), strings.HasPrefix(upper, "HELO"): + _, _ = conn.Write([]byte("250-fake.local\r\n250 OK\r\n")) + case strings.HasPrefix(upper, "QUIT"): + _, _ = conn.Write([]byte("221 bye\r\n")) + return + default: + _, _ = conn.Write([]byte("502 not implemented\r\n")) + } + } + }() + return done +} diff --git a/go.mod b/go.mod index c32f998..2c638b1 100644 --- a/go.mod +++ b/go.mod @@ -1,11 +1,11 @@ module github.com/labstack/gommon -go 1.18 +go 1.23.0 require ( - github.com/mattn/go-colorable v0.1.13 - github.com/mattn/go-isatty v0.0.20 - github.com/stretchr/testify v1.8.4 + github.com/mattn/go-colorable v0.1.14 + github.com/mattn/go-isatty v0.0.21 + github.com/stretchr/testify v1.11.1 github.com/valyala/fasttemplate v1.2.2 ) @@ -13,6 +13,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect - golang.org/x/sys v0.15.0 // indirect + golang.org/x/sys v0.29.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index f25a876..8aabe4f 100644 --- a/go.sum +++ b/go.sum @@ -1,22 +1,19 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= -github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= +github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.21 h1:xYae+lCNBP7QuW4PUnNG61ffM4hVIfm+zUzDuSzYLGs= +github.com/mattn/go-isatty v0.0.21/go.mod h1:ZXfXG4SQHsB/w3ZeOYbR0PrPwLy+n6xiMrJlRFqopa4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= -github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= -golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU= +golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/log/log.go b/log/log.go index 25f719a..d4c970e 100644 --- a/log/log.go +++ b/log/log.go @@ -138,93 +138,94 @@ func (l *Logger) SetHeader(h string) { } func (l *Logger) Print(i ...interface{}) { - l.log(0, "", i...) - // fmt.Fprintln(l.output, i...) + l.log(0, fmt.Sprint(i...), false) } func (l *Logger) Printf(format string, args ...interface{}) { - l.log(0, format, args...) + l.log(0, fmt.Sprintf(format, args...), false) } func (l *Logger) Printj(j JSON) { - l.log(0, "json", j) + l.logJSON(0, j) } func (l *Logger) Debug(i ...interface{}) { - l.log(DEBUG, "", i...) + l.log(DEBUG, fmt.Sprint(i...), false) } func (l *Logger) Debugf(format string, args ...interface{}) { - l.log(DEBUG, format, args...) + l.log(DEBUG, fmt.Sprintf(format, args...), false) } func (l *Logger) Debugj(j JSON) { - l.log(DEBUG, "json", j) + l.logJSON(DEBUG, j) } func (l *Logger) Info(i ...interface{}) { - l.log(INFO, "", i...) + l.log(INFO, fmt.Sprint(i...), false) } func (l *Logger) Infof(format string, args ...interface{}) { - l.log(INFO, format, args...) + l.log(INFO, fmt.Sprintf(format, args...), false) } func (l *Logger) Infoj(j JSON) { - l.log(INFO, "json", j) + l.logJSON(INFO, j) } func (l *Logger) Warn(i ...interface{}) { - l.log(WARN, "", i...) + l.log(WARN, fmt.Sprint(i...), false) } func (l *Logger) Warnf(format string, args ...interface{}) { - l.log(WARN, format, args...) + l.log(WARN, fmt.Sprintf(format, args...), false) } func (l *Logger) Warnj(j JSON) { - l.log(WARN, "json", j) + l.logJSON(WARN, j) } func (l *Logger) Error(i ...interface{}) { - l.log(ERROR, "", i...) + l.log(ERROR, fmt.Sprint(i...), false) } func (l *Logger) Errorf(format string, args ...interface{}) { - l.log(ERROR, format, args...) + l.log(ERROR, fmt.Sprintf(format, args...), false) } func (l *Logger) Errorj(j JSON) { - l.log(ERROR, "json", j) + l.logJSON(ERROR, j) } func (l *Logger) Fatal(i ...interface{}) { - l.log(fatalLevel, "", i...) + l.log(fatalLevel, fmt.Sprint(i...), false) os.Exit(1) } func (l *Logger) Fatalf(format string, args ...interface{}) { - l.log(fatalLevel, format, args...) + l.log(fatalLevel, fmt.Sprintf(format, args...), false) os.Exit(1) } func (l *Logger) Fatalj(j JSON) { - l.log(fatalLevel, "json", j) + l.logJSON(fatalLevel, j) os.Exit(1) } func (l *Logger) Panic(i ...interface{}) { - l.log(panicLevel, "", i...) - panic(fmt.Sprint(i...)) + msg := fmt.Sprint(i...) + l.log(panicLevel, msg, false) + panic(msg) } func (l *Logger) Panicf(format string, args ...interface{}) { - l.log(panicLevel, format, args...) - panic(fmt.Sprintf(format, args...)) + msg := fmt.Sprintf(format, args...) + l.log(panicLevel, msg, false) + panic(msg) } func (l *Logger) Panicj(j JSON) { - l.log(panicLevel, "json", j) + l.logJSON(panicLevel, j) panic(j) } @@ -348,71 +349,74 @@ func Panicj(j JSON) { global.Panicj(j) } -func (l *Logger) log(level Lvl, format string, args ...interface{}) { - if level >= l.Level() || level == 0 { - buf := l.bufferPool.Get().(*bytes.Buffer) - buf.Reset() - defer l.bufferPool.Put(buf) - _, file, line, _ := runtime.Caller(l.skip) - message := "" - - if format == "" { - message = fmt.Sprint(args...) - } else if format == "json" { - b, err := json.Marshal(args[0]) - if err != nil { - panic(err) - } - message = string(b) - } else { - message = fmt.Sprintf(format, args...) +func (l *Logger) logJSON(level Lvl, j JSON) { + b, err := json.Marshal(j) + if err != nil { + panic(err) + } + l.log(level, string(b), true) +} + +func (l *Logger) log(level Lvl, message string, jsonBody bool) { + if level < l.Level() && level != 0 { + return + } + buf := l.bufferPool.Get().(*bytes.Buffer) + buf.Reset() + defer l.bufferPool.Put(buf) + // JSON callers route through an extra logJSON wrapper; account for + // that frame so runtime.Caller still lands on the user's code. + skip := l.skip + if jsonBody { + skip++ + } + _, file, line, _ := runtime.Caller(skip) + + _, err := l.template.ExecuteFunc(buf, func(w io.Writer, tag string) (int, error) { + switch tag { + case "time_rfc3339": + return w.Write([]byte(time.Now().Format(time.RFC3339))) + case "time_rfc3339_nano": + return w.Write([]byte(time.Now().Format(time.RFC3339Nano))) + case "level": + return w.Write([]byte(l.levels[level])) + case "prefix": + return w.Write([]byte(l.prefix)) + case "long_file": + return w.Write([]byte(file)) + case "short_file": + return w.Write([]byte(path.Base(file))) + case "line": + return w.Write([]byte(strconv.Itoa(line))) } + return 0, nil + }) + if err != nil { + return + } - _, err := l.template.ExecuteFunc(buf, func(w io.Writer, tag string) (int, error) { - switch tag { - case "time_rfc3339": - return w.Write([]byte(time.Now().Format(time.RFC3339))) - case "time_rfc3339_nano": - return w.Write([]byte(time.Now().Format(time.RFC3339Nano))) - case "level": - return w.Write([]byte(l.levels[level])) - case "prefix": - return w.Write([]byte(l.prefix)) - case "long_file": - return w.Write([]byte(file)) - case "short_file": - return w.Write([]byte(path.Base(file))) - case "line": - return w.Write([]byte(strconv.Itoa(line))) - } - return 0, nil - }) - - if err == nil { - s := buf.String() - i := buf.Len() - 1 - if i >= 0 && s[i] == '}' { - // JSON header - buf.Truncate(i) - buf.WriteByte(',') - if format == "json" { - buf.WriteString(message[1:]) - } else { - buf.WriteString(`"message":`) - buf.WriteString(strconv.Quote(message)) - buf.WriteString(`}`) - } - } else { - // Text header - if len(s) > 0 { - buf.WriteByte(' ') - } - buf.WriteString(message) - } - buf.WriteByte('\n') - l.mutex.Lock() - defer l.mutex.Unlock() - l.output.Write(buf.Bytes()) + s := buf.String() + i := buf.Len() - 1 + if i >= 0 && s[i] == '}' { + // JSON header + buf.Truncate(i) + buf.WriteByte(',') + if jsonBody { + buf.WriteString(message[1:]) + } else { + buf.WriteString(`"message":`) + buf.WriteString(strconv.Quote(message)) + buf.WriteString(`}`) + } + } else { + // Text header + if len(s) > 0 { + buf.WriteByte(' ') } + buf.WriteString(message) } + buf.WriteByte('\n') + l.mutex.Lock() + defer l.mutex.Unlock() + l.output.Write(buf.Bytes()) } diff --git a/log/log_test.go b/log/log_test.go index 78d3204..af1b02c 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -129,6 +129,25 @@ func TestEmptyHeader(t *testing.T) { assert.Contains(t, b.String(), `captain's log`) } +// TestCallerFile verifies that both text and JSON log paths report the +// user's call site, not an internal wrapper frame. This guards against +// skip-count drift when the core log method is refactored. +func TestCallerFile(t *testing.T) { + l := New("test") + l.SetHeader("${short_file}") + b := new(bytes.Buffer) + l.SetOutput(b) + l.SetLevel(DEBUG) + + b.Reset() + l.Info("text call") + assert.Contains(t, b.String(), "log_test.go", "text path should report caller file") + + b.Reset() + l.Infoj(JSON{"hello": "world"}) + assert.Contains(t, b.String(), "log_test.go", "JSON path should report caller file") +} + func BenchmarkLog(b *testing.B) { l := New("test") l.SetOutput(new(bytes.Buffer)) diff --git a/random/random.go b/random/random.go index 16deda2..282241b 100644 --- a/random/random.go +++ b/random/random.go @@ -31,10 +31,15 @@ var ( func New() *Random { // https://tip.golang.org/doc/go1.19#:~:text=Read%20no%20longer%20buffers%20random%20data%20obtained%20from%20the%20operating%20system%20between%20calls - p := sync.Pool{New: func() interface{} { - return bufio.NewReader(rand.Reader) - }} - return &Random{readerPool: p} + // sync.Pool must not be copied after first use; construct it directly + // on the struct to avoid copying from a local var. + return &Random{ + readerPool: sync.Pool{ + New: func() interface{} { + return bufio.NewReader(rand.Reader) + }, + }, + } } func (r *Random) String(length uint8, charsets ...string) string { From 309ee6e1aa26c972fbf56dec3b4f950476910514 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sun, 19 Apr 2026 15:00:48 -0700 Subject: [PATCH 2/4] docs: refresh README with email/random packages and Go 1.23 floor Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 1669eb5..1cb4440 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,17 @@ # Gommon [![GoDoc](http://img.shields.io/badge/go-documentation-blue.svg?style=flat-square)](http://godoc.org/github.com/labstack/gommon) [![Coverage Status](http://img.shields.io/coveralls/labstack/gommon.svg?style=flat-square)](https://coveralls.io/r/labstack/gommon) -Common packages for Go -- [Bytes](https://github.com/labstack/gommon/tree/master/bytes) - Format/parse bytes. -- [Color](https://github.com/labstack/gommon/tree/master/color) - Style terminal text. -- [Log](https://github.com/labstack/gommon/tree/master/log) - Simple logging. +Common packages for Go. + +Requires Go 1.23 or later. + +- [Bytes](https://github.com/labstack/gommon/tree/master/bytes) — format/parse byte sizes (binary IEC and decimal SI). +- [Color](https://github.com/labstack/gommon/tree/master/color) — style terminal text. +- [Email](https://github.com/labstack/gommon/tree/master/email) — send email over SMTP; supports STARTTLS and implicit TLS (SMTPS, port 465). +- [Log](https://github.com/labstack/gommon/tree/master/log) — simple leveled logger with text and JSON output. +- [Random](https://github.com/labstack/gommon/tree/master/random) — cryptographically secure random strings over configurable charsets. + +## Install + +```sh +go get github.com/labstack/gommon +``` From d468f46d966ed923436da031bbbae65dfa6c2e70 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sun, 19 Apr 2026 15:03:09 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix(bytes):=20use=207EiB=20instead=20of=208?= =?UTF-8?q?EiB=20in=20tests=20=E2=80=94=202^63=20overflow=20is=20non-porta?= =?UTF-8?q?ble?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parse("8EiB") computes 2^63, which exceeds MaxInt64. The float-to-int conversion of out-of-range values is implementation-dependent per the Go spec, so the prior assertion `math.MaxInt64 == b-1` (relying on two's-complement wrap) only held on some toolchains. 7EiB fits within int64 and tests EiB parsing without depending on undefined behaviour. Co-Authored-By: Claude Opus 4.7 (1M context) --- bytes/bytes_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/bytes/bytes_test.go b/bytes/bytes_test.go index 5549982..8edcf06 100644 --- a/bytes/bytes_test.go +++ b/bytes/bytes_test.go @@ -246,24 +246,26 @@ func TestBytesParse(t *testing.T) { assert.Equal(t, int64(10133099161583616), b) } - // EiB - b, err = Parse("8EiB") + // EiB — 7EiB stays within int64; 8EiB == 2^63 overflows and the + // float-to-int conversion of out-of-range values is + // implementation-dependent per the Go spec. + b, err = Parse("7EiB") if assert.NoError(t, err) { - assert.True(t, math.MaxInt64 == b-1) + assert.Equal(t, int64(8070450532247928832), b) } - b, err = Parse("8Ei") + b, err = Parse("7Ei") if assert.NoError(t, err) { - assert.True(t, math.MaxInt64 == b-1) + assert.Equal(t, int64(8070450532247928832), b) } // EiB with spaces - b, err = Parse("8 EiB") + b, err = Parse("7 EiB") if assert.NoError(t, err) { - assert.True(t, math.MaxInt64 == b-1) + assert.Equal(t, int64(8070450532247928832), b) } - b, err = Parse("8 Ei") + b, err = Parse("7 Ei") if assert.NoError(t, err) { - assert.True(t, math.MaxInt64 == b-1) + assert.Equal(t, int64(8070450532247928832), b) } // KB From 9e285fef2219006e16da0f33832b12e8816603ef Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sun, 19 Apr 2026 15:08:55 -0700 Subject: [PATCH 4/4] fix(email): propagate EHLO errors; always clone TLSConfig; honest comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drive c.Hello explicitly before checking the STARTTLS extension. (*smtp.Client).Extension triggers a lazy hello() and swallows its error, which would silently treat a failed EHLO as "STARTTLS not advertised" and keep the connection in the clear even when the caller configured TLS. - Always Clone the caller's TLSConfig in dial() instead of partial- cloning only when ServerName is empty. Removes the "sometimes mutated" branch and the implicit "do not mutate TLSConfig concurrently with Send" contract ambiguity. - Reword the dial comment so STARTTLS-opportunistic-on-non-465 reads as the honest contract it is ("stays cleartext if server doesn't advertise"), not a guarantee of TLS. - Expand doc comments on TLSConfig and DialTimeout to state the clone behavior and timeout scope. - log: note that skip++ for jsonBody must stay in sync with logJSON. - Tests: bump TestDial_Timeout from 150ms -> 500ms to absorb CI scheduler jitter (upper bound bumped to 3s — still well-scoped). Document intent in TestDial_PlainAccepts by asserting STARTTLS is not advertised on the fake server. Co-Authored-By: Claude Opus 4.7 (1M context) --- email/email.go | 36 ++++++++++++++++++++++++++---------- email/email_test.go | 10 ++++++++-- log/log.go | 4 +++- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/email/email.go b/email/email.go index f48ea6e..b340d9b 100644 --- a/email/email.go +++ b/email/email.go @@ -19,10 +19,13 @@ type ( Template *template.Template // TLSConfig, when non-nil, is used for both implicit TLS (SMTPS) // and STARTTLS. Callers that need a custom root pool or a - // specific server name should set this. + // specific ServerName should set this. The config is cloned per + // dial, so callers can reuse a single value across sends; they + // must not mutate it concurrently with an in-flight Send. TLSConfig *tls.Config - // DialTimeout caps how long the initial connection waits. - // Zero means no caller-imposed timeout. + // DialTimeout caps the TCP (and, for SMTPS, TLS) connect phase. + // It does not bound the full SMTP conversation after the client + // is returned. Zero means no caller-imposed timeout. DialTimeout time.Duration smtpAddress string } @@ -131,8 +134,11 @@ func (e *Email) Send(m *Message) (err error) { m.buffer.WriteString(m.boundary) m.buffer.WriteString("--") - // Dial. Port 465 is SMTPS (implicit TLS) per IANA; everything else - // connects plaintext and upgrades via STARTTLS when advertised. + // Dial. Port 465 is SMTPS (implicit TLS) per IANA and always uses + // TLS. Other ports connect plaintext and opportunistically upgrade + // to STARTTLS only if the server advertises it — if the server + // doesn't, the connection stays in the clear. Operators that + // require TLS must use port 465. c, err := e.dial() if err != nil { return @@ -178,12 +184,15 @@ func (e *Email) dial() (*smtp.Client, error) { return nil, err } - tlsConfig := e.TLSConfig - if tlsConfig == nil { + // Always clone so we never mutate the caller's TLSConfig. + var tlsConfig *tls.Config + if e.TLSConfig == nil { tlsConfig = &tls.Config{ServerName: host} - } else if tlsConfig.ServerName == "" { - tlsConfig = tlsConfig.Clone() - tlsConfig.ServerName = host + } else { + tlsConfig = e.TLSConfig.Clone() + if tlsConfig.ServerName == "" { + tlsConfig.ServerName = host + } } dialer := &net.Dialer{Timeout: e.DialTimeout} @@ -210,6 +219,13 @@ func (e *Email) dial() (*smtp.Client, error) { conn.Close() return nil, err } + // Drive EHLO explicitly so we can surface its error. (*Client).Extension + // triggers a lazy hello() and swallows its error, which would silently + // treat a failed EHLO as "STARTTLS not advertised" and stay cleartext. + if err := c.Hello("localhost"); err != nil { + c.Close() + return nil, err + } if ok, _ := c.Extension("STARTTLS"); ok { if err := c.StartTLS(tlsConfig); err != nil { c.Close() diff --git a/email/email_test.go b/email/email_test.go index 9eb653b..92af861 100644 --- a/email/email_test.go +++ b/email/email_test.go @@ -25,6 +25,10 @@ func TestDial_PlainAccepts(t *testing.T) { c, err := e.dial() assert.NoError(t, err) if c != nil { + // Our fake server never advertises STARTTLS, so dial must have + // stayed plaintext — document that intent. + ok, _ := c.Extension("STARTTLS") + assert.False(t, ok, "fake server should not advertise STARTTLS") _ = c.Quit() } <-done @@ -32,16 +36,18 @@ func TestDial_PlainAccepts(t *testing.T) { // TestDial_Timeout verifies DialTimeout caps dial wait on unreachable // hosts. Uses 203.0.113.1 (TEST-NET-3, RFC 5737) which is unroutable. +// Timeout is 500ms to absorb CI scheduler jitter; the upper bound is +// still well under a round-trip expectation. func TestDial_Timeout(t *testing.T) { e := New("203.0.113.1:465") - e.DialTimeout = 150 * time.Millisecond + e.DialTimeout = 500 * time.Millisecond start := time.Now() _, err := e.dial() elapsed := time.Since(start) assert.Error(t, err) - assert.Less(t, elapsed, 2*time.Second) + assert.Less(t, elapsed, 3*time.Second) } // TestDial_InvalidAddress verifies malformed addresses fail fast. diff --git a/log/log.go b/log/log.go index d4c970e..a223351 100644 --- a/log/log.go +++ b/log/log.go @@ -365,7 +365,9 @@ func (l *Logger) log(level Lvl, message string, jsonBody bool) { buf.Reset() defer l.bufferPool.Put(buf) // JSON callers route through an extra logJSON wrapper; account for - // that frame so runtime.Caller still lands on the user's code. + // that frame so runtime.Caller still lands on the user's code. Keep + // this in sync with logJSON — if the wrapper is ever inlined away + // or moved, drop the increment. skip := l.skip if jsonBody { skip++