Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[performance] Don't retry/backoff invalid http requests that will never succeed #609

Merged
merged 4 commits into from
May 26, 2022
Merged
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
8 changes: 8 additions & 0 deletions internal/httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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 {
Expand Down
63 changes: 63 additions & 0 deletions internal/httpclient/request.go
Original file line number Diff line number Diff line change
@@ -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
}
11 changes: 9 additions & 2 deletions internal/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions vendor/golang.org/x/net/http/httpguts/guts.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading