Skip to content

Commit 17421c4

Browse files
authored
Fix nil pointer dereference with malformed URLs when tracing is enabled (#1055)
1 parent 55af278 commit 17421c4

File tree

4 files changed

+80
-6
lines changed

4 files changed

+80
-6
lines changed

fasthttp/sentryfasthttp.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,11 @@ func convert(ctx *fasthttp.RequestCtx) *http.Request {
152152
r.Method = string(ctx.Method())
153153

154154
uri := ctx.URI()
155-
url, err := url.Parse(fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path()))
156-
if err == nil {
157-
r.URL = url
155+
r.URL = &url.URL{Path: string(uri.Path())}
156+
r.URL.RawQuery = string(uri.QueryString())
157+
158+
if parsedURL, err := url.Parse(fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path())); err == nil {
159+
r.URL = parsedURL
158160
r.URL.RawQuery = string(uri.QueryString())
159161
}
160162

fasthttp/sentryfasthttp_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,3 +571,35 @@ func TestSetHubOnContext(t *testing.T) {
571571
t.Fatalf("expected hub to be %v, but got %v", hub, retrievedHub)
572572
}
573573
}
574+
575+
// TestMalformedURLNoPanic verifies that malformed URLs don't cause panics
576+
// when tracing is enabled
577+
func TestMalformedURLNoPanic(t *testing.T) {
578+
err := sentry.Init(sentry.ClientOptions{
579+
EnableTracing: true,
580+
TracesSampleRate: 1.0,
581+
})
582+
if err != nil {
583+
t.Fatal(err)
584+
}
585+
586+
sentryHandler := sentryfasthttp.New(sentryfasthttp.Options{})
587+
588+
handler := sentryHandler.Handle(func(ctx *fasthttp.RequestCtx) {
589+
ctx.SetStatusCode(fasthttp.StatusOK)
590+
ctx.SetBodyString("OK")
591+
})
592+
593+
ctx := &fasthttp.RequestCtx{}
594+
ctx.Request.SetRequestURI("http://localhost/%zz")
595+
ctx.Request.Header.SetMethod("GET")
596+
ctx.Request.SetHost("localhost")
597+
ctx.Request.Header.Set("User-Agent", "fasthttp")
598+
599+
handler(ctx)
600+
601+
// Should complete successfully without panic
602+
if ctx.Response.StatusCode() != fasthttp.StatusOK {
603+
t.Errorf("Expected 200, got %d", ctx.Response.StatusCode())
604+
}
605+
}

fiber/sentryfiber.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,11 @@ func convert(ctx *fiber.Ctx) *http.Request {
152152
r.Method = utils.CopyString(ctx.Method())
153153

154154
uri := ctx.Request().URI()
155-
url, err := url.Parse(fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path()))
156-
if err == nil {
157-
r.URL = url
155+
r.URL = &url.URL{Path: string(uri.Path())}
156+
r.URL.RawQuery = string(uri.QueryString())
157+
158+
if parsedURL, err := url.Parse(fmt.Sprintf("%s://%s%s", uri.Scheme(), uri.Host(), uri.Path())); err == nil {
159+
r.URL = parsedURL
158160
r.URL.RawQuery = string(uri.QueryString())
159161
}
160162

fiber/sentryfiber_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"errors"
55
"fmt"
66
"net/http"
7+
"net/url"
78
"reflect"
89
"strings"
910
"testing"
@@ -594,3 +595,40 @@ func TestSetHubOnContext(t *testing.T) {
594595
t.Fatalf("Expected status code %d, got %d", http.StatusOK, resp.StatusCode)
595596
}
596597
}
598+
599+
// TestMalformedURLNoPanic verifies that malformed URLs don't cause panics
600+
// when tracing is enabled,
601+
func TestMalformedURLNoPanic(t *testing.T) {
602+
err := sentry.Init(sentry.ClientOptions{
603+
EnableTracing: true,
604+
TracesSampleRate: 1.0,
605+
})
606+
if err != nil {
607+
t.Fatal(err)
608+
}
609+
610+
app := fiber.New()
611+
app.Use(sentryfiber.New(sentryfiber.Options{Timeout: 3 * time.Second, WaitForDelivery: true}))
612+
613+
app.Get("/*", func(c *fiber.Ctx) error {
614+
return c.SendString("OK")
615+
})
616+
617+
req := &http.Request{
618+
Method: "GET",
619+
URL: &url.URL{Scheme: "http", Host: "localhost", Path: "/%zz"},
620+
Header: make(http.Header),
621+
Host: "localhost",
622+
}
623+
req.Header.Set("User-Agent", "fiber")
624+
625+
resp, err := app.Test(req)
626+
if err != nil {
627+
t.Fatalf("Request failed: %v", err)
628+
}
629+
defer resp.Body.Close()
630+
631+
if resp.StatusCode != http.StatusOK {
632+
t.Errorf("Expected 200, got %d", resp.StatusCode)
633+
}
634+
}

0 commit comments

Comments
 (0)