From af9915e4e7c32669dd7300b876a7c416fe39e979 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 19:45:49 +0200 Subject: [PATCH 1/9] Add test for WriteToTempFile failure scenario This new test verifies error handling for WriteToTempFile when the TMPDIR environment variable is set to an invalid directory. It ensures that the method fails as expected under these conditions, improving code robustness. --- msg_nowin_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/msg_nowin_test.go b/msg_nowin_test.go index 820be24..e338c57 100644 --- a/msg_nowin_test.go +++ b/msg_nowin_test.go @@ -61,3 +61,18 @@ func TestMsg_WriteToSendmail(t *testing.T) { t.Errorf("WriteToSendmail failed: %s", err) } } + +func TestMsg_WriteToTempFileFailed(t *testing.T) { + m := NewMsg() + _ = m.From("Toni Tester ") + _ = m.To("Ellenor Tester ") + m.SetBodyString(TypeTextPlain, "This is a test") + + if err := os.Setenv("TMPDIR", "/invalid/directory/that/does/not/exist"); err != nil { + t.Errorf("failed to set TMPDIR environment variable: %s", err) + } + _, err := m.WriteToTempFile() + if err == nil { + t.Errorf("WriteToTempFile() did not fail as expected") + } +} From 19330fc1081f641442312c96ed3197e2f9e129ce Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 20:30:23 +0200 Subject: [PATCH 2/9] Refactor random number generation and add version-specific implementations Removed error handling from `randNum` in `random_test.go` and introduced new `randNum` implementations for different Go versions (1.19, 1.20-1.21, 1.22+). This ensures compatibility with different versions of Go and utilizes the appropriate version-specific random number generation methods. We are using math/rand instead crypto/rand. For our needs this should be sufficient. --- random.go | 22 ---------------------- random_119.go | 22 ++++++++++++++++++++++ random_121.go | 20 ++++++++++++++++++++ random_122.go | 22 ++++++++++++++++++++++ random_test.go | 8 +------- 5 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 random_119.go create mode 100644 random_121.go create mode 100644 random_122.go diff --git a/random.go b/random.go index 831b118..3a3f16b 100644 --- a/random.go +++ b/random.go @@ -7,8 +7,6 @@ package mail import ( "crypto/rand" "encoding/binary" - "fmt" - "math/big" "strings" ) @@ -52,23 +50,3 @@ func randomStringSecure(length int) (string, error) { return randString.String(), nil } - -// 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) - } - length64 := big.NewInt(int64(length)) - if !length64.IsUint64() { - return 0, fmt.Errorf("big.NewInt() generation returned negative value: %d", length64) - } - randNum64, err := rand.Int(rand.Reader, length64) - if err != nil { - return 0, err - } - randomNum := int(randNum64.Int64()) - if randomNum < 0 { - return 0, fmt.Errorf("generated random number does not fit as int64: %d", randNum64) - } - return randomNum, nil -} diff --git a/random_119.go b/random_119.go new file mode 100644 index 0000000..b084305 --- /dev/null +++ b/random_119.go @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors +// +// SPDX-License-Identifier: MIT + +//go:build go1.19 && !go1.20 +// +build go1.19,!go1.20 + +package mail + +import ( + "math/rand" + "time" +) + +// randNum returns a random number with a maximum value of length +func randNum(maxval int) int { + if maxval <= 0 { + return 0 + } + rand.Seed(time.Now().UnixNano()) + return rand.Intn(maxval) +} diff --git a/random_121.go b/random_121.go new file mode 100644 index 0000000..b401bc8 --- /dev/null +++ b/random_121.go @@ -0,0 +1,20 @@ +// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors +// +// SPDX-License-Identifier: MIT + +//go:build go1.20 && !go1.22 +// +build go1.20,!go1.22 + +package mail + +import ( + "math/rand" +) + +// randNum returns a random number with a maximum value of length +func randNum(maxval int) int { + if maxval <= 0 { + return 0 + } + return rand.Intn(maxval) +} diff --git a/random_122.go b/random_122.go new file mode 100644 index 0000000..9a6132b --- /dev/null +++ b/random_122.go @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors +// +// SPDX-License-Identifier: MIT + +//go:build go1.22 +// +build go1.22 + +package mail + +import ( + "math/rand/v2" +) + +// randNum returns a random number with a maximum value of maxval. +// go-mail compiled with Go 1.22+ will make use of the novel math/rand/v2 interface +// Older versions of Go will use math/rand +func randNum(maxval int) int { + if maxval <= 0 { + return 0 + } + return rand.IntN(maxval) +} diff --git a/random_test.go b/random_test.go index 51e3ba6..caa5e58 100644 --- a/random_test.go +++ b/random_test.go @@ -55,13 +55,7 @@ func TestRandomNum(t *testing.T) { for _, tc := range tt { t.Run(tc.testName, func(t *testing.T) { - rn, err := randNum(tc.max) - if err != nil { - t.Errorf("random number generation failed: %s", err) - } - if rn < 0 { - t.Errorf("random number generation failed: %d is smaller than zero", rn) - } + rn := randNum(tc.max) if rn > tc.max { t.Errorf("random number generation failed: %d is bigger than given value %d", rn, tc.max) } From f5d4cdafeace52dfb353b097cd340dd282dbd18c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 20:39:30 +0200 Subject: [PATCH 3/9] Increase test iterations for SetMessageID randomness check Updated the TestMsg_SetMessageIDRandomness test to run 50,000 iterations instead of 100, ensuring a more robust evaluation of the Msg.SetMessageID method's randomness. Additionally, streamlined the retrieval of Message ID using GetMessageID. --- msg_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/msg_test.go b/msg_test.go index 0656ff2..f570b02 100644 --- a/msg_test.go +++ b/msg_test.go @@ -786,13 +786,11 @@ func TestMsg_SetMessageIDWithValue(t *testing.T) { // TestMsg_SetMessageIDRandomness tests the randomness of Msg.SetMessageID methods func TestMsg_SetMessageIDRandomness(t *testing.T) { var mids []string - for i := 0; i < 100; i++ { + for i := 0; i < 50_000; i++ { m := NewMsg() m.SetMessageID() - mid := m.GetGenHeader(HeaderMessageID) - if len(mid) > 0 { - mids = append(mids, mid[0]) - } + mid := m.GetMessageID() + mids = append(mids, mid) } c := make(map[string]int) for i := range mids { From 77920be1a116f9cb0810a581d50f2db699731e15 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 20:39:50 +0200 Subject: [PATCH 4/9] Remove unnecessary error handling Eliminated redundant error handling for random number generation as errors from these functions do not require special attention. This reduces code complexity and improves readability. --- msg.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msg.go b/msg.go index 11d8ab1..b66bffe 100644 --- a/msg.go +++ b/msg.go @@ -467,8 +467,8 @@ func (m *Msg) SetMessageID() { if err != nil { hostname = "localhost.localdomain" } - randNumPrimary, _ := randNum(100000000) - randNumSecondary, _ := randNum(10000) + randNumPrimary := randNum(100000000) + randNumSecondary := randNum(10000) randString, _ := randomStringSecure(17) procID := os.Getpid() * randNumSecondary messageID := fmt.Sprintf("%d.%d%d.%s@%s", procID, randNumPrimary, randNumSecondary, From 52061f97c654834982f714df316bb76c759f7ae8 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 20:58:29 +0200 Subject: [PATCH 5/9] Add tests for nil SendError MessageID and Msg These tests ensure that accessing MessageID and Msg methods on a nil SendError pointer returns the expected values (empty string and nil respectively). This helps in validating the error handling logic and avoiding potential nil pointer dereference issues. --- senderror_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/senderror_test.go b/senderror_test.go index 8b7bfb3..bf823e9 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -108,6 +108,13 @@ func TestSendError_MessageID(t *testing.T) { } } +func TestSendError_MessageIDNil(t *testing.T) { + var se *SendError + if se.MessageID() != "" { + t.Error("expected empty string on nil-senderror") + } +} + func TestSendError_Msg(t *testing.T) { var se *SendError err := returnSendError(ErrAmbiguous, false) @@ -131,6 +138,13 @@ func TestSendError_Msg(t *testing.T) { } } +func TestSendError_MsgNil(t *testing.T) { + var se *SendError + if se.Msg() != nil { + t.Error("expected nil on nil-senderror") + } +} + // returnSendError is a helper method to retunr a SendError with a specific reason func returnSendError(r SendErrReason, t bool) error { message := NewMsg() From f3633e1913b923e68b6fd004a02a95c9744a54ee Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 21:11:19 +0200 Subject: [PATCH 6/9] Add tests for SendError functionalities Introduce tests for `SendError` to verify behavior of `errors.Is` and string representation. `TestSendError_IsFail` checks for error mismatch, and `TestSendError_ErrorMulti` verifies the formatted error message. --- senderror_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/senderror_test.go b/senderror_test.go index bf823e9..9a9b1ec 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -145,6 +145,24 @@ func TestSendError_MsgNil(t *testing.T) { } } +func TestSendError_IsFail(t *testing.T) { + err1 := returnSendError(ErrAmbiguous, false) + err2 := returnSendError(ErrSMTPMailFrom, false) + if errors.Is(err1, err2) { + t.Errorf("error mismatch, ErrAmbiguous should not be equal to ErrSMTPMailFrom") + } +} + +func TestSendError_ErrorMulti(t *testing.T) { + expected := `ambiguous reason, check Msg.SendError for message specific reasons, ` + + `affected recipient(s): , ` + err := &SendError{Reason: ErrAmbiguous, isTemp: false, affectedMsg: nil, + rcpt: []string{"", ""}} + if err.Error() != expected { + t.Errorf("error mismatch, expected: %s, got: %s", expected, err.Error()) + } +} + // returnSendError is a helper method to retunr a SendError with a specific reason func returnSendError(r SendErrReason, t bool) error { message := NewMsg() From 40534570200ed066977283bb3c207752566b1c6a Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 21:15:13 +0200 Subject: [PATCH 7/9] Refactor TestSendError_ErrorMulti formatting Reformatted the instantiation of the SendError struct to improve readability. This helps maintain consistency and clarity in the code base for better collaboration and future maintenance. --- senderror_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/senderror_test.go b/senderror_test.go index 9a9b1ec..e04b7ee 100644 --- a/senderror_test.go +++ b/senderror_test.go @@ -156,8 +156,10 @@ func TestSendError_IsFail(t *testing.T) { func TestSendError_ErrorMulti(t *testing.T) { expected := `ambiguous reason, check Msg.SendError for message specific reasons, ` + `affected recipient(s): , ` - err := &SendError{Reason: ErrAmbiguous, isTemp: false, affectedMsg: nil, - rcpt: []string{"", ""}} + err := &SendError{ + Reason: ErrAmbiguous, isTemp: false, affectedMsg: nil, + rcpt: []string{"", ""}, + } if err.Error() != expected { t.Errorf("error mismatch, expected: %s, got: %s", expected, err.Error()) } From 0b9a215e7d37c7fe52d96303798c6b2fa04ad9a4 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 21:42:10 +0200 Subject: [PATCH 8/9] Add deferred TMPDIR restoration in test setup Store and restore the original TMPDIR value using a deferred function in `msg_nowin_test.go`. This ensures the TMPDIR environment variable is restored after the test runs, preventing potential side effects on other tests or processes. --- msg_nowin_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/msg_nowin_test.go b/msg_nowin_test.go index e338c57..6cde71a 100644 --- a/msg_nowin_test.go +++ b/msg_nowin_test.go @@ -68,6 +68,13 @@ func TestMsg_WriteToTempFileFailed(t *testing.T) { _ = m.To("Ellenor Tester ") m.SetBodyString(TypeTextPlain, "This is a test") + curTmpDir := os.Getenv("TMPDIR") + defer func() { + if err := os.Setenv("TMPDIR", curTmpDir); err != nil { + t.Errorf("failed to set TMPDIR environment variable: %s", err) + } + }() + if err := os.Setenv("TMPDIR", "/invalid/directory/that/does/not/exist"); err != nil { t.Errorf("failed to set TMPDIR environment variable: %s", err) } From 3f0ac027e2d9fa315121ffcc9f79763448312c7c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Fri, 20 Sep 2024 22:06:11 +0200 Subject: [PATCH 9/9] Add test for zero input in randNum This commit introduces a new test case in random_test.go to ensure that the randNum function returns zero when given an input of zero. This helps validate the correctness of edge case handling in the random number generation logic. --- random_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/random_test.go b/random_test.go index caa5e58..e69b4e7 100644 --- a/random_test.go +++ b/random_test.go @@ -62,3 +62,10 @@ func TestRandomNum(t *testing.T) { }) } } + +func TestRandomNumZero(t *testing.T) { + rn := randNum(0) + if rn != 0 { + t.Errorf("random number generation failed: %d is not zero", rn) + } +}