Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
81 changes: 68 additions & 13 deletions internal/reader/fetcher/request_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,16 @@ type RequestBuilder struct {
clientProxyURL *url.URL
clientTimeout time.Duration
useClientProxy bool
useKeepalive bool
withoutRedirects bool
ignoreTLSErrors bool
disableHTTP2 bool
disableCompression bool
proxyRotator *proxyrotator.ProxyRotator
feedProxyURL string

// client is lazily built and reused when keepalive is enabled.
client *http.Client
}

func NewRequestBuilder() *RequestBuilder {
Expand Down Expand Up @@ -125,6 +129,14 @@ func (r *RequestBuilder) WithoutRedirects() *RequestBuilder {
return r
}

func (r *RequestBuilder) UseKeepalive(value bool) *RequestBuilder {
r.useKeepalive = value
if !value {
r.Close()
}
return r
}

func (r *RequestBuilder) DisableHTTP2(value bool) *RequestBuilder {
r.disableHTTP2 = value
return r
Expand All @@ -141,6 +153,44 @@ func (r *RequestBuilder) WithoutCompression() *RequestBuilder {
}

func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, error) {
client, err := r.clientOrNew()
if err != nil {
return nil, err
}

headers := r.headers.Clone()
if !r.useKeepalive {
headers.Set("Connection", "close")
defer r.Close()
}

return r.send(client, requestURL, headers)
}

// Close releases idle connections held by the keep-alive client, if any.
// Safe to call even if no request was sent.
func (r *RequestBuilder) Close() {
if r.client != nil {
r.client.CloseIdleConnections()
r.client = nil
}
}

func (r *RequestBuilder) clientOrNew() (*http.Client, error) {
if r.client == nil {
client, err := r.newClient()
if err != nil {
return nil, err
}
r.client = client
}

return r.client, nil
}

// newClient builds a configured *http.Client from the builder's settings.
// The proxy rotator advances once per call.
func (r *RequestBuilder) newClient() (*http.Client, error) {
var clientProxyURL *url.URL

switch {
Expand Down Expand Up @@ -231,14 +281,13 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
transport.TLSNextProto = map[string]func(string, *tls.Conn) http.RoundTripper{}
}

var clientProxyURLRedacted string
if clientProxyURL != nil {
transport.Proxy = http.ProxyURL(clientProxyURL)
clientProxyURLRedacted = clientProxyURL.Redacted()
}

client := &http.Client{
Timeout: r.clientTimeout,
Timeout: r.clientTimeout,
Transport: transport,
}

if r.withoutRedirects {
Expand All @@ -247,14 +296,28 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
}
}

client.Transport = transport
var clientProxyURLRedacted string
if clientProxyURL != nil {
clientProxyURLRedacted = clientProxyURL.Redacted()
}
slog.Debug("Built HTTP client", slog.Group("client",
slog.Bool("without_redirects", r.withoutRedirects),
slog.Bool("use_app_client_proxy", r.useClientProxy),
slog.String("client_proxy_url", clientProxyURLRedacted),
slog.Bool("ignore_tls_errors", r.ignoreTLSErrors),
slog.Bool("disable_http2", r.disableHTTP2),
))

return client, nil
}

func (r *RequestBuilder) send(client *http.Client, requestURL string, headers http.Header) (*http.Response, error) {
req, err := http.NewRequest("GET", requestURL, nil)
if err != nil {
return nil, err
}

req.Header = r.headers
req.Header = headers.Clone()
if r.disableCompression {
req.Header.Set("Accept-Encoding", "identity")
} else {
Expand All @@ -267,19 +330,11 @@ func (r *RequestBuilder) ExecuteRequest(requestURL string) (*http.Response, erro
req.Header.Set("Accept", defaultAcceptHeader)
}

req.Header.Set("Connection", "close")

slog.Debug("Making outgoing request", slog.Group("request",
slog.String("method", req.Method),
slog.String("url", req.URL.String()),
slog.Any("headers", req.Header),
slog.Bool("without_redirects", r.withoutRedirects),
slog.Bool("use_app_client_proxy", r.useClientProxy),
slog.String("client_proxy_url", clientProxyURLRedacted),
slog.Bool("ignore_tls_errors", r.ignoreTLSErrors),
slog.Bool("disable_http2", r.disableHTTP2),
))

return client.Do(req)
}

Expand Down
80 changes: 80 additions & 0 deletions internal/reader/fetcher/request_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
package fetcher // import "miniflux.app/v2/internal/reader/fetcher"

import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync"
"testing"
"time"

Expand Down Expand Up @@ -336,6 +338,84 @@ func TestRequestBuilder_ConnectionCloseHeader(t *testing.T) {
defer resp.Body.Close()
}

func TestRequestBuilder_UseKeepaliveNoConnectionClose(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if got := r.Header.Get("Connection"); got == "close" {
t.Errorf("UseKeepalive should not send 'Connection: close', got %q", got)
}
w.WriteHeader(http.StatusOK)
}))
defer server.Close()

builder := NewRequestBuilder().UseKeepalive(true)
defer builder.Close()

resp, err := builder.ExecuteRequest(server.URL)
if err != nil {
t.Fatalf("ExecuteRequest: %v", err)
}
resp.Body.Close()
}

func TestRequestBuilder_UseKeepaliveReusesConnection(t *testing.T) {
var (
mu sync.Mutex
remotes = map[string]struct{}{}
requests int
)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
remotes[r.RemoteAddr] = struct{}{}
requests++
mu.Unlock()
w.WriteHeader(http.StatusOK)
}))
defer server.Close()

builder := NewRequestBuilder().UseKeepalive(true)
defer builder.Close()

for range 5 {
resp, err := builder.ExecuteRequest(server.URL)
if err != nil {
t.Fatalf("ExecuteRequest: %v", err)
}
// Drain + close so the connection returns to the idle pool and the
// next iteration can reuse it.
io.Copy(io.Discard, resp.Body)
resp.Body.Close()
}

if requests != 5 {
t.Fatalf("expected 5 requests, got %d", requests)
}
if len(remotes) != 1 {
t.Errorf("expected a single reused TCP connection, got %d distinct remotes: %v", len(remotes), remotes)
}
}

func TestRequestBuilder_ExecuteRequestDoesNotMutateBuilderHeaders(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}))
defer server.Close()

builder := NewRequestBuilder()
resp, err := builder.ExecuteRequest(server.URL)
if err != nil {
t.Fatalf("ExecuteRequest: %v", err)
}
resp.Body.Close()

// Per-request headers (Accept, Accept-Encoding, Connection) must not leak
// back into the builder so that it can be reused for further requests.
for _, h := range []string{"Accept", "Accept-Encoding", "Connection"} {
if got := builder.headers.Get(h); got != "" {
t.Errorf("builder.headers[%q] leaked from ExecuteRequest: %q", h, got)
}
}
}

func TestRequestBuilder_WithCustomApplicationProxyURL(t *testing.T) {
proxyURL, _ := url.Parse("http://proxy.example.com:8080")
builder := NewRequestBuilder()
Expand Down
14 changes: 8 additions & 6 deletions internal/reader/subscription/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@ func (f *subscriptionFinder) findSubscriptionsFromWellKnownURLs(websiteURL strin
baseURLs = append(baseURLs, websiteURL)
}

// Some websites redirects unknown URLs to the home page.
// As result, the list of known URLs is returned to the subscription list.
// We don't want the user to choose between invalid feed URLs.
f.requestBuilder.WithoutRedirects().UseKeepalive(true)

defer f.requestBuilder.Close()

var subscriptions Subscriptions
for _, baseURL := range baseURLs {
for _, known := range knownURLs {
Expand All @@ -218,12 +225,7 @@ func (f *subscriptionFinder) findSubscriptionsFromWellKnownURLs(websiteURL strin
continue
}

// Some websites redirects unknown URLs to the home page.
// As result, the list of known URLs is returned to the subscription list.
// We don't want the user to choose between invalid feed URLs.
requestBuilder := f.requestBuilder.WithoutRedirects()

responseHandler := fetcher.NewResponseHandler(requestBuilder.ExecuteRequest(fullURL))
responseHandler := fetcher.NewResponseHandler(f.requestBuilder.ExecuteRequest(fullURL))
localizedError := responseHandler.LocalizedError()
responseHandler.Close()

Expand Down