Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(smtp): use usestarttls (match the docs) #252

Merged
merged 2 commits into from
May 27, 2022
Merged

fix(smtp): use usestarttls (match the docs) #252

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3afab5
fix(smtp): use `usestarttls` (match the docs)
JosephKav May 26, 2022
66cb389
fix: `starttls` as alias to prevent a breaking change
JosephKav May 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions pkg/services/smtp/smtp_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package smtp
import (
"errors"
"fmt"
"github.com/containrrr/shoutrrr/pkg/util"
"net/url"
"strconv"

"github.com/containrrr/shoutrrr/pkg/util"

JosephKav marked this conversation as resolved.
Show resolved Hide resolved
"github.com/containrrr/shoutrrr/pkg/format"
"github.com/containrrr/shoutrrr/pkg/types"
)
Expand All @@ -23,7 +24,7 @@ type Config struct {
Subject string `desc:"The subject of the sent mail" key:"subject,title" default:"Shoutrrr Notification"`
Auth authType `desc:"SMTP authentication method" key:"auth" default:"Unknown"`
Encryption encMethod `desc:"Encryption method" default:"Auto" key:"encryption"`
UseStartTLS bool `desc:"Whether to use StartTLS encryption" default:"Yes" key:"starttls"`
UseStartTLS bool `desc:"Whether to use StartTLS encryption" default:"Yes" key:"usestarttls"`
JosephKav marked this conversation as resolved.
Show resolved Hide resolved
UseHTML bool `desc:"Whether the message being sent is in HTML" default:"No" key:"usehtml"`
}

Expand Down
27 changes: 15 additions & 12 deletions pkg/services/smtp/smtp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package smtp

import (
"fmt"
"log"
"net/smtp"
"net/url"
Expand Down Expand Up @@ -42,9 +43,10 @@ var _ = Describe("the SMTP service", func() {
})
When("parsing the configuration URL", func() {
It("should be identical after de-/serialization", func() {
testURL := "smtp://user:[email protected]:2225/?auth=None&encryption=ExplicitTLS&fromaddress=sender%40example.com&fromname=Sender&starttls=No&subject=Subject&toaddresses=rec1%40example.com%2Crec2%40example.com&usehtml=Yes"
testURL := "smtp://user:[email protected]:2225/?auth=None&encryption=ExplicitTLS&fromaddress=sender%40example.com&fromname=Sender&subject=Subject&toaddresses=rec1%40example.com%2Crec2%40example.com&usehtml=Yes&usestarttls=No"
Copy link
Contributor Author

@JosephKav JosephKav May 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed why I had to move this to the end. This is Go doing its usual alphabetising of a map right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, actually Go randomizes the map order. But to make the tests repeatable, we sort the keys in alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, must be the yaml encoder that alphabetises my maps then. Unless I have been extremely 'lucky' to have it alphabetised correctly every time I checked after a save

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it does, it has it's own map impl: https://pkg.go.dev/gopkg.in/yaml.v2#MapSlice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish v3 has a MapSlice so that I could choose the order when I save it. Had to make it save the file, then go through and check whether the blocks have been written in the correct order (they won't have been...), then bubble sort those blocks back to how it should be and save again! I did spend a while on ordering the maps, but then found out they were alphabetised and am calling that good enough


url, err := url.Parse(testURL)
fmt.Println(url)
Expect(err).NotTo(HaveOccurred(), "parsing")

config := &Config{}
Expand All @@ -53,6 +55,7 @@ var _ = Describe("the SMTP service", func() {
Expect(err).NotTo(HaveOccurred(), "verifying")

outputURL := config.GetURL()
fmt.Printf("\n\n%s\n%s\n\n-", outputURL, testURL)

Expect(outputURL.String()).To(Equal(testURL))

Expand Down Expand Up @@ -189,7 +192,7 @@ var _ = Describe("the SMTP service", func() {
When("given a typical usage case configuration URL", func() {

It("should send notifications without any errors", func() {
testURL := "smtp://user:[email protected]:2225/?startTLS=no&[email protected]&[email protected],[email protected]&useHTML=yes"
testURL := "smtp://user:[email protected]:2225/?useStartTLS=no&[email protected]&[email protected],[email protected]&useHTML=yes"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand Down Expand Up @@ -217,7 +220,7 @@ var _ = Describe("the SMTP service", func() {
When("given a configuration URL with authentication disabled", func() {

It("should send notifications without any errors", func() {
testURL := "smtp://example.com:2225/?startTLS=no&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand All @@ -240,7 +243,7 @@ var _ = Describe("the SMTP service", func() {
When("given a configuration URL with StartTLS but it is not supported", func() {

It("should send notifications without any errors", func() {
testURL := "smtp://example.com:2225/?startTLS=yes&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=yes&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand All @@ -263,7 +266,7 @@ var _ = Describe("the SMTP service", func() {
When("server communication fails", func() {

It("should fail when not being able to enable StartTLS", func() {
testURL := "smtp://example.com:2225/?startTLS=yes&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=yes&auth=none&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand All @@ -281,7 +284,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when authentication type is invalid", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=bad&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=bad&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{}, "", "")
if msg, test := standard.IsTestSetupFailure(err); test {
Skip(msg)
Expand All @@ -292,7 +295,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when not being able to use authentication type", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=crammd5&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=crammd5&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand All @@ -309,7 +312,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when not being able to send to recipient", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand All @@ -326,7 +329,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when the recipient is not accepted", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
err := testSendRecipient(testURL, []string{
"250 mx.google.com at your service",
"250 Sender OK",
Expand All @@ -341,7 +344,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when the server does not accept the data stream", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
err := testSendRecipient(testURL, []string{
"250 mx.google.com at your service",
"250 Sender OK",
Expand All @@ -357,7 +360,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when the server does not accept the data stream content", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
err := testSendRecipient(testURL, []string{
"250 mx.google.com at your service",
"250 Sender OK",
Expand All @@ -374,7 +377,7 @@ var _ = Describe("the SMTP service", func() {
})

It("should fail when the server does not close the connection gracefully", func() {
testURL := "smtp://example.com:2225/?startTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
testURL := "smtp://example.com:2225/?useStartTLS=no&auth=none&[email protected]&[email protected]&useHTML=no"
err := testIntegration(testURL, []string{
"250-mx.google.com at your service",
"250-SIZE 35651584",
Expand Down