http client: remove dependency on global config options

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
+	inputURL string
-	requestURL          string
+
-	etagHeader          string
+	requestURL                 string
-	lastModifiedHeader  string
+	requestEtagHeader          string
-	authorizationHeader string
+	requestLastModifiedHeader  string
-	username            string
+	requestAuthorizationHeader string
-	password            string
+	requestUsername            string
-	userAgent           string
+	requestPassword            string
-	Insecure            bool
+	requestUserAgent           string
-	fetchViaProxy       bool
+
+	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
+	etagHeader := c.requestEtagHeader
-	if c.etagHeader == "" {
+	if c.requestEtagHeader == "" {
 		etagHeader = "None"
 	}
 
-	lastModifiedHeader := c.lastModifiedHeader
+	lastModifiedHeader := c.requestLastModifiedHeader
-	if c.lastModifiedHeader == "" {
+	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.requestAuthorizationHeader != "" || (c.requestUsername != "" && c.requestPassword != ""),
-		c.userAgent,
+		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.requestUsername = username
-		c.password = password
+		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.requestLastModifiedHeader = etagHeader
-	c.lastModifiedHeader = lastModifiedHeader
+	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 != "" {
+	if c.requestUsername != "" && c.requestPassword != "" {
-		request.SetBasicAuth(c.username, c.password)
+		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 {
+	if c.useProxy && c.ClientProxyURL != "" {
-		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
+		proxyURL, err := url.Parse(c.ClientProxyURL)
-	}
-
-	if c.fetchViaProxy && config.Opts.HasHTTPClientProxyConfigured() {
-		proxyURL, err := url.Parse(config.Opts.HTTPClientProxy())
 		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 != "" {
+	if c.requestEtagHeader != "" {
-		headers.Add("If-None-Match", c.etagHeader)
+		headers.Add("If-None-Match", c.requestEtagHeader)
 	}
 
-	if c.lastModifiedHeader != "" {
+	if c.requestLastModifiedHeader != "" {
-		headers.Add("If-Modified-Since", c.lastModifiedHeader)
+		headers.Add("If-Modified-Since", c.requestLastModifiedHeader)
 	}
 
-	if c.authorizationHeader != "" {
+	if c.requestAuthorizationHeader != "" {
-		headers.Add("Authorization", c.authorizationHeader)
+		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}
-}

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)
+	}
+}

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)
 

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()
 	}

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)
 	}

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)
+	clt := client.NewClientWithConfig(websiteURL, config.Opts)
-	request.WithCredentials(username, password)
+	clt.WithCredentials(username, password)
-	request.WithUserAgent(userAgent)
+	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)
+	clt := client.NewClientWithConfig(websiteURL, config.Opts)
-	response, browserErr := browser.Exec(request)
+	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)
+		clt := client.NewClientWithConfig(fullURL, config.Opts)
-		request.WithCredentials(username, password)
+		clt.WithCredentials(username, password)
-		request.WithUserAgent(userAgent)
+		clt.WithUserAgent(userAgent)
-		response, err := request.Get()
+		response, err := clt.Get()
 		if err != nil {
 			continue
 		}

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)