Skip to content

Commit

Permalink
feat: Limit reading bytes from request bodies (#168)
Browse files Browse the repository at this point in the history 
This change both limits the number of bytes that the SDK attempts to
read and buffer in memory, as well as delays as much work as possible.

It requires breaking changes to the exported API.

Notes on API changes:

- Request.FromHTTPRequest
+ NewRequest

  The difference is that NewRequest does not read the request body.
  FromHTTPRequest was called from HTTP middleware in integrations in the
  beginning of the request handling flow. Attempting to read the whole
  body there is often wasteful, since most requests should not result in
  reporting an error to Sentry.

  With this change, Scope.SetRequest keeps a lazy buffer that clones a
  limited portion of the request body only if it is read by the user.

- Scope.SetRequest(Request)
+ Scope.SetRequest(*http.Request)

  For most integrations (fasthttp being the only exception), this means
  less work needs to be done when handling a request. Converting from
  http.Request to sentry.Request is only done when there is an error to
  be reported.
  It also means we can keep a reference to the request body without
  reading all bytes upfront.

- Event.Request type Request
+ Event.Request type *Request

  Avoids copying of possibly large struct.
  • Loading branch information
@rhcarvalho
rhcarvalho authored Feb 25, 2020
1 parent 7b7d396 commit db5e5da
Show file tree
Hide file tree
Showing 15 changed files with 353 additions and 124 deletions.
2 changes: 1 addition & 1 deletion echo/sentryecho.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func New(options Options) echo.MiddlewareFunc {
func (h *handler) handle(next echo.HandlerFunc) echo.HandlerFunc {
return func(ctx echo.Context) error {
hub := sentry.CurrentHub().Clone()
hub.Scope().SetRequest(sentry.Request{}.FromHTTPRequest(ctx.Request()))
hub.Scope().SetRequest(ctx.Request())
ctx.Set(valuesKey, hub)
defer h.recoverWithSentry(hub, ctx.Request())
return next(ctx)
Expand Down
37 changes: 19 additions & 18 deletions fasthttp/sentryfasthttp.go
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package sentryfasthttp

import (
"bytes"
"context"
"fmt"
"net"
"strings"
"io/ioutil"
"net/http"
"net/url"
"time"

"github.com/getsentry/sentry-go"
Expand Down Expand Up @@ -62,7 +64,9 @@ func New(options Options) *Handler {
func (h *Handler) Handle(handler fasthttp.RequestHandler) fasthttp.RequestHandler {
return func(ctx *fasthttp.RequestCtx) {
hub := sentry.CurrentHub().Clone()
hub.Scope().SetRequest(extractRequestData(ctx))
scope := hub.Scope()
scope.SetRequest(convert(ctx))
scope.SetRequestBody(ctx.Request.Body())
ctx.SetUserValue(valuesKey, hub)
defer h.recoverWithSentry(hub, ctx)
handler(ctx)
Expand Down Expand Up @@ -93,44 +97,41 @@ func GetHubFromContext(ctx *fasthttp.RequestCtx) *sentry.Hub {
return nil
}

func extractRequestData(ctx *fasthttp.RequestCtx) sentry.Request {
func convert(ctx *fasthttp.RequestCtx) *http.Request {
defer func() {
if err := recover(); err != nil {
sentry.Logger.Printf("%v", err)
}
}()

r := sentry.Request{}
r := new(http.Request)

r.Method = string(ctx.Method())
uri := ctx.URI()
r.URL = fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path())
// Ignore error.
r.URL, _ = url.Parse(fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path()))

// Headers
headers := make(map[string]string)
r.Header = make(http.Header)
r.Header.Add("Host", string(ctx.Host()))
ctx.Request.Header.VisitAll(func(key, value []byte) {
headers[string(key)] = string(value)
r.Header.Add(string(key), string(value))
})
headers["Host"] = string(ctx.Host())
r.Headers = headers
r.Host = string(ctx.Host())

// Cookies
cookies := []string{}
ctx.Request.Header.VisitAllCookie(func(key, value []byte) {
cookies = append(cookies, fmt.Sprintf("%s=%s", key, value))
r.AddCookie(&http.Cookie{Name: string(key), Value: string(value)})
})
r.Cookies = strings.Join(cookies, "; ")

// Env
if addr, port, err := net.SplitHostPort(ctx.RemoteAddr().String()); err == nil {
r.Env = map[string]string{"REMOTE_ADDR": addr, "REMOTE_PORT": port}
}
r.RemoteAddr = ctx.RemoteAddr().String()

// QueryString
r.QueryString = string(ctx.URI().QueryString())
r.URL.RawQuery = string(ctx.URI().QueryString())

// Body
r.Data = string(ctx.Request.Body())
r.Body = ioutil.NopCloser(bytes.NewReader(ctx.Request.Body()))

return r
}
63 changes: 60 additions & 3 deletions fasthttp/sentryfasthttp_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package sentryfasthttp_test

import (
"fmt"
"net"
"net/http"
"strings"
"testing"
"time"

Expand All @@ -15,6 +17,8 @@ import (
)

func TestIntegration(t *testing.T) {
largePayload := strings.Repeat("Large", 3*1024) // 15 KB

tests := []struct {
Path string
Method string
Expand All @@ -32,7 +36,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelFatal,
Message: "test",
Request: sentry.Request{
Request: &sentry.Request{
URL: "http://example.com/panic",
Method: "GET",
Headers: map[string]string{
Expand All @@ -55,7 +59,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "post: payload",
Request: sentry.Request{
Request: &sentry.Request{
URL: "http://example.com/post",
Method: "POST",
Data: "payload",
Expand All @@ -78,7 +82,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "get",
Request: sentry.Request{
Request: &sentry.Request{
URL: "http://example.com/get",
Method: "GET",
Headers: map[string]string{
Expand All @@ -89,6 +93,59 @@ func TestIntegration(t *testing.T) {
},
},
},
{
Path: "/post/large",
Method: "POST",
Body: largePayload,
Handler: func(ctx *fasthttp.RequestCtx) {
hub := sentryfasthttp.GetHubFromContext(ctx)
hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(ctx.Request.Body())/1024))
},

WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "post: 15 KB",
Request: &sentry.Request{
URL: "http://example.com/post/large",
Method: "POST",
// Actual request body omitted because too large.
Data: "",
Headers: map[string]string{
"Content-Length": "15360",
"Content-Type": "application/x-www-form-urlencoded",
"Host": "example.com",
"User-Agent": "fasthttp",
},
},
},
},
{
Path: "/post/body-ignored",
Method: "POST",
Body: "client sends, fasthttp always reads, SDK reports",
Handler: func(ctx *fasthttp.RequestCtx) {
hub := sentryfasthttp.GetHubFromContext(ctx)
hub.CaptureMessage("body ignored")
},

WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "body ignored",
Request: &sentry.Request{
URL: "http://example.com/post/body-ignored",
Method: "POST",
// Actual request body included because fasthttp always
// reads full request body.
Data: "client sends, fasthttp always reads, SDK reports",
Headers: map[string]string{
"Content-Length": "48",
"Content-Type": "application/x-www-form-urlencoded",
"Host": "example.com",
"User-Agent": "fasthttp",
},
},
},
},
}

eventsCh := make(chan *sentry.Event, len(tests))
Expand Down
2 changes: 1 addition & 1 deletion gin/sentrygin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func New(options Options) gin.HandlerFunc {

func (h *handler) handle(ctx *gin.Context) {
hub := sentry.CurrentHub().Clone()
hub.Scope().SetRequest(sentry.Request{}.FromHTTPRequest(ctx.Request))
hub.Scope().SetRequest(ctx.Request)
ctx.Set(valuesKey, hub)
defer h.recoverWithSentry(hub, ctx.Request)
ctx.Next()
Expand Down
4 changes: 2 additions & 2 deletions http/sentryhttp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func New(options Options) *Handler {
func (h *Handler) Handle(handler http.Handler) http.Handler {
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
hub := sentry.CurrentHub().Clone()
hub.Scope().SetRequest(sentry.Request{}.FromHTTPRequest(r))
hub.Scope().SetRequest(r)
ctx := sentry.SetHubOnContext(
r.Context(),
hub,
Expand All @@ -66,7 +66,7 @@ func (h *Handler) Handle(handler http.Handler) http.Handler {
func (h *Handler) HandleFunc(handler http.HandlerFunc) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
hub := sentry.CurrentHub().Clone()
hub.Scope().SetRequest(sentry.Request{}.FromHTTPRequest(r))
hub.Scope().SetRequest(r)
ctx := sentry.SetHubOnContext(
r.Context(),
hub,
Expand Down
64 changes: 60 additions & 4 deletions http/sentryhttp_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package sentryhttp_test

import (
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand All @@ -15,6 +16,8 @@ import (
)

func TestIntegration(t *testing.T) {
largePayload := strings.Repeat("Large", 3*1024) // 15 KB

tests := []struct {
Path string
Method string
Expand All @@ -32,7 +35,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelFatal,
Message: "test",
Request: sentry.Request{
Request: &sentry.Request{
URL: "/panic",
Method: "GET",
Headers: map[string]string{
Expand All @@ -58,7 +61,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "post: payload",
Request: sentry.Request{
Request: &sentry.Request{
URL: "/post",
Method: "POST",
Data: "payload",
Expand All @@ -80,7 +83,7 @@ func TestIntegration(t *testing.T) {
WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "get",
Request: sentry.Request{
Request: &sentry.Request{
URL: "/get",
Method: "GET",
Headers: map[string]string{
Expand All @@ -90,6 +93,60 @@ func TestIntegration(t *testing.T) {
},
},
},
{
Path: "/post/large",
Method: "POST",
Body: largePayload,
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
hub := sentry.GetHubFromContext(r.Context())
body, err := ioutil.ReadAll(r.Body)
if err != nil {
t.Error(err)
}
hub.CaptureMessage(fmt.Sprintf("post: %d KB", len(body)/1024))
}),

WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "post: 15 KB",
Request: &sentry.Request{
URL: "/post/large",
Method: "POST",
// Actual request body omitted because too large.
Data: "",
Headers: map[string]string{
"Accept-Encoding": "gzip",
"Content-Length": "15360",
"User-Agent": "Go-http-client/1.1",
},
},
},
},
{
Path: "/post/body-ignored",
Method: "POST",
Body: "client sends, server ignores, SDK doesn't read",
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
hub := sentry.GetHubFromContext(r.Context())
hub.CaptureMessage("body ignored")
}),

WantEvent: &sentry.Event{
Level: sentry.LevelInfo,
Message: "body ignored",
Request: &sentry.Request{
URL: "/post/body-ignored",
Method: "POST",
// Actual request body omitted because not read.
Data: "",
Headers: map[string]string{
"Accept-Encoding": "gzip",
"Content-Length": "46",
"User-Agent": "Go-http-client/1.1",
},
},
},
},
}

eventsCh := make(chan *sentry.Event, len(tests))
Expand Down Expand Up @@ -124,7 +181,6 @@ func TestIntegration(t *testing.T) {
wantRequest := tt.WantEvent.Request
wantRequest.URL = srv.URL + wantRequest.URL
wantRequest.Headers["Host"] = srv.Listener.Addr().String()
tt.WantEvent.Request = wantRequest
want = append(want, tt.WantEvent)

req, err := http.NewRequest(tt.Method, srv.URL+tt.Path, strings.NewReader(tt.Body))
Expand Down
Loading

0 comments on commit db5e5da

Please sign in to comment.