[performance] Don't retry/backoff invalid http requests that will nev…

…er succeed (#609)

* add httpguts (ew)

* add ValidateRequest err wrapping logic

* don't retry on unrecoverable errors

* i am very clever

go.mod

@@ -44,7 +44,7 @@ require (
 	github.com/uptrace/bun/dialect/sqlitedialect v1.1.3
 	github.com/wagslane/go-password-validator v0.3.0
 	golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f
-	golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4
+	golang.org/x/net v0.0.0-20220524220425-1d687d428aca
 	golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5
 	golang.org/x/text v0.3.7
 	gopkg.in/mcuadros/go-syslog.v2 v2.3.0

go.sum

@@ -665,6 +665,8 @@ golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4 h1:HVyaeDAYux4pnY+D/SiwmLOR36ewZ4iGQIIrtnuCjFA=
 golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
+golang.org/x/net v0.0.0-20220524220425-1d687d428aca h1:xTaFYiPROfpPhqrfTIDXj0ri1SpfueYT951s4bAuDO8=
+golang.org/x/net v0.0.0-20220524220425-1d687d428aca/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=

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 {

internal/httpclient/request.go

@@ -0,0 +1,63 @@
+/*
+   GoToSocial
+   Copyright (C) 2021-2022 GoToSocial Authors [email protected]
+
+   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
+}

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