From 0a48f1e0bf4725ab4e2f8cecc27c7e5246c69e85 Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 2 Apr 2025 09:37:04 +0200 Subject: [PATCH 1/5] removed unreachable code --- backends/p_spf_test.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/backends/p_spf_test.go b/backends/p_spf_test.go index 72021bb..eb20ca1 100644 --- a/backends/p_spf_test.go +++ b/backends/p_spf_test.go @@ -54,15 +54,4 @@ func TestSpf(t *testing.T) { } return - - if err := l.Close(); err != nil { - t.Error(err) - return - } - - if err := os.Remove("./test_spf.log"); err != nil { - t.Error(err) - return - } - } From 68af318fadd8bf9905c66d937f14c06a93c989f2 Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 2 Apr 2025 09:48:37 +0200 Subject: [PATCH 2/5] minor refactor of unused functions --- backends/backend.go | 3 +-- backends/gateway_test.go | 3 +-- backends/p_compressor.go | 4 +++- backends/p_guerrilla_db_redis.go | 9 +++++++-- backends/p_redis_test.go | 4 ++-- backends/p_spf_test.go | 2 -- backends/redis_generic.go | 2 ++ client.go | 4 ++++ 8 files changed, 20 insertions(+), 11 deletions(-) diff --git a/backends/backend.go b/backends/backend.go index 20e4f89..9206f1d 100644 --- a/backends/backend.go +++ b/backends/backend.go @@ -233,8 +233,7 @@ func (s *service) shutdown() Errors { // processor. func (s *service) AddProcessor(name string, p ProcessorConstructor) { // wrap in a constructor since we want to defer calling it - var c ProcessorConstructor - c = func() Decorator { + c := func() Decorator { return p() } // add to our processors list diff --git a/backends/gateway_test.go b/backends/gateway_test.go index fc11113..af1ad61 100644 --- a/backends/gateway_test.go +++ b/backends/gateway_test.go @@ -1,7 +1,6 @@ package backends import ( - "fmt" "strings" "testing" "time" @@ -12,7 +11,7 @@ import ( func TestStates(t *testing.T) { gw := BackendGateway{} - str := fmt.Sprintf("%s", gw.State) + str := gw.State.String() if strings.Index(str, "NewState") != 0 { t.Error("Backend should begin in NewState") } diff --git a/backends/p_compressor.go b/backends/p_compressor.go index fb99dbc..fcd9379 100644 --- a/backends/p_compressor.go +++ b/backends/p_compressor.go @@ -87,11 +87,13 @@ func (c *DataCompressor) String() string { } // clear it, without clearing the pool +/* func (c *DataCompressor) clear() { c.ExtraHeaders = []byte{} c.Data = nil } - +*/ +// Compress the email data and delivery header together func Compressor() Decorator { return func(p Processor) Processor { return ProcessWith(func(e *mail.Envelope, task SelectTask) (Result, error) { diff --git a/backends/p_guerrilla_db_redis.go b/backends/p_guerrilla_db_redis.go index 7bf2b5e..df5e26e 100644 --- a/backends/p_guerrilla_db_redis.go +++ b/backends/p_guerrilla_db_redis.go @@ -70,6 +70,8 @@ type guerrillaDBAndRedisConfig struct { // Load the backend config for the backend. It has already been unmarshalled // from the main config file 'backend' config "backend_config" // Now we need to convert each type and copy into the guerrillaDBAndRedisConfig struct +// currently unused +/* func (g *GuerrillaDBAndRedisBackend) loadConfig(backendConfig BackendConfig) (err error) { configType := BaseConfig(&guerrillaDBAndRedisConfig{}) bcfg, err := Svc.ExtractConfig(backendConfig, configType) @@ -80,15 +82,18 @@ func (g *GuerrillaDBAndRedisBackend) loadConfig(backendConfig BackendConfig) (er g.config = m return nil } +*/ +// get the number of workers to use for the batcher +// currently unused +/* func (g *GuerrillaDBAndRedisBackend) getNumberOfWorkers() int { return g.config.NumberOfWorkers } - +*/ type redisClient struct { isConnected bool conn RedisConn - time int } // compressedData struct will be compressed using zlib when printed via fmt diff --git a/backends/p_redis_test.go b/backends/p_redis_test.go index 509e4ee..f14ce1f 100644 --- a/backends/p_redis_test.go +++ b/backends/p_redis_test.go @@ -38,7 +38,7 @@ func TestRedisGeneric(t *testing.T) { }() if gateway, ok := g.(*BackendGateway); ok { r := gateway.Process(e, TaskSaveMail) - if strings.Index(r.String(), "250 2.0.0 OK") == -1 { + if !strings.Contains(r.String(), "250 2.0.0 OK") { t.Error("redis processor didn't result with expected result, it said", r) } } @@ -51,7 +51,7 @@ func TestRedisGeneric(t *testing.T) { t.Error(err) return } else { - if strings.Index(string(b), "SETEX") == -1 { + if !strings.Contains(string(b), "SETEX") { t.Error("Log did not contain SETEX, the log was: ", string(b)) } } diff --git a/backends/p_spf_test.go b/backends/p_spf_test.go index eb20ca1..857f2a1 100644 --- a/backends/p_spf_test.go +++ b/backends/p_spf_test.go @@ -52,6 +52,4 @@ func TestSpf(t *testing.T) { t.Error("Log did not contain 'successfully uploaded', the log was: ", string(b)) return } - - return } diff --git a/backends/redis_generic.go b/backends/redis_generic.go index 6ad95de..e5adfd7 100644 --- a/backends/redis_generic.go +++ b/backends/redis_generic.go @@ -28,6 +28,7 @@ func (m *RedisMockConn) Do(commandName string, args ...interface{}) (reply inter return nil, nil } +//lint:ignore U1000 Ignore unused function as we might need it later on type dialOptions struct { readTimeout time.Duration writeTimeout time.Duration @@ -37,6 +38,7 @@ type dialOptions struct { } type RedisDialOption struct { + //lint:ignore U1000 Ignore unused function as we might need it later on f func(*dialOptions) } diff --git a/client.go b/client.go index 70f8042..3259de0 100644 --- a/client.go +++ b/client.go @@ -241,6 +241,10 @@ func (c *client) parsePath(in []byte, p pathParser) (mail.Address, error) { return address, err } +// rcptTo parses the recipient address +// currently unused +/* func (s *server) rcptTo() (address mail.Address, err error) { return address, err } +*/ From 491d34bf330e573de1fad3672a34341392057b43 Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 2 Apr 2025 09:53:56 +0200 Subject: [PATCH 3/5] Errors renamed to follow naming conventions --- .gitignore | 2 ++ config.go | 2 +- models.go | 6 +++--- server.go | 10 +++++----- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 6906cfe..9013654 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ tests/pidfilex.pid tests/testlog* *.pem backends/*.logs +backends/test_spf.log +tests/go-guerrilla.pid diff --git a/config.go b/config.go index 810efcd..9d491bd 100644 --- a/config.go +++ b/config.go @@ -199,7 +199,7 @@ func (c *AppConfig) Load(jsonBytes []byte) error { } // check if any of the currently two tls options are enabled, // if not, skip - if c.Servers[i].TLS.AlwaysOn == false && c.Servers[i].TLS.StartTLSOn == false { + if !(c.Servers[i].TLS.AlwaysOn || c.Servers[i].TLS.StartTLSOn) { continue } diff --git a/models.go b/models.go index 43b5c12..f0dae75 100644 --- a/models.go +++ b/models.go @@ -7,8 +7,8 @@ import ( ) var ( - LineLimitExceeded = errors.New("maximum line length exceeded") - MessageSizeExceeded = errors.New("maximum message size exceeded") + ErrLineLimitExceeded = errors.New("maximum line length exceeded") + ErrMessageSizeExceeded = errors.New("maximum message size exceeded") ) // we need to adjust the limit, so we embed io.LimitedReader @@ -27,7 +27,7 @@ func (alr *adjustableLimitedReader) Read(p []byte) (n int, err error) { n, err = alr.R.Read(p) if err == io.EOF && alr.R.N <= 0 { // return our custom error since io.Reader returns EOF - err = LineLimitExceeded + err = ErrLineLimitExceeded } return } diff --git a/server.go b/server.go index 34bbf1f..d571826 100644 --- a/server.go +++ b/server.go @@ -529,7 +529,7 @@ func (s *server) handleClient(client *client) { } else if netErr, ok := err.(net.Error); ok && netErr.Timeout() { s.log().WithError(err).Warnf("Timeout: %s", client.RemoteIP) return - } else if err == LineLimitExceeded { + } else if err == ErrLineLimitExceeded { client.sendResponse(r.FailLineTooLong) client.kill() break @@ -689,11 +689,11 @@ func (s *server) handleClient(client *client) { err = fmt.Errorf("maximum DATA size exceeded (%d)", sc.MaxSize) } if err != nil { - if err == LineLimitExceeded { - client.sendResponse(r.FailReadLimitExceededDataCmd, " ", LineLimitExceeded.Error()) + if err == ErrLineLimitExceeded { + client.sendResponse(r.FailReadLimitExceededDataCmd, " ", ErrLineLimitExceeded.Error()) client.kill() - } else if err == MessageSizeExceeded { - client.sendResponse(r.FailMessageSizeExceeded, " ", MessageSizeExceeded.Error()) + } else if err == ErrMessageSizeExceeded { + client.sendResponse(r.FailMessageSizeExceeded, " ", ErrMessageSizeExceeded.Error()) client.kill() } else { client.sendResponse(r.FailReadErrorDataCmd, " ", err.Error()) From e0defe1371113ffa4b8e3fcd926052ccd7978687 Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 2 Apr 2025 09:59:39 +0200 Subject: [PATCH 4/5] more linter fixes --- api_test.go | 14 +++++++------- client.go | 3 ++- server.go | 14 +++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/api_test.go b/api_test.go index 880b731..adf2db6 100644 --- a/api_test.go +++ b/api_test.go @@ -501,7 +501,7 @@ func talkToServer(address string) (err error) { return } in := bufio.NewReader(conn) - str, err := in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } @@ -509,7 +509,7 @@ func talkToServer(address string) (err error) { if err != nil { return err } - str, err = in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } @@ -517,7 +517,7 @@ func talkToServer(address string) (err error) { if err != nil { return err } - str, err = in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } @@ -525,7 +525,7 @@ func talkToServer(address string) (err error) { if err != nil { return err } - str, err = in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } @@ -533,7 +533,7 @@ func talkToServer(address string) (err error) { if err != nil { return err } - str, err = in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } @@ -553,11 +553,11 @@ func talkToServer(address string) (err error) { if err != nil { return err } - str, err = in.ReadString('\n') + _, err = in.ReadString('\n') if err != nil { return err } - _ = str + return nil } diff --git a/client.go b/client.go index 3259de0..a2e037a 100644 --- a/client.go +++ b/client.go @@ -53,7 +53,8 @@ type client struct { bufin *smtpBufferedReader bufout *bufio.Writer smtpReader *textproto.Reader - ar *adjustableLimitedReader + //lint:ignore U1000 unused + ar *adjustableLimitedReader // guards access to conn connGuard sync.Mutex log log.Logger diff --git a/server.go b/server.go index d571826..b3860a5 100644 --- a/server.go +++ b/server.go @@ -50,11 +50,12 @@ type server struct { timeout atomic.Value // stores time.Duration listenInterface string clientPool *Pool - wg sync.WaitGroup // for waiting to shutdown - listener net.Listener - closedListener chan bool - hosts allowedHosts // stores map[string]bool for faster lookup - state int + //lint:ignore U1000 unused + wg sync.WaitGroup // for waiting to shutdown + listener net.Listener + closedListener chan bool + hosts allowedHosts // stores map[string]bool for faster lookup + state int // If log changed after a config reload, newLogStore stores the value here until it's safe to change it logStore atomic.Value mainlogStore atomic.Value @@ -242,8 +243,7 @@ func (s *server) setAllowedHosts(allowedHosts []string) { // Begin accepting SMTP clients. Will block unless there is an error or server.Shutdown() is called func (s *server) Start(startWG *sync.WaitGroup) error { - var clientID uint64 - clientID = 0 + var clientID uint64 = 0 listener, err := net.Listen("tcp", s.listenInterface) s.listener = listener From d9eabf36e79819c74532ec5781c3c2b4c8c0f49b Mon Sep 17 00:00:00 2001 From: Philipp Resch Date: Wed, 2 Apr 2025 10:06:41 +0200 Subject: [PATCH 5/5] removal of deprecated ioutil --- api_test.go | 21 ++++++++++----------- backends/p_redis_test.go | 3 +-- log/hook.go | 2 +- log/log.go | 7 +++---- mail/envelope_test.go | 3 +-- server_test.go | 8 ++++---- tests/guerrilla_test.go | 31 ++++++++++++++++--------------- 7 files changed, 36 insertions(+), 39 deletions(-) diff --git a/api_test.go b/api_test.go index adf2db6..8ce4eee 100644 --- a/api_test.go +++ b/api_test.go @@ -4,7 +4,6 @@ import ( "bufio" "errors" "fmt" - "io/ioutil" "net" "os" "strings" @@ -180,7 +179,7 @@ func TestSMTPLoadFile(t *testing.T) { } ` - err := ioutil.WriteFile("goguerrilla.conf.api", []byte(json), 0644) + err := os.WriteFile("goguerrilla.conf.api", []byte(json), 0644) if err != nil { t.Error("could not write guerrilla.conf.api", err) return @@ -207,7 +206,7 @@ func TestSMTPLoadFile(t *testing.T) { t.Error("d.Config.LogFile != tests/go-guerrilla.pid") } - err := ioutil.WriteFile("goguerrilla.conf.api", []byte(json2), 0644) + err := os.WriteFile("goguerrilla.conf.api", []byte(json2), 0644) if err != nil { t.Error("could not write guerrilla.conf.api", err) return @@ -254,7 +253,7 @@ func TestReopenLog(t *testing.T) { d.Shutdown() } - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -309,7 +308,7 @@ func TestReopenServerLog(t *testing.T) { d.Shutdown() } - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -321,7 +320,7 @@ func TestReopenServerLog(t *testing.T) { t.Error("Main log did not re-opened, expecting \"re-opened main log file\"") } - b, err = ioutil.ReadFile(testServerLog) + b, err = os.ReadFile(testServerLog) if err != nil { t.Error("could not read logfile") return @@ -366,7 +365,7 @@ func TestSetConfig(t *testing.T) { d.Shutdown() } - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -471,7 +470,7 @@ func TestSetAddProcessor(t *testing.T) { d.Shutdown() - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -636,7 +635,7 @@ func TestPubSubAPI(t *testing.T) { t.Error(err) } - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -669,7 +668,7 @@ func TestAPILog(t *testing.T) { t.Error("log dest is not tests/testlog, it was ", l.GetLogDest()) } - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return @@ -760,7 +759,7 @@ func TestCustomBackendResult(t *testing.T) { d.Shutdown() - b, err := ioutil.ReadFile("tests/testlog") + b, err := os.ReadFile("tests/testlog") if err != nil { t.Error("could not read logfile") return diff --git a/backends/p_redis_test.go b/backends/p_redis_test.go index f14ce1f..75c4221 100644 --- a/backends/p_redis_test.go +++ b/backends/p_redis_test.go @@ -1,7 +1,6 @@ package backends import ( - "io/ioutil" "os" "strings" "testing" @@ -47,7 +46,7 @@ func TestRedisGeneric(t *testing.T) { t.Error(err) return } - if b, err := ioutil.ReadFile("./test_redis.log"); err != nil { + if b, err := os.ReadFile("./test_redis.log"); err != nil { t.Error(err) return } else { diff --git a/log/hook.go b/log/hook.go index 72caf02..a75e9f1 100644 --- a/log/hook.go +++ b/log/hook.go @@ -42,7 +42,7 @@ type LogrusHook struct { // newLogrusHook creates a new hook. dest can be a file name or one of the following strings: // "stderr" - log to stderr, lines will be written to os.Stdout // "stdout" - log to stdout, lines will be written to os.Stdout -// "off" - no log, lines will be written to ioutil.Discard +// "off" - no log, lines will be written to io.Discard func NewLogrusHook(dest string) (LoggerHook, error) { hookMu.Lock() defer hookMu.Unlock() diff --git a/log/log.go b/log/log.go index a0f46fb..47d3829 100644 --- a/log/log.go +++ b/log/log.go @@ -2,7 +2,6 @@ package log import ( "io" - "io/ioutil" "net" "os" "sync" @@ -131,7 +130,7 @@ func GetLogger(dest string, level string) (Logger, error) { return l, nil } // we'll use the hook to output instead - logrus.Out = ioutil.Discard + logrus.Out = io.Discard // setup the hook h, err := NewLogrusHook(dest) if err != nil { @@ -159,11 +158,11 @@ func newLogrus(o OutputOption, level string) (*log.Logger, error) { } else if o == OutputStdout { out = os.Stdout } else if o == OutputOff { - out = ioutil.Discard + out = io.Discard } } else { // we'll use a hook to output instead - out = ioutil.Discard + out = io.Discard } logger := &log.Logger{ diff --git a/mail/envelope_test.go b/mail/envelope_test.go index e53133f..dde4301 100644 --- a/mail/envelope_test.go +++ b/mail/envelope_test.go @@ -3,7 +3,6 @@ package mail import ( "fmt" "io" - "io/ioutil" "strings" "testing" ) @@ -116,7 +115,7 @@ func TestEnvelope(t *testing.T) { r := e.NewReader() - data, _ := ioutil.ReadAll(r) + data, _ := io.ReadAll(r) if len(data) != e.Len() { t.Error("e.Len() is incorrect, it shown ", e.Len(), " but we wanted ", len(data)) } diff --git a/server_test.go b/server_test.go index 1218628..0ae48ec 100644 --- a/server_test.go +++ b/server_test.go @@ -11,7 +11,7 @@ import ( "crypto/tls" "fmt" - "io/ioutil" + "net" "github.com/phires/go-guerrilla/backends" @@ -196,15 +196,15 @@ func cleanTestArtifacts(t *testing.T) { func TestTLSConfig(t *testing.T) { defer cleanTestArtifacts(t) - if err := ioutil.WriteFile("rootca.test.pem", []byte(rootCAPK), 0644); err != nil { + if err := os.WriteFile("rootca.test.pem", []byte(rootCAPK), 0644); err != nil { t.Fatal("couldn't create rootca.test.pem file.", err) return } - if err := ioutil.WriteFile("client.test.key", []byte(clientPrvKey), 0644); err != nil { + if err := os.WriteFile("client.test.key", []byte(clientPrvKey), 0644); err != nil { t.Fatal("couldn't create client.test.key file.", err) return } - if err := ioutil.WriteFile("client.test.pem", []byte(clientPubKey), 0644); err != nil { + if err := os.WriteFile("client.test.pem", []byte(clientPubKey), 0644); err != nil { t.Fatal("couldn't create client.test.pem file.", err) return } diff --git a/tests/guerrilla_test.go b/tests/guerrilla_test.go index e647bd4..3153434 100644 --- a/tests/guerrilla_test.go +++ b/tests/guerrilla_test.go @@ -31,7 +31,6 @@ import ( "crypto/tls" "errors" "fmt" - "io/ioutil" "net" "strings" @@ -195,7 +194,7 @@ func TestStart(t *testing.T) { } time.Sleep(time.Second) app.Shutdown() - if read, err := ioutil.ReadFile("./testlog"); err == nil { + if read, err := os.ReadFile("./testlog"); err == nil { logOutput := string(read) if i := strings.Index(logOutput, "Listening on TCP 127.0.0.1:4654"); i < 0 { t.Error("Server did not listen on 127.0.0.1:4654") @@ -299,7 +298,7 @@ func TestGreeting(t *testing.T) { } } app.Shutdown() - if read, err := ioutil.ReadFile("./testlog"); err == nil { + if read, err := os.ReadFile("./testlog"); err == nil { logOutput := string(read) if i := strings.Index(logOutput, "Handle client [127.0.0.1"); i < 0 { t.Error("Server did not handle any clients") @@ -356,7 +355,7 @@ func TestShutDown(t *testing.T) { } } // assuming server has shutdown by now - if read, err := ioutil.ReadFile("./testlog"); err == nil { + if read, err := os.ReadFile("./testlog"); err == nil { logOutput := string(read) // fmt.Println(logOutput) if i := strings.Index(logOutput, "Handle client [127.0.0.1"); i < 0 { @@ -913,11 +912,11 @@ func TestNestedMailCmd(t *testing.T) { t.Error("Hello command failed", err.Error()) } // repeat > 64 characters in local part - response, err := Command(conn, bufin, "MAIL FROM:") + _, err := Command(conn, bufin, "MAIL FROM:") if err != nil { t.Error("command failed", err.Error()) } - response, err = Command(conn, bufin, "MAIL FROM:") + response, err := Command(conn, bufin, "MAIL FROM:") if err != nil { t.Error("command failed", err.Error()) } @@ -1029,27 +1028,29 @@ func TestDataMaxLength(t *testing.T) { t.Error("Hello command failed", err.Error()) } - response, err := Command(conn, bufin, "MAIL FROM:test@grr.la") + _, err = Command(conn, bufin, "MAIL FROM:test@grr.la") if err != nil { t.Error("command failed", err.Error()) } //fmt.Println(response) - response, err = Command(conn, bufin, "RCPT TO:") + _, err = Command(conn, bufin, "RCPT TO:") if err != nil { t.Error("command failed", err.Error()) } //fmt.Println(response) - response, err = Command(conn, bufin, "DATA") + _, err = Command(conn, bufin, "DATA") if err != nil { t.Error("command failed", err.Error()) } - response, err = Command( + response, err := Command( conn, bufin, fmt.Sprintf("Subject:test\r\n\r\nHello %s\r\n.\r\n", strings.Repeat("n", int(config.Servers[0].MaxSize-20)))) - + if err != nil { + t.Error("command failed", err.Error()) + } //expected := "500 Line too long" expected := "451 4.3.0 Error: maximum DATA size exceeded" if strings.Index(response, expected) != 0 { @@ -1120,17 +1121,17 @@ func TestDataCommand(t *testing.T) { t.Error("Hello command failed", err.Error()) } - response, err := Command(conn, bufin, "MAIL FROM:") + _, err = Command(conn, bufin, "MAIL FROM:") if err != nil { t.Error("command failed", err.Error()) } //fmt.Println(response) - response, err = Command(conn, bufin, "RCPT TO:") + _, err = Command(conn, bufin, "RCPT TO:") if err != nil { t.Error("command failed", err.Error()) } //fmt.Println(response) - response, err = Command(conn, bufin, "DATA") + _, err = Command(conn, bufin, "DATA") if err != nil { t.Error("command failed", err.Error()) } @@ -1141,7 +1142,7 @@ func TestDataCommand(t *testing.T) { testHeader+"\r\nHello World\r\n.\r\n") */ _ = testHeader - response, err = Command( + response, err := Command( conn, bufin, email+"\r\n.\r\n")