From 13d9d86acd988da5929da829ccd251916a9517b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Sat, 12 Aug 2023 19:01:22 -0700 Subject: [PATCH] Consider base path when generating third-party services API endpoint --- internal/integration/apprise/apprise.go | 15 ++++++--- internal/integration/espial/espial.go | 20 +++--------- internal/integration/linkding/linkding.go | 27 ++++------------ .../integration/nunuxkeeper/nunuxkeeper.go | 18 +++-------- internal/integration/pinboard/pinboard.go | 2 +- internal/integration/readwise/readwise.go | 2 +- internal/integration/wallabag/wallabag.go | 9 +++--- internal/url/url.go | 23 ++++++++++++++ internal/url/url_test.go | 31 +++++++++++++++++++ 9 files changed, 87 insertions(+), 60 deletions(-) diff --git a/internal/integration/apprise/apprise.go b/internal/integration/apprise/apprise.go index b073788b..89ea3b6e 100644 --- a/internal/integration/apprise/apprise.go +++ b/internal/integration/apprise/apprise.go @@ -11,8 +11,11 @@ import ( "miniflux.app/v2/internal/http/client" "miniflux.app/v2/internal/model" + "miniflux.app/v2/internal/url" ) +const defaultClientTimeout = 1 * time.Second + // Client represents a Apprise client. type Client struct { servicesURL string @@ -27,12 +30,16 @@ func NewClient(serviceURL, baseURL string) *Client { // PushEntry pushes entry to apprise func (c *Client) PushEntry(entry *model.Entry) error { if c.baseURL == "" || c.servicesURL == "" { - return fmt.Errorf("apprise: missing credentials") + return fmt.Errorf("apprise: missing base URL or service URL") } - timeout := time.Duration(1 * time.Second) - _, err := net.DialTimeout("tcp", c.baseURL, timeout) + _, err := net.DialTimeout("tcp", c.baseURL, defaultClientTimeout) if err != nil { - clt := client.New(c.baseURL + "/notify") + apiEndpoint, err := url.JoinBaseURLAndPath(c.baseURL, "/notify") + if err != nil { + return fmt.Errorf(`apprise: invalid API endpoint: %v`, err) + } + + clt := client.New(apiEndpoint) message := "[" + entry.Title + "]" + "(" + entry.URL + ")" + "\n\n" data := &Data{ Urls: c.servicesURL, diff --git a/internal/integration/espial/espial.go b/internal/integration/espial/espial.go index 293adca9..4751a880 100644 --- a/internal/integration/espial/espial.go +++ b/internal/integration/espial/espial.go @@ -5,10 +5,9 @@ package espial // import "miniflux.app/v2/internal/integration/espial" import ( "fmt" - "net/url" - "path" "miniflux.app/v2/internal/http/client" + "miniflux.app/v2/internal/url" ) // Document structure of an Espial document @@ -33,7 +32,7 @@ func NewClient(baseURL, apiKey string) *Client { // AddEntry sends an entry to Espial. func (c *Client) AddEntry(link, title, content, tags string) error { if c.baseURL == "" || c.apiKey == "" { - return fmt.Errorf("espial: missing credentials") + return fmt.Errorf("espial: missing base URL or API key") } doc := &Document{ @@ -43,12 +42,12 @@ func (c *Client) AddEntry(link, title, content, tags string) error { Tags: tags, } - apiURL, err := getAPIEndpoint(c.baseURL, "/api/add") + apiEndpoint, err := url.JoinBaseURLAndPath(c.baseURL, "/api/add") if err != nil { - return err + return fmt.Errorf(`espial: invalid API endpoint: %v`, err) } - clt := client.New(apiURL) + clt := client.New(apiEndpoint) clt.WithAuthorization("ApiKey " + c.apiKey) response, err := clt.PostJSON(doc) if err != nil { @@ -61,12 +60,3 @@ func (c *Client) AddEntry(link, title, content, tags string) error { return nil } - -func getAPIEndpoint(baseURL, pathURL string) (string, error) { - u, err := url.Parse(baseURL) - if err != nil { - return "", fmt.Errorf("espial: invalid API endpoint: %v", err) - } - u.Path = path.Join(u.Path, pathURL) - return u.String(), nil -} diff --git a/internal/integration/linkding/linkding.go b/internal/integration/linkding/linkding.go index 8ed960d0..8fa98602 100644 --- a/internal/integration/linkding/linkding.go +++ b/internal/integration/linkding/linkding.go @@ -5,10 +5,10 @@ package linkding // import "miniflux.app/v2/internal/integration/linkding" import ( "fmt" - "net/url" "strings" "miniflux.app/v2/internal/http/client" + "miniflux.app/v2/internal/url" ) // Document structure of a Linkding document @@ -33,7 +33,7 @@ func NewClient(baseURL, apiKey, tags string, unread bool) *Client { } // AddEntry sends an entry to Linkding. -func (c *Client) AddEntry(title, url string) error { +func (c *Client) AddEntry(title, entryURL string) error { if c.baseURL == "" || c.apiKey == "" { return fmt.Errorf("linkding: missing credentials") } @@ -43,18 +43,18 @@ func (c *Client) AddEntry(title, url string) error { } doc := &Document{ - Url: url, + Url: entryURL, Title: title, TagNames: strings.FieldsFunc(c.tags, tagsSplitFn), Unread: c.unread, } - apiURL, err := getAPIEndpoint(c.baseURL, "/api/bookmarks/") + apiEndpoint, err := url.JoinBaseURLAndPath(c.baseURL, "/api/bookmarks/") if err != nil { - return err + return fmt.Errorf(`linkding: invalid API endpoint: %v`, err) } - clt := client.New(apiURL) + clt := client.New(apiEndpoint) clt.WithAuthorization("Token " + c.apiKey) response, err := clt.PostJSON(doc) if err != nil { @@ -67,18 +67,3 @@ func (c *Client) AddEntry(title, url string) error { return nil } - -func getAPIEndpoint(baseURL, pathURL string) (string, error) { - u, err := url.Parse(baseURL) - if err != nil { - return "", fmt.Errorf("linkding: invalid API endpoint: %v", err) - } - - relative, err := url.Parse(pathURL) - if err != nil { - return "", fmt.Errorf("linkding: invalid API endpoint: %v", err) - } - - u = u.ResolveReference(relative) - return u.String(), nil -} diff --git a/internal/integration/nunuxkeeper/nunuxkeeper.go b/internal/integration/nunuxkeeper/nunuxkeeper.go index 6ad37f23..b734cc98 100644 --- a/internal/integration/nunuxkeeper/nunuxkeeper.go +++ b/internal/integration/nunuxkeeper/nunuxkeeper.go @@ -5,10 +5,9 @@ package nunuxkeeper // import "miniflux.app/v2/internal/integration/nunuxkeeper" import ( "fmt" - "net/url" - "path" "miniflux.app/v2/internal/http/client" + "miniflux.app/v2/internal/url" ) // Document structure of a Nununx Keeper document @@ -43,12 +42,12 @@ func (c *Client) AddEntry(link, title, content string) error { ContentType: "text/html", } - apiURL, err := getAPIEndpoint(c.baseURL, "/v2/documents") + apiEndpoint, err := url.JoinBaseURLAndPath(c.baseURL, "/v2/documents") if err != nil { - return err + return fmt.Errorf(`nunux-keeper: invalid API endpoint: %v`, err) } - clt := client.New(apiURL) + clt := client.New(apiEndpoint) clt.WithCredentials("api", c.apiKey) response, err := clt.PostJSON(doc) if err != nil { @@ -61,12 +60,3 @@ func (c *Client) AddEntry(link, title, content string) error { return nil } - -func getAPIEndpoint(baseURL, pathURL string) (string, error) { - u, err := url.Parse(baseURL) - if err != nil { - return "", fmt.Errorf("nunux-keeper: invalid API endpoint: %v", err) - } - u.Path = path.Join(u.Path, pathURL) - return u.String(), nil -} diff --git a/internal/integration/pinboard/pinboard.go b/internal/integration/pinboard/pinboard.go index 40d14c74..0977c703 100644 --- a/internal/integration/pinboard/pinboard.go +++ b/internal/integration/pinboard/pinboard.go @@ -23,7 +23,7 @@ func NewClient(authToken string) *Client { // AddBookmark sends a link to Pinboard. func (c *Client) AddBookmark(link, title, tags string, markAsUnread bool) error { if c.authToken == "" { - return fmt.Errorf("pinboard: missing credentials") + return fmt.Errorf("pinboard: missing auth token") } toRead := "no" diff --git a/internal/integration/readwise/readwise.go b/internal/integration/readwise/readwise.go index 8a8d869b..16032b30 100644 --- a/internal/integration/readwise/readwise.go +++ b/internal/integration/readwise/readwise.go @@ -31,7 +31,7 @@ func NewClient(apiKey string) *Client { // AddEntry sends an entry to Readwise Reader. func (c *Client) AddEntry(link string) error { if c.apiKey == "" { - return fmt.Errorf("readwise: missing credentials") + return fmt.Errorf("readwise: missing API key") } doc := &Document{ diff --git a/internal/integration/wallabag/wallabag.go b/internal/integration/wallabag/wallabag.go index b158bea8..b8044445 100644 --- a/internal/integration/wallabag/wallabag.go +++ b/internal/integration/wallabag/wallabag.go @@ -10,6 +10,7 @@ import ( "net/url" "miniflux.app/v2/internal/http/client" + internal_url "miniflux.app/v2/internal/url" ) // Client represents a Wallabag client. @@ -43,9 +44,9 @@ func (c *Client) AddEntry(link, title, content string) error { } func (c *Client) createEntry(accessToken, link, title, content string) error { - endpoint, err := url.JoinPath(c.baseURL, "/api/entries.json") + endpoint, err := internal_url.JoinBaseURLAndPath(c.baseURL, "/api/entries.json") if err != nil { - return fmt.Errorf("wallbag: unable to generate entries endpoint using %q: %v", c.baseURL, err) + return fmt.Errorf("wallbag: unable to generate entries endpoint: %v", err) } data := map[string]string{"url": link, "title": title} @@ -75,9 +76,9 @@ func (c *Client) getAccessToken() (string, error) { values.Add("username", c.username) values.Add("password", c.password) - endpoint, err := url.JoinPath(c.baseURL, "/oauth/v2/token") + endpoint, err := internal_url.JoinBaseURLAndPath(c.baseURL, "/oauth/v2/token") if err != nil { - return "", fmt.Errorf("wallbag: unable to generate token endpoint using %q: %v", c.baseURL, err) + return "", fmt.Errorf("wallbag: unable to generate token endpoint: %v", err) } clt := client.New(endpoint) diff --git a/internal/url/url.go b/internal/url/url.go index 3345deec..4f0b0c0f 100644 --- a/internal/url/url.go +++ b/internal/url/url.go @@ -79,3 +79,26 @@ func Domain(websiteURL string) string { return parsedURL.Host } + +// JoinBaseURLAndPath returns a URL string with the provided path elements joined together. +func JoinBaseURLAndPath(baseURL, path string) (string, error) { + if baseURL == "" { + return "", fmt.Errorf("empty base URL") + } + + if path == "" { + return "", fmt.Errorf("empty path") + } + + _, err := url.Parse(baseURL) + if err != nil { + return "", fmt.Errorf("invalid base URL: %w", err) + } + + finalURL, err := url.JoinPath(baseURL, path) + if err != nil { + return "", fmt.Errorf("unable to join base URL %s and path %s: %w", baseURL, path, err) + } + + return finalURL, nil +} diff --git a/internal/url/url_test.go b/internal/url/url_test.go index c275767b..712ad57f 100644 --- a/internal/url/url_test.go +++ b/internal/url/url_test.go @@ -89,3 +89,34 @@ func TestDomain(t *testing.T) { } } } + +func TestJoinBaseURLAndPath(t *testing.T) { + type args struct { + baseURL string + path string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + {"empty base url", args{"", "/api/bookmarks/"}, "", true}, + {"empty path", args{"https://example.com", ""}, "", true}, + {"invalid base url", args{"incorrect url", ""}, "", true}, + {"valid", args{"https://example.com", "/api/bookmarks/"}, "https://example.com/api/bookmarks/", false}, + {"valid", args{"https://example.com/subfolder", "/api/bookmarks/"}, "https://example.com/subfolder/api/bookmarks/", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := JoinBaseURLAndPath(tt.args.baseURL, tt.args.path) + if (err != nil) != tt.wantErr { + t.Errorf("JoinBaseURLAndPath error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("JoinBaseURLAndPath = %v, want %v", got, tt.want) + } + }) + } +}