[performance] Don't retry/backoff invalid http requests that will never succeed (#609)v0.3.4

* add httpguts (ew)

* add ValidateRequest err wrapping logic

* don't retry on unrecoverable errors

* i am very clever

3 files changed, 80 insertions, 2 deletions

diff --git a/internal/httpclient/client.go b/internal/httpclient/client.go
index 1a1f5e53b..55b98bbb6 100644
--- a/internal/httpclient/client.go
+++ b/internal/httpclient/client.go
@@ -28,6 +28,9 @@ import (
 	"time"
 )
 
+// ErrInvalidRequest is returned if a given HTTP request is invalid and cannot be performed.
+var ErrInvalidRequest = errors.New("invalid http request")
+
 // ErrReservedAddr is returned if a dialed address resolves to an IP within a blocked or reserved net.
 var ErrReservedAddr = errors.New("dial within blocked / reserved IP range")
 
@@ -164,6 +167,11 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
 		defer func() { <-c.queue }()
 	}
 
+	// Firstly, ensure this is a valid request
+	if err := ValidateRequest(req); err != nil {
+		return nil, err
+	}
+
 	// Perform the HTTP request
 	rsp, err := c.client.Do(req)
 	if err != nil {
diff --git a/internal/httpclient/request.go b/internal/httpclient/request.go
new file mode 100644
index 000000000..c58d0b565
--- /dev/null
+++ b/internal/httpclient/request.go
@@ -0,0 +1,63 @@
+/*
+   GoToSocial
+   Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU Affero General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU Affero General Public License for more details.
+
+   You should have received a copy of the GNU Affero General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package httpclient
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+
+	"golang.org/x/net/http/httpguts"
+)
+
+// ValidateRequest performs the same request validation logic found in the default
+// net/http.Transport{}.roundTrip() function, but pulls it out into this separate
+// function allowing validation errors to be wrapped under a single error type.
+func ValidateRequest(r *http.Request) error {
+	switch {
+	case r.URL == nil:
+		return fmt.Errorf("%w: nil url", ErrInvalidRequest)
+	case r.Header == nil:
+		return fmt.Errorf("%w: nil header", ErrInvalidRequest)
+	case r.URL.Host == "":
+		return fmt.Errorf("%w: empty url host", ErrInvalidRequest)
+	case r.URL.Scheme != "http" && r.URL.Scheme != "https":
+		return fmt.Errorf("%w: unsupported protocol %q", ErrInvalidRequest, r.URL.Scheme)
+	case strings.IndexFunc(r.Method, func(r rune) bool { return !httpguts.IsTokenRune(r) }) != -1:
+		return fmt.Errorf("%w: invalid method %q", ErrInvalidRequest, r.Method)
+	}
+
+	for key, values := range r.Header {
+		// Check field key name is valid
+		if !httpguts.ValidHeaderFieldName(key) {
+			return fmt.Errorf("%w: invalid header field name %q", ErrInvalidRequest, key)
+		}
+
+		// Check each field value is valid
+		for i := 0; i < len(values); i++ {
+			if !httpguts.ValidHeaderFieldValue(values[i]) {
+				return fmt.Errorf("%w: invalid header field value %q", ErrInvalidRequest, values[i])
+			}
+		}
+	}
+
+	// ps. kim wrote this
+
+	return nil
+}
diff --git a/internal/transport/transport.go b/internal/transport/transport.go
index c52686c43..22dfbeb9a 100644
--- a/internal/transport/transport.go
+++ b/internal/transport/transport.go
@@ -35,6 +35,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/superseriousbusiness/activity/pub"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/httpclient"
 )
 
 // Transport wraps the pub.Transport interface with some additional functionality for fetching remote media.
@@ -123,8 +124,14 @@ func (t *transport) do(r *http.Request, signer func(*http.Request) error, retryO
 
 			// Generate error from status code for logging
 			err = errors.New(`http response "` + rsp.Status + `"`)
-		} else if errorsv2.Is(err, context.DeadlineExceeded, context.Canceled) {
-			// Return early if context has cancelled
+		} else if errorsv2.Is(err,
+			context.DeadlineExceeded,
+			context.Canceled,
+			httpclient.ErrInvalidRequest,
+			httpclient.ErrBodyTooLarge,
+			httpclient.ErrReservedAddr,
+		) {
+			// Return on non-retryable errors
 			return nil, err
 		} else if strings.Contains(err.Error(), "stopped after 10 redirects") {
 			// Don't bother if net/http returned after too many redirects