From 16b7b3bc3e4237abbacdf8695a685ca9a03dc5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Sun, 27 Sep 2020 14:29:48 -0700 Subject: [PATCH] http client: remove dependency on global config options --- http/client/client.go | 131 ++++++++++++++++++++-------------- http/client/client_test.go | 51 +++++++++++++ reader/feed/handler.go | 4 +- reader/icon/finder.go | 5 +- reader/scraper/scraper.go | 3 +- reader/subscription/finder.go | 23 +++--- ui/opml_upload.go | 3 +- 7 files changed, 148 insertions(+), 72 deletions(-) create mode 100644 http/client/client_test.go diff --git a/http/client/client.go b/http/client/client.go index fb416f72..4d91b3e8 100644 --- a/http/client/client.go +++ b/http/client/client.go @@ -6,7 +6,6 @@ package client // import "miniflux.app/http/client" import ( "bytes" - "crypto/tls" "crypto/x509" "encoding/json" "fmt" @@ -26,6 +25,11 @@ import ( "miniflux.app/version" ) +const ( + defaultHTTPClientTimeout = 20 + defaultHTTPClientMaxBodySize = 15 * 1024 * 1024 +) + var ( // DefaultUserAgent sets the User-Agent header used for any requests by miniflux. DefaultUserAgent = "Mozilla/5.0 (compatible; Miniflux/" + version.Version + "; +https://miniflux.app)" @@ -36,79 +40,105 @@ var ( errRequestTimeout = "Website unreachable, the request timed out after %d seconds" ) -// Client is a HTTP Client :) +// Client builds and executes HTTP requests. type Client struct { - inputURL string - requestURL string - etagHeader string - lastModifiedHeader string - authorizationHeader string - username string - password string - userAgent string - Insecure bool - fetchViaProxy bool + inputURL string + + requestURL string + requestEtagHeader string + requestLastModifiedHeader string + requestAuthorizationHeader string + requestUsername string + requestPassword string + requestUserAgent string + + useProxy bool + + ClientTimeout int + ClientMaxBodySize int64 + ClientProxyURL string +} + +// New initializes a new HTTP client. +func New(url string) *Client { + return &Client{ + inputURL: url, + requestUserAgent: DefaultUserAgent, + ClientTimeout: defaultHTTPClientTimeout, + ClientMaxBodySize: defaultHTTPClientMaxBodySize, + } +} + +// NewClientWithConfig initializes a new HTTP client with application config options. +func NewClientWithConfig(url string, opts *config.Options) *Client { + return &Client{ + inputURL: url, + requestUserAgent: DefaultUserAgent, + ClientTimeout: opts.HTTPClientTimeout(), + ClientMaxBodySize: opts.HTTPClientMaxBodySize(), + ClientProxyURL: opts.HTTPClientProxy(), + } } func (c *Client) String() string { - etagHeader := c.etagHeader - if c.etagHeader == "" { + etagHeader := c.requestEtagHeader + if c.requestEtagHeader == "" { etagHeader = "None" } - lastModifiedHeader := c.lastModifiedHeader - if c.lastModifiedHeader == "" { + lastModifiedHeader := c.requestLastModifiedHeader + if c.requestLastModifiedHeader == "" { lastModifiedHeader = "None" } return fmt.Sprintf( - `InputURL=%q RequestURL=%q ETag=%s LastModified=%s BasicAuth=%v UserAgent=%q`, + `InputURL=%q RequestURL=%q ETag=%s LastModified=%s Auth=%v UserAgent=%q`, c.inputURL, c.requestURL, etagHeader, lastModifiedHeader, - c.authorizationHeader != "" || (c.username != "" && c.password != ""), - c.userAgent, + c.requestAuthorizationHeader != "" || (c.requestUsername != "" && c.requestPassword != ""), + c.requestUserAgent, ) } // WithCredentials defines the username/password for HTTP Basic authentication. func (c *Client) WithCredentials(username, password string) *Client { if username != "" && password != "" { - c.username = username - c.password = password + c.requestUsername = username + c.requestPassword = password } return c } -// WithAuthorization defines authorization header value. +// WithAuthorization defines the authorization HTTP header value. func (c *Client) WithAuthorization(authorization string) *Client { - c.authorizationHeader = authorization + c.requestAuthorizationHeader = authorization return c } // WithCacheHeaders defines caching headers. func (c *Client) WithCacheHeaders(etagHeader, lastModifiedHeader string) *Client { - c.etagHeader = etagHeader - c.lastModifiedHeader = lastModifiedHeader + c.requestLastModifiedHeader = etagHeader + c.requestLastModifiedHeader = lastModifiedHeader return c } -// WithProxy enable proxy for current HTTP client request. +// WithProxy enable proxy for the current HTTP request. func (c *Client) WithProxy() *Client { - c.fetchViaProxy = true + c.useProxy = true return c } -// WithUserAgent defines the User-Agent header to use for outgoing requests. +// WithUserAgent defines the User-Agent header to use for HTTP requests. func (c *Client) WithUserAgent(userAgent string) *Client { if userAgent != "" { - c.userAgent = userAgent + c.requestUserAgent = userAgent } return c } -// Get execute a GET HTTP request. +// Get performs a GET HTTP request. func (c *Client) Get() (*Response, error) { request, err := c.buildRequest(http.MethodGet, nil) if err != nil { @@ -118,7 +148,7 @@ func (c *Client) Get() (*Response, error) { return c.executeRequest(request) } -// PostForm execute a POST HTTP request with form values. +// PostForm performs a POST HTTP request with form encoded values. func (c *Client) PostForm(values url.Values) (*Response, error) { request, err := c.buildRequest(http.MethodPost, strings.NewReader(values.Encode())) if err != nil { @@ -129,7 +159,7 @@ func (c *Client) PostForm(values url.Values) (*Response, error) { return c.executeRequest(request) } -// PostJSON execute a POST HTTP request with JSON payload. +// PostJSON performs a POST HTTP request with a JSON payload. func (c *Client) PostJSON(data interface{}) (*Response, error) { b, err := json.Marshal(data) if err != nil { @@ -173,7 +203,7 @@ func (c *Client) executeRequest(request *http.Request) (*Response, error) { case net.Error: nerr := uerr.Err.(net.Error) if nerr.Timeout() { - err = errors.NewLocalizedError(errRequestTimeout, config.Opts.HTTPClientTimeout()) + err = errors.NewLocalizedError(errRequestTimeout, c.ClientTimeout) } else if nerr.Temporary() { err = errors.NewLocalizedError(errTemporaryNetworkOperation, nerr) } @@ -183,7 +213,7 @@ func (c *Client) executeRequest(request *http.Request) (*Response, error) { return nil, err } - if resp.ContentLength > config.Opts.HTTPClientMaxBodySize() { + if resp.ContentLength > c.ClientMaxBodySize { return nil, fmt.Errorf("client: response too large (%d bytes)", resp.ContentLength) } @@ -228,15 +258,15 @@ func (c *Client) buildRequest(method string, body io.Reader) (*http.Request, err request.Header = c.buildHeaders() - if c.username != "" && c.password != "" { - request.SetBasicAuth(c.username, c.password) + if c.requestUsername != "" && c.requestPassword != "" { + request.SetBasicAuth(c.requestUsername, c.requestPassword) } return request, nil } func (c *Client) buildClient() http.Client { - client := http.Client{Timeout: time.Duration(config.Opts.HTTPClientTimeout()) * time.Second} + client := http.Client{Timeout: time.Duration(c.ClientTimeout) * time.Second} transport := &http.Transport{ DialContext: (&net.Dialer{ // Default is 30s. @@ -253,12 +283,8 @@ func (c *Client) buildClient() http.Client { IdleConnTimeout: 10 * time.Second, } - if c.Insecure { - transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} - } - - if c.fetchViaProxy && config.Opts.HasHTTPClientProxyConfigured() { - proxyURL, err := url.Parse(config.Opts.HTTPClientProxy()) + if c.useProxy && c.ClientProxyURL != "" { + proxyURL, err := url.Parse(c.ClientProxyURL) if err != nil { logger.Error("[HttpClient] Proxy URL error: %v", err) } else { @@ -274,26 +300,21 @@ func (c *Client) buildClient() http.Client { func (c *Client) buildHeaders() http.Header { headers := make(http.Header) - headers.Add("User-Agent", c.userAgent) + headers.Add("User-Agent", c.requestUserAgent) headers.Add("Accept", "*/*") - if c.etagHeader != "" { - headers.Add("If-None-Match", c.etagHeader) + if c.requestEtagHeader != "" { + headers.Add("If-None-Match", c.requestEtagHeader) } - if c.lastModifiedHeader != "" { - headers.Add("If-Modified-Since", c.lastModifiedHeader) + if c.requestLastModifiedHeader != "" { + headers.Add("If-Modified-Since", c.requestLastModifiedHeader) } - if c.authorizationHeader != "" { - headers.Add("Authorization", c.authorizationHeader) + if c.requestAuthorizationHeader != "" { + headers.Add("Authorization", c.requestAuthorizationHeader) } headers.Add("Connection", "close") return headers } - -// New returns a new HTTP client. -func New(url string) *Client { - return &Client{inputURL: url, userAgent: DefaultUserAgent, Insecure: false} -} diff --git a/http/client/client_test.go b/http/client/client_test.go new file mode 100644 index 00000000..fb66d1ec --- /dev/null +++ b/http/client/client_test.go @@ -0,0 +1,51 @@ +// Copyright 2020 Frédéric Guillot. All rights reserved. +// Use of this source code is governed by the Apache 2.0 +// license that can be found in the LICENSE file. + +package client // import "miniflux.app/http/client" + +import "testing" + +func TestClientWithDelay(t *testing.T) { + clt := New("http://httpbin.org/delay/5") + clt.ClientTimeout = 1 + _, err := clt.Get() + if err == nil { + t.Fatal(`The client should stops after 1 second`) + } +} + +func TestClientWithError(t *testing.T) { + clt := New("http://httpbin.org/status/502") + clt.ClientTimeout = 1 + response, err := clt.Get() + if err != nil { + t.Fatal(err) + } + + if response.StatusCode != 502 { + t.Fatalf(`Unexpected response status code: %d`, response.StatusCode) + } + + if !response.HasServerFailure() { + t.Fatal(`A 500 error is considered as server failure`) + } +} + +func TestClientWithResponseTooLarge(t *testing.T) { + clt := New("http://httpbin.org/bytes/100") + clt.ClientMaxBodySize = 10 + _, err := clt.Get() + if err == nil { + t.Fatal(`The client should fails when reading a response too large`) + } +} + +func TestClientWithBasicAuth(t *testing.T) { + clt := New("http://httpbin.org/basic-auth/testuser/testpassword") + clt.WithCredentials("testuser", "testpassword") + _, err := clt.Get() + if err != nil { + t.Fatalf(`The client should be authenticated successfully: %v`, err) + } +} diff --git a/reader/feed/handler.go b/reader/feed/handler.go index e1bedaab..20f19ce9 100644 --- a/reader/feed/handler.go +++ b/reader/feed/handler.go @@ -41,7 +41,7 @@ func (h *Handler) CreateFeed(userID, categoryID int64, url string, crawler bool, return nil, errors.NewLocalizedError(errCategoryNotFound) } - request := client.New(url) + request := client.NewClientWithConfig(url, config.Opts) request.WithCredentials(username, password) request.WithUserAgent(userAgent) @@ -108,7 +108,7 @@ func (h *Handler) RefreshFeed(userID, feedID int64) error { originalFeed.CheckedNow() originalFeed.ScheduleNextCheck(weeklyEntryCount) - request := client.New(originalFeed.FeedURL) + request := client.NewClientWithConfig(originalFeed.FeedURL, config.Opts) request.WithCredentials(originalFeed.Username, originalFeed.Password) request.WithUserAgent(originalFeed.UserAgent) diff --git a/reader/icon/finder.go b/reader/icon/finder.go index 5dde2235..231528d4 100644 --- a/reader/icon/finder.go +++ b/reader/icon/finder.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "strings" + "miniflux.app/config" "miniflux.app/crypto" "miniflux.app/http/client" "miniflux.app/logger" @@ -23,7 +24,7 @@ import ( // FindIcon try to find the website's icon. func FindIcon(websiteURL string, fetchViaProxy bool) (*model.Icon, error) { rootURL := url.RootURL(websiteURL) - clt := client.New(rootURL) + clt := client.NewClientWithConfig(rootURL, config.Opts) if fetchViaProxy { clt.WithProxy() } @@ -90,7 +91,7 @@ func parseDocument(websiteURL string, data io.Reader) (string, error) { } func downloadIcon(iconURL string, fetchViaProxy bool) (*model.Icon, error) { - clt := client.New(iconURL) + clt := client.NewClientWithConfig(iconURL, config.Opts) if fetchViaProxy { clt.WithProxy() } diff --git a/reader/scraper/scraper.go b/reader/scraper/scraper.go index 045bfbc0..c0b968a7 100644 --- a/reader/scraper/scraper.go +++ b/reader/scraper/scraper.go @@ -10,6 +10,7 @@ import ( "io" "strings" + "miniflux.app/config" "miniflux.app/http/client" "miniflux.app/logger" "miniflux.app/reader/readability" @@ -20,7 +21,7 @@ import ( // Fetch downloads a web page and returns relevant contents. func Fetch(websiteURL, rules, userAgent string) (string, error) { - clt := client.New(websiteURL) + clt := client.NewClientWithConfig(websiteURL, config.Opts) if userAgent != "" { clt.WithUserAgent(userAgent) } diff --git a/reader/subscription/finder.go b/reader/subscription/finder.go index e4f2a812..db5e22c6 100644 --- a/reader/subscription/finder.go +++ b/reader/subscription/finder.go @@ -10,6 +10,7 @@ import ( "regexp" "strings" + "miniflux.app/config" "miniflux.app/errors" "miniflux.app/http/client" "miniflux.app/reader/browser" @@ -30,15 +31,15 @@ func FindSubscriptions(websiteURL, userAgent, username, password string, fetchVi websiteURL = findYoutubeChannelFeed(websiteURL) websiteURL = parseYoutubeVideoPage(websiteURL) - request := client.New(websiteURL) - request.WithCredentials(username, password) - request.WithUserAgent(userAgent) + clt := client.NewClientWithConfig(websiteURL, config.Opts) + clt.WithCredentials(username, password) + clt.WithUserAgent(userAgent) if fetchViaProxy { - request.WithProxy() + clt.WithProxy() } - response, err := browser.Exec(request) + response, err := browser.Exec(clt) if err != nil { return nil, err } @@ -118,8 +119,8 @@ func parseYoutubeVideoPage(websiteURL string) string { return websiteURL } - request := client.New(websiteURL) - response, browserErr := browser.Exec(request) + clt := client.NewClientWithConfig(websiteURL, config.Opts) + response, browserErr := browser.Exec(clt) if browserErr != nil { return websiteURL } @@ -155,10 +156,10 @@ func tryWellKnownUrls(websiteURL, userAgent, username, password string) (Subscri if err != nil { continue } - request := client.New(fullURL) - request.WithCredentials(username, password) - request.WithUserAgent(userAgent) - response, err := request.Get() + clt := client.NewClientWithConfig(fullURL, config.Opts) + clt.WithCredentials(username, password) + clt.WithUserAgent(userAgent) + response, err := clt.Get() if err != nil { continue } diff --git a/ui/opml_upload.go b/ui/opml_upload.go index 4c94db65..6f9109bf 100644 --- a/ui/opml_upload.go +++ b/ui/opml_upload.go @@ -7,6 +7,7 @@ package ui // import "miniflux.app/ui" import ( "net/http" + "miniflux.app/config" "miniflux.app/http/client" "miniflux.app/http/request" "miniflux.app/http/response/html" @@ -87,7 +88,7 @@ func (h *handler) fetchOPML(w http.ResponseWriter, r *http.Request) { view.Set("countUnread", h.store.CountUnreadEntries(user.ID)) view.Set("countErrorFeeds", h.store.CountErrorFeeds(user.ID)) - clt := client.New(url) + clt := client.NewClientWithConfig(url, config.Opts) resp, err := clt.Get() if err != nil { view.Set("errorMessage", err)