fix(gotify): handle token ending in / (#235)

internal/testutils/mockclientservice.go

@@ -0,0 +1,8 @@
+package testutils
+
+import "net/http"
+
+// MockClientService is used to allow mocking the HTTP client when testing
+type MockClientService interface {
+	GetHTTPClient() *http.Client
+}

pkg/services/gotify/gotify.go

@@ -1,9 +1,7 @@
 package gotify
 
 import (
-	"bytes"
 	"crypto/tls"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -13,14 +11,16 @@ import (
 	"github.com/containrrr/shoutrrr/pkg/format"
 	"github.com/containrrr/shoutrrr/pkg/services/standard"
 	"github.com/containrrr/shoutrrr/pkg/types"
+	"github.com/containrrr/shoutrrr/pkg/util/jsonclient"
 )
 
 // Service providing Gotify as a notification service
 type Service struct {
 	standard.Standard
-	config *Config
-	pkr    format.PropKeyResolver
-	Client *http.Client
+	config     *Config
+	pkr        format.PropKeyResolver
+	httpClient *http.Client
+	client     jsonclient.Client
 }
 
 // Initialize loads ServiceConfig from configURL and sets logger for this Service
@@ -32,7 +32,7 @@ func (service *Service) Initialize(configURL *url.URL, logger types.StdLogger) e
 	service.pkr = format.NewPropKeyResolver(service.config)
 	err := service.config.SetURL(configURL)
 
-	service.Client = &http.Client{
+	service.httpClient = &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{
 				// If DisableTLS is specified, we might still need to disable TLS verification
@@ -44,6 +44,7 @@ func (service *Service) Initialize(configURL *url.URL, logger types.StdLogger) e
 		// Set a reasonable timeout to prevent one bad transfer from block all subsequent ones
 		Timeout: 10 * time.Second,
 	}
+	service.client = jsonclient.NewWithHTTPClient(service.httpClient)
 
 	return err
 }
@@ -69,11 +70,8 @@ func isTokenValid(token string) bool {
 
 func buildURL(config *Config) (string, error) {
 	token := config.Token
-	if len(token) > 0 && token[0] == '/' {
-		token = token[1:]
-	}
 	if !isTokenValid(token) {
-		return "", fmt.Errorf("invalid gotify token \"%s\"", token)
+		return "", fmt.Errorf("invalid gotify token %q", token)
 	}
 	scheme := "https"
 	if config.DisableTLS {
@@ -96,24 +94,26 @@ func (service *Service) Send(message string, params *types.Params) error {
 	if err != nil {
 		return err
 	}
-	jsonBody, err := json.Marshal(JSON{
+
+	request := &messageRequest{
 		Message:  message,
 		Title:    config.Title,
 		Priority: config.Priority,
-	})
-	if err != nil {
-		return err
 	}
-	jsonBuffer := bytes.NewBuffer(jsonBody)
-	resp, err := service.Client.Post(postURL, "application/json", jsonBuffer)
+	response := &messageResponse{}
+	err = service.client.Post(postURL, request, response)
 	if err != nil {
+		errorRes := &errorResponse{}
+		if service.client.ErrorResponse(err, errorRes) {
+			return errorRes
+		}
 		return fmt.Errorf("failed to send notification to Gotify: %s", err)
 	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
-		return fmt.Errorf("Gotify notification returned %d HTTP status code", resp.StatusCode)
-	}
 
 	return nil
 }
+
+// GetHTTPClient is only supposed to be used for mocking the httpclient when testing
+func (service *Service) GetHTTPClient() *http.Client {
+	return service.httpClient
+}

pkg/services/gotify/gotify_config.go

@@ -1,11 +1,12 @@
 package gotify
 
 import (
-	"github.com/containrrr/shoutrrr/pkg/format"
-	"github.com/containrrr/shoutrrr/pkg/types"
 	"net/url"
 	"strings"
 
+	"github.com/containrrr/shoutrrr/pkg/format"
+	"github.com/containrrr/shoutrrr/pkg/types"
+
 	"github.com/containrrr/shoutrrr/pkg/services/standard"
 )
 
@@ -44,11 +45,19 @@ func (config *Config) getURL(resolver types.ConfigQueryResolver) *url.URL {
 
 func (config *Config) setURL(resolver types.ConfigQueryResolver, url *url.URL) error {
 
-	tokenIndex := strings.LastIndex(url.Path, "/")
-	config.Path = url.Path[:tokenIndex]
+	path := url.Path
+	if len(path) > 0 && path[len(path)-1] == '/' {
+		path = path[:len(path)-1]
+	}
+	tokenIndex := strings.LastIndex(path, "/") + 1
+
+	config.Path = path[:tokenIndex]
+	if config.Path == "/" {
+		config.Path = config.Path[1:]
+	}
 
 	config.Host = url.Host
-	config.Token = url.Path[tokenIndex:]
+	config.Token = path[tokenIndex:]
 
 	for key, vals := range url.Query() {
 		if err := resolver.Set(key, vals[0]); err != nil {

pkg/services/gotify/gotify_json.go

@@ -1,8 +1,27 @@
 package gotify
 
-// JSON is the actual payload being sent to the Gotify API
-type JSON struct {
+import "fmt"
+
+// messageRequest is the actual payload being sent to the Gotify API
+type messageRequest struct {
 	Message  string `json:"message"`
 	Title    string `json:"title"`
 	Priority int    `json:"priority"`
 }
+
+type messageResponse struct {
+	messageRequest
+	ID    uint64 `json:"id"`
+	AppID uint64 `json:"appid"`
+	Date  string `json:"date"`
+}
+
+type errorResponse struct {
+	Name        string `json:"error"`
+	Code        uint64 `json:"errorCode"`
+	Description string `json:"errorDescription"`
+}
+
+func (er *errorResponse) Error() string {
+	return fmt.Sprintf("server respondend with %v (%v): %v", er.Name, er.Code, er.Description)
+}

pkg/services/gotify/gotify_test.go

@@ -1,12 +1,13 @@
 package gotify
 
 import (
-	"errors"
-	"github.com/jarcoal/httpmock"
 	"log"
 	"net/url"
 	"testing"
 
+	"github.com/containrrr/shoutrrr/internal/testutils"
+
+	"github.com/jarcoal/httpmock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -76,36 +77,52 @@ var _ = Describe("the Gotify plugin URL building and token validation functions"
 			Expect(isTokenValid(token)).To(BeFalse())
 		})
 	})
+	Describe("creating the API URL", func() {
+		When("the token is invalid", func() {
+			It("should return an error", func() {
+				config := Config{
+					Token: "invalid",
+				}
+				_, err := buildURL(&config)
+				Expect(err).To(HaveOccurred())
+			})
+		})
+	})
 	Describe("creating a config", func() {
 		When("parsing the configuration URL", func() {
 			It("should be identical after de-/serialization (with path)", func() {
 				testURL := "gotify://my.gotify.tld/gotify/Aaa.bbb.ccc.ddd?title=Test+title"
 
-				url, err := url.Parse(testURL)
-				Expect(err).NotTo(HaveOccurred(), "parsing")
-
 				config := &Config{}
-				err = config.SetURL(url)
-				Expect(err).NotTo(HaveOccurred(), "verifying")
-
-				outputURL := config.GetURL()
-				Expect(outputURL.String()).To(Equal(testURL))
-
+				Expect(config.SetURL(testutils.URLMust(testURL))).To(Succeed())
+				Expect(config.GetURL().String()).To(Equal(testURL))
 			})
 			It("should be identical after de-/serialization (without path)", func() {
 				testURL := "gotify://my.gotify.tld/Aaa.bbb.ccc.ddd?disabletls=Yes&priority=1&title=Test+title"
 
-				url, err := url.Parse(testURL)
-				Expect(err).NotTo(HaveOccurred(), "parsing")
-
 				config := &Config{}
-				err = config.SetURL(url)
-				Expect(err).NotTo(HaveOccurred(), "verifying")
+				Expect(config.SetURL(testutils.URLMust(testURL))).To(Succeed())
+				Expect(config.GetURL().String()).To(Equal(testURL))
 
-				outputURL := config.GetURL()
+			})
+			It("should allow slash at the end of the token", func() {
+				url := testutils.URLMust("gotify://my.gotify.tld/Aaa.bbb.ccc.ddd/")
 
-				Expect(outputURL.String()).To(Equal(testURL))
+				config := &Config{}
+				Expect(config.SetURL(url)).To(Succeed())
+				Expect(config.Token).To(Equal("Aaa.bbb.ccc.ddd"))
+			})
+			It("should allow slash at the end of the token, with additional path", func() {
+				url := testutils.URLMust("gotify://my.gotify.tld/path/to/gotify/Aaa.bbb.ccc.ddd/")
 
+				config := &Config{}
+				Expect(config.SetURL(url)).To(Succeed())
+				Expect(config.Token).To(Equal("Aaa.bbb.ccc.ddd"))
+			})
+			It("should not crash on empty token or path slash at the end of the token", func() {
+				config := &Config{}
+				Expect(config.SetURL(testutils.URLMust("gotify://my.gotify.tld//"))).To(Succeed())
+				Expect(config.SetURL(testutils.URLMust("gotify://my.gotify.tld/"))).To(Succeed())
 			})
 		})
 	})
@@ -119,23 +136,27 @@ var _ = Describe("the Gotify plugin URL building and token validation functions"
 		It("should not report an error if the server accepts the payload", func() {
 			serviceURL, _ := url.Parse("gotify://my.gotify.tld/Aaa.bbb.ccc.ddd")
 			err = service.Initialize(serviceURL, logger)
-			httpmock.ActivateNonDefault(service.Client)
+			httpmock.ActivateNonDefault(service.GetHTTPClient())
 			Expect(err).NotTo(HaveOccurred())
 
 			targetURL := "https://my.gotify.tld/message?token=Aaa.bbb.ccc.ddd"
-			httpmock.RegisterResponder("POST", targetURL, httpmock.NewStringResponder(200, ""))
+			httpmock.RegisterResponder("POST", targetURL, testutils.JSONRespondMust(200, messageResponse{}))
 
 			err = service.Send("Message", nil)
 			Expect(err).NotTo(HaveOccurred())
 		})
 		It("should not panic if an error occurs when sending the payload", func() {
 			serviceURL, _ := url.Parse("gotify://my.gotify.tld/Aaa.bbb.ccc.ddd")
 			err = service.Initialize(serviceURL, logger)
-			httpmock.ActivateNonDefault(service.Client)
+			httpmock.ActivateNonDefault(service.GetHTTPClient())
 			Expect(err).NotTo(HaveOccurred())
 
 			targetURL := "https://my.gotify.tld/message?token=Aaa.bbb.ccc.ddd"
-			httpmock.RegisterResponder("POST", targetURL, httpmock.NewErrorResponder(errors.New("dummy error")))
+			httpmock.RegisterResponder("POST", targetURL, testutils.JSONRespondMust(401, errorResponse{
+				Name:        "Unauthorized",
+				Code:        401,
+				Description: "you need to provide a valid access token or user credentials to access this api",
+			}))
 
 			err = service.Send("Message", nil)
 			Expect(err).To(HaveOccurred())

pkg/services/services_test.go

@@ -1,14 +1,15 @@
 package services_test
 
 import (
-	"github.com/containrrr/shoutrrr/pkg/router"
-	"github.com/containrrr/shoutrrr/pkg/services/gotify"
-	"github.com/containrrr/shoutrrr/pkg/types"
-	"github.com/jarcoal/httpmock"
 	"log"
 	"net/http"
 	"testing"
 
+	"github.com/containrrr/shoutrrr/internal/testutils"
+	"github.com/containrrr/shoutrrr/pkg/router"
+	"github.com/containrrr/shoutrrr/pkg/types"
+	"github.com/jarcoal/httpmock"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -41,6 +42,7 @@ var serviceURLs = map[string]string{
 
 var serviceResponses = map[string]string{
 	"pushbullet": `{"created": 0}`,
+	"gotify":     `{"id": 0}`,
 }
 
 var logger = log.New(GinkgoWriter, "Test", log.LstdFlags)
@@ -88,9 +90,8 @@ var _ = Describe("services", func() {
 				service, err := serviceRouter.Locate(configURL)
 				Expect(err).NotTo(HaveOccurred())
 
-				if key == "gotify" {
-					gotifyService := service.(*gotify.Service)
-					httpmock.ActivateNonDefault(gotifyService.Client)
+				if mockService, ok := service.(testutils.MockClientService); ok {
+					httpmock.ActivateNonDefault(mockService.GetHTTPClient())
 				}
 
 				err = service.Send("test", (*types.Params)(&map[string]string{

pkg/util/jsonclient/jsonclient.go

@@ -31,9 +31,15 @@ type client struct {
 	indent     string
 }
 
+// NewClient returns a new JSON Client using the default http.Client
 func NewClient() Client {
+	return NewWithHTTPClient(http.DefaultClient)
+}
+
+// NewWithHTTPClient returns a new JSON Client using the specified http.Client
+func NewWithHTTPClient(httpClient *http.Client) Client {
 	return &client{
-		httpClient: http.DefaultClient,
+		httpClient: httpClient,
 		headers: http.Header{
 			"Content-Type": []string{ContentType},
 		},