Skip to content

Commit

Permalink
[bugfix] Add back removed ValidateRequest() before backoff-retry loop (
Browse files Browse the repository at this point in the history
…#1805)

* add back removed ValidateRequest() before backoff-retry loop

Signed-off-by: kim <[email protected]>

* include response body in error response log

Signed-off-by: kim <[email protected]>

* improved error response body draining

Signed-off-by: kim <[email protected]>

* add more code commenting

Signed-off-by: kim <[email protected]>

* move new error response logic to gtserror, handle instead in transport.Transport{} impl

Signed-off-by: kim <[email protected]>

* appease ye oh mighty linter

Signed-off-by: kim <[email protected]>

* fix mockhttpclient not setting request in http response

Signed-off-by: kim <[email protected]>

---------

Signed-off-by: kim <[email protected]>
  • Loading branch information
NyaaaWhatsUpDoc authored May 21, 2023
1 parent 107237c commit 2063d01
Show file tree
Hide file tree
Showing 12 changed files with 299 additions and 32 deletions.
4 changes: 2 additions & 2 deletions internal/gtserror/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const (
notFoundKey
errorTypeKey

// error types
TypeSMTP ErrorType = "smtp" // smtp (mail) error
// Types returnable from Type(...).
TypeSMTP ErrorType = "smtp" // smtp (mail)
)

// StatusCode checks error for a stored status code value. For example
Expand Down
66 changes: 66 additions & 0 deletions internal/gtserror/new.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// GoToSocial
// Copyright (C) GoToSocial Authors [email protected]
// SPDX-License-Identifier: AGPL-3.0-or-later
//
// 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 gtserror

import (
"errors"
"net/http"

"codeberg.org/gruf/go-byteutil"
)

// NewResponseError crafts an error from provided HTTP response
// including the method, status and body (if any provided). This
// will also wrap the returned error using WithStatusCode().
func NewResponseError(rsp *http.Response) error {
var buf byteutil.Buffer

// Get URL string ahead of time.
urlStr := rsp.Request.URL.String()

// Alloc guesstimate of required buf size.
buf.Guarantee(0 +
len(rsp.Request.Method) +
12 + // request to
len(urlStr) +
17 + // failed: status="
len(rsp.Status) +
8 + // " body="
256 + // max body size
1, // "
)

// Build error message string without
// using "fmt", as chances are this will
// be used in a hot code path and we
// know all the incoming types involved.
_, _ = buf.WriteString(rsp.Request.Method)
_, _ = buf.WriteString(" request to ")
_, _ = buf.WriteString(urlStr)
_, _ = buf.WriteString(" failed: status=\"")
_, _ = buf.WriteString(rsp.Status)
_, _ = buf.WriteString("\" body=\"")
_, _ = buf.WriteString(drainBody(rsp.Body, 256))
_, _ = buf.WriteString("\"")

// Create new error from msg.
err := errors.New(buf.String())

// Wrap error to provide status code.
return WithStatusCode(err, rsp.StatusCode)
}
91 changes: 91 additions & 0 deletions internal/gtserror/new_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package gtserror_test

import (
"bytes"
"fmt"
"io"
"net/http"
"net/url"
"strings"
"testing"

"github.com/superseriousbusiness/gotosocial/internal/gtserror"
)

func TestResponseError(t *testing.T) {
testResponseError(t, http.Response{
Body: toBody(`{"error": "user not found"}`),
Request: &http.Request{
Method: "GET",
URL: toURL("https://google.com/users/sundar"),
},
Status: "404 Not Found",
})
testResponseError(t, http.Response{
Body: toBody("Unauthorized"),
Request: &http.Request{
Method: "POST",
URL: toURL("https://google.com/inbox"),
},
Status: "401 Unauthorized",
})
testResponseError(t, http.Response{
Body: toBody(""),
Request: &http.Request{
Method: "GET",
URL: toURL("https://google.com/users/sundar"),
},
Status: "404 Not Found",
})
}

func testResponseError(t *testing.T, rsp http.Response) {
var body string
if rsp.Body == http.NoBody {
body = "<empty>"
} else {
var b []byte
rsp.Body, b = copyBody(rsp.Body)
trunc := len(b)
if trunc > 256 {
trunc = 256
}
body = string(b[:trunc])
}
expect := fmt.Sprintf(
"%s request to %s failed: status=\"%s\" body=\"%s\"",
rsp.Request.Method,
rsp.Request.URL.String(),
rsp.Status,
body,
)
err := gtserror.NewResponseError(&rsp)
if str := err.Error(); str != expect {
t.Errorf("unexpected error string: recv=%q expct=%q", str, expect)
}
}

func toURL(u string) *url.URL {
url, err := url.Parse(u)
if err != nil {
panic(err)
}
return url
}

func toBody(s string) io.ReadCloser {
if s == "" {
return http.NoBody
}
r := strings.NewReader(s)
return io.NopCloser(r)
}

func copyBody(rc io.ReadCloser) (io.ReadCloser, []byte) {
b, err := io.ReadAll(rc)
if err != nil {
panic(err)
}
r := bytes.NewReader(b)
return io.NopCloser(r), b
}
42 changes: 42 additions & 0 deletions internal/gtserror/util.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// GoToSocial
// Copyright (C) GoToSocial Authors [email protected]
// SPDX-License-Identifier: AGPL-3.0-or-later
//
// 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 gtserror

import (
"io"

"codeberg.org/gruf/go-byteutil"
)

// drainBody will produce a truncated output of the content
// of given io.ReadCloser body, useful for logs / errors.
func drainBody(body io.ReadCloser, trunc int) string {
// Limit response to 'trunc' bytes.
buf := make([]byte, trunc)

// Read body into err buffer.
n, _ := io.ReadFull(body, buf)

if n == 0 {
// No error body, return
// reasonable error str.
return "<empty>"
}

return byteutil.B2S(buf[:n])
}
21 changes: 16 additions & 5 deletions internal/httpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ import (
)

var (
// ErrInvalidRequest is returned if a given HTTP request is invalid and cannot be performed.
ErrInvalidRequest = errors.New("invalid http request")

// ErrInvalidNetwork is returned if the request would not be performed over TCP
ErrInvalidNetwork = errors.New("invalid network type")

Expand Down Expand Up @@ -90,6 +93,9 @@ type Config struct {
// cases to protect against forged / unknown content-lengths
// - protection from server side request forgery (SSRF) by only dialing
// out to known public IP prefixes, configurable with allows/blocks
// - retry-backoff logic for error temporary HTTP error responses
// - optional request signing
// - request logging
type Client struct {
client http.Client
badHosts cache.Cache[string, struct{}]
Expand Down Expand Up @@ -156,14 +162,14 @@ func New(cfg Config) *Client {
return &c
}

// Do ...
// Do will essentially perform http.Client{}.Do() with retry-backoff functionality.
func (c *Client) Do(r *http.Request) (*http.Response, error) {
return c.DoSigned(r, func(r *http.Request) error {
return nil // no request signing
})
}

// DoSigned ...
// DoSigned will essentially perform http.Client{}.Do() with retry-backoff functionality and requesting signing..
func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, err error) {
const (
// max no. attempts.
Expand All @@ -173,6 +179,11 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e
baseBackoff = 2 * time.Second
)

// First validate incoming request.
if err := ValidateRequest(r); err != nil {
return nil, err
}

// Get request hostname.
host := r.URL.Hostname()

Expand Down Expand Up @@ -234,8 +245,8 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e
return rsp, nil
}

// Generate error from status code for logging
err = errors.New(`http response "` + rsp.Status + `"`)
// Create loggable error from response status code.
err = fmt.Errorf(`http response: %s`, rsp.Status)

// Search for a provided "Retry-After" header value.
if after := rsp.Header.Get("Retry-After"); after != "" {
Expand Down Expand Up @@ -307,7 +318,7 @@ func (c *Client) DoSigned(r *http.Request, sign SignFunc) (rsp *http.Response, e
return
}

// do ...
// do wraps http.Client{}.Do() to provide safely limited response bodies.
func (c *Client) do(req *http.Request) (*http.Response, error) {
// Perform the HTTP request.
rsp, err := c.client.Do(req)
Expand Down
62 changes: 62 additions & 0 deletions internal/httpclient/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// GoToSocial
// Copyright (C) GoToSocial Authors [email protected]
// SPDX-License-Identifier: AGPL-3.0-or-later
//
// 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
}
4 changes: 1 addition & 3 deletions internal/transport/deliver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package transport

import (
"context"
"fmt"
"net/http"
"net/url"
"sync"
Expand Down Expand Up @@ -131,8 +130,7 @@ func (t *transport) deliver(ctx context.Context, b []byte, to *url.URL) error {

if code := rsp.StatusCode; code != http.StatusOK &&
code != http.StatusCreated && code != http.StatusAccepted {
err := fmt.Errorf("POST request to %s failed: %s", url, rsp.Status)
return gtserror.WithStatusCode(err, rsp.StatusCode)
return gtserror.NewResponseError(rsp)
}

return nil
Expand Down
4 changes: 1 addition & 3 deletions internal/transport/dereference.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package transport

import (
"context"
"fmt"
"io"
"net/http"
"net/url"
Expand Down Expand Up @@ -66,8 +65,7 @@ func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, erro
defer rsp.Body.Close()

if rsp.StatusCode != http.StatusOK {
err := fmt.Errorf("GET request to %s failed: %s", iriStr, rsp.Status)
return nil, gtserror.WithStatusCode(err, rsp.StatusCode)
return nil, gtserror.NewResponseError(rsp)
}

return io.ReadAll(rsp.Body)
Expand Down
Loading

0 comments on commit 2063d01

Please sign in to comment.